35161 - Choosing "Save Page As" to a read-only destination crashes

Status: VERIFIED FIXED

(Core Graveyard :: File Handling, defect, P2)

Description

[Miguel Covarrubias]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux 2.2.5-15 i586; en-US; m14)
BuildID:    2000040616

Trying to save a file to a directory where I don't have permissions
produces a succession of Alert boxes saying "Unknown error".  After
about three of these Alerts, Mozilla crashes.


Reproducible: Always
Steps to Reproduce:
1.Try to download a file to a directory without write permissions
2.
3.

Actual Results:  Three uninformative Alerts went up and Mozilla crashed.


Expected Results:  Put up an informative Alert and put up the file chooser
dialog again for the user to select a writable place to put the file.

Comment 1

[R.K.Aa.]

this is a variant of bug 27609 but the crash is new. (27609 was a leftover from
bug 23821 where mozilla actually was hanging. Marked fixed 2000-02-10)

Comment 2

[Asa Dotzler [:asa]]

-> networking

Comment 3

[Gagan]

->law

Comment 4

[Asa Dotzler [:asa]]

*** Bug 36534 has been marked as a duplicate of this bug. ***

Comment 5

[leger]

Adding crash keyword.

Comment 6

[don]

Move to M21 target milestone.

Comment 7

[Bill Law]

Upping to M18; this is bad and should be relatively easy to remedy.

Comment 8

[Tomer Brisker]

*** Bug 45629 has been marked as a duplicate of this bug. ***

Comment 9

[Tomer Brisker]

*** Bug 47751 has been marked as a duplicate of this bug. ***

Comment 10

[verah (gone)]

*** Bug 47071 has been marked as a duplicate of this bug. ***

Comment 11

[verah (gone)]

Nav triage team: Changing summary to something we understand; [nsbeta3+]

Comment 12

[Bill Law]

Marking P1.

Comment 13

[Phil Peterson]

PDT downgrading to P2 and putting [PDTP2] in status whiteboard

Comment 14

[Bill Law]

I have two minor patches which avoid the crash.  Waiting for Warren to OK the 
buffered streams one.

Index: nsStreamXferOp.cpp
===================================================================
RCS file: /cvsroot/mozilla/xpfe/components/xfer/src/nsStreamXferOp.cpp,v
retrieving revision 1.32
diff -w -r1.32 nsStreamXferOp.cpp
330a331
>                     break;
333a335
>                 break;

Index: base/src/nsBufferedStreams.cpp
> ===================================================================
> RCS file: /cvsroot/mozilla/netwerk/base/src/nsBufferedStreams.cpp,v
> retrieving revision 1.8
> diff -r1.8 nsBufferedStreams.cpp
> 335a336,339
> >     if (!mStream) {
> >         // Stream already cancelled/flushed; probably because of error.
> >         return NS_OK;
> >     }

Comment 15

[Richard Waid]

*** Bug 49919 has been marked as a duplicate of this bug. ***

Comment 16

[don]

Move to RTM.

Comment 17

[don]

Adding patch keyword.

Comment 18

[Phil Peterson]

PDT marking [rtm need info] since no code reviews are listed.

Comment 19

[Bill Law]

Attachment: Comprehensive patch for this bug and also 48620, 50697, and 42173)

Comment 20

[don]

Fix in hand.

Comment 21

[Bill Law]

*** Bug 50697 has been marked as a duplicate of this bug. ***

Comment 22

[Bill Law]

r=mcafee

I'm deeming myself module owner for the xfer stuff, and I approve it.

Going to mscott for super-review (he knows this stuff).

Comment 23

[Bill Law]

*** Bug 48620 has been marked as a duplicate of this bug. ***

Comment 24

[Scott MacGregor]

Hey Bill, your patch looks great to me. I assume the changes to getString that
you made fix the exception errors that get thrown every now and then?

Also, I take it you have approval from I10N for the localization changes?
Assuming that's cool...
sr=mscott

Comment 25

[Bill Law]

Yes, getString was barfing when fetching "unknownTime" and I fixed that as well.

You are right about the l10n clearance on the new strings.  We're working on it.

Thanks.

Comment 26

[don]

Adding cc to L10n folks ...

Comment 27

[msanz]

You can assume now that it's cool with L10n :-) Approved.

Comment 28

[Jaime Rodriguez, Jr.]

Adding myself to cc: list.

Comment 29

[don]

r=mcafee and sr=mscott

PDT, please approve.

Comment 30

[selmer (gone)]

rtm-, this is much bigger than a breadbasket.  Is there a very very small fix
that makes the crash go away?

Comment 31

[Bill Law]

Yes.  The simple patch to nsBufferedStream.cpp (posted to description a long
time ago) will stop the crash 99.9% of the time.  The additional "break"
statements to nsStreamXferOp.cpp will stop the crash in the other .1% of the time.

The message box would still suck, though.

Comment 32

[kill this account]

on my linux debug branch build of today when I try to save a file to
/junk.hmtl it pops up a dialog saying "Unknown error [3 80004005]".

I note the "Saving File" dialog shows the saving "To:" as "//unk.html"
which is missing the leading "j".

Comment 33

[Bill Law]

Gotta fix this one.  I have the patch but it doesn't work any more :-(.  Should
be relatively easy to get it back in gear.

Comment 34

[Bill Law]

Setting target milestone.

Comment 35

[R.K.Aa.]

*** Bug 64924 has been marked as a duplicate of this bug. ***

Comment 36

[Bill Law]

Attachment: test

Comment 37

[Bill Law]

This has been fixed.  There is still a problem handling this error in many 
cases, though; see bug 67216.

Comment 38

[Chris Petersen]

Verified on the linux 2003-01-23-05 trunk.