30784 - Save attachment, copy message to bogus location, no error msg

Status: RESOLVED WORKSFORME

(MailNews Core :: Backend, defect)

Description

[Richard Zach]

There are no error messages on file operation errors in mailnews.  Saving a
message or saving an attachment to a directory where the user has no access
priviledges, is write protected, or where the path exists but points to a
directory, should result in an error message rather than silently fail.

To reproduce:
1. Select message
2. File | Save As
3. Enter path that points to non-writeable directory or non existant directory
4. Click OK

Actual result: dialog is dismissed, file not saved

Expected result: error message

Linux build 2000.03.06.09

Comment 1

[Phil Peterson]

This bug is a little too general. Over-general bugs are hard to assign to one
engineer and hard to verify fixed. Let's restrict it to saving attachments and
copying messages. Please file other bugs on other problems. Reassigning this one
to jefft.

Comment 2

[Richard Zach]

Those were the only two cases I meant.  I'm marking this critcal and beta1 since
it can easily result in data loss in the following scenario: save message using
File | Save > File, delete message, compact folder.  The message is now gone,
since I entered the wrong path, or the disk was full, or the network connection
to my NFS down, or I forgot to mount the diskette, or whatever, and I didn't
realize it because I got no error message.

Comment 3

[jefft]

I completely agree we shouldn't just silently failed. Bad. Bad. Bad.

Comment 4

[Phil Peterson]

[PDT+] w/b minus on 3/8

Comment 5

[jefft]

nsIFileStream is ill-implemented. It does not propagate file opening error back 
to the client. There is no eay way to come up with a reasonable fix at the 
moment. I don't know who owns the nsIFileStream and nsIFileSpec. We need to have 
a dedicated owner for this. Taking out the PDT+ sign.

See the following, the error occurs in the constructor.


//------------------------------------------------------------------------------
----------
FileImpl::FileImpl(const nsFileSpec& inFile, int nsprMode, PRIntn accessMode)
//------------------------------------------------------------------------------
----------
: mFileDesc(nsnull)
, mNSPRMode(-1)
, mFailed(PR_FALSE)
, mEOF(PR_FALSE)
, mLength(-1)
, mGotBuffers(PR_FALSE)
{
    NS_INIT_REFCNT();

    mWriteCursor = nsnull;
    mWriteLimit  = nsnull;

    nsresult rv = Open(inFile, nsprMode, accessMode);		// this sets 
nsprMode
    
    if (NS_FAILED(rv))
    {
#if DEBUG
        char *fileName = inFile.GetLeafName();
        printf("Opening file %s failed\n", fileName);
        nsCRT::free(fileName);
#endif
    }
    
}

Comment 6

[jefft]

There are too many places people are not paying attention to the return code or 
ignore the return code intentionally. The fix will not be pretty, messy 
actually. The strategy is to patch here and there to avoid big code changes.

Comment 7

[jefft]

Attachment: A fix

Comment 8

[jefft]

Fix checked in.

Comment 9

[Richard Zach]

Jeff, I tried this with Linux build 2000.03.14.18, and it's still broken.  Too
early?

Comment 10

[jefft]

Please, try today's build. You might be a little bit too early. Thanks,

Comment 11

[jefft]

Note: when you see the error message it ends with ">>>>>>1.6" this will be fix 
when I fix bug 31929.

Comment 12

[Richard Zach]

This works on 2000.03.15.09 (trunk build, I think) but not on 2000.03.15.11-nb1b.

There's one thing that's still broken: saving a message and giving a directory
path as the file name still fails silently.  Saving an attachment and giving a
directory as the file name produces an error.

Comment 13

[Richard Zach]

Here's another weird thing: On IMAP folder, save a message to bogus location.  I
get the error dialog.  Then, some messages fail to display (ie: header area
doesn't refresh, body area is grey).  It only happens on IMAP, and only if the
save fails.  Switching folders makes the problem go away.  Maybe some synch
problem with the server?  Do you see this?

Comment 14

[jefft]

Yes, the fix is trunk (tip) build only. I probably missed testing the directory 
as the file name case.

Comment 15

[Phil Peterson]

Reopen for directory case.

Comment 16

[jefft]

No, I haven't noticed that weird behavior. I'll do some more testing.

Comment 17

[jefft]

M16 ...

Comment 18

[selmer (gone)]

Not beta2 stopper, marking M18.  Please add beta2 keyword if you disagree.

Comment 19

[David :Bienvenu]

seems like this should be fixed.

Comment 20

[lchiang]

- per mail triage.

Comment 21

[scottputterman]

sorry for the extra email. Removing mail2 keyword.

Comment 22

[scottputterman]

reassigning jefft's bugs to naving

Comment 23

[Navin Gupta]

over to ducarroz

Comment 24

[Stewart]

I receive a "Save As" error (two, actually) in Mozilla's Mail & News. In 1.2a
the problem seems to have decreased in severity, but still exists nonetheless.

At one time, saving an attachment (an image, for example) from a newsgroup
message would crash the entire browser. Right-Clicking the attachment filename
in the "Attachments" window and selecting "Save As" would do something and
appear to return to normal; but no file would be saved.

In 1.2a I can now save the attachment via the Attachments window, but not by
right-clicking an inline image.

Test scenario involves the alt.binaries.pictures.wallpaper newsgroup and
Leafnode running on my server.

Comment 25

[Wayne Mery (:wsmwk)]

another one for your talents

Comment 26

[Tony Mechelynck [:tonymec]]

(In reply to comment #25)
> another one for your talents
> 

Answer: I don't know. My mail accounts are defined for the root profile (yeah, I know it is bad form) and the root user has write permit everywhere.

I tried giving a directory name in the "file name box" of the file picker, but it switched to that directory, still asking for a file name.

--------------------
No significant activity in a year and a half before Wayne's comment quoted above. I'm resetting A+QA. Jean-François, if you still want this bug, go ahead and take it back.

Comment 27

[Wayne Mery (:wsmwk)]

Jason, can you test?

Comment 28

[Jason Oster (:Parasyte)]

Wayne, I'd be glad to, but I will be out for the next week. The earliest I can help is Nov. 9.

Comment 29

[Jason Oster (:Parasyte)]

Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.10pre) Gecko/20100407 Lightning/1.0b1 Shredder/3.0.5pre

The STR in comment #0 still has the same result: File not saved, no error message.  Additionally, the error console is empty.  (I used /usr/ as my test directory when saving a message, on Ubuntu 9.10 x86_64)


Wayne, to answer your question, yes I can see a potential for data loss here.

Comment 30

[ISHIKAWA, Chiaki]

I think at least partially (failure to notify error when the attachment is written to a write-protected directory) this is a dup of 
Last Comment Bug 567585 - TB3 fails to raise an error when it tries to save an attachment to write-protected directory.

Comment 31

[ISHIKAWA, Chiaki]

I just noticed that this bugzilla entry is filed in, gasp, 2000(!?).
A dataloss bug still unfixed more than a dozen years later is a world-record holder, isn't it?
I was just checking the related bugzilla entries and could not resist writing this. Hope it gets fixed soon. I wonder how many users were lost due this error/bug. Sigh.

Comment 32

[Tony Mechelynck [:tonymec]]

(In reply to ISHIKAWA, Chiaki from comment #31)
> I just noticed that this bugzilla entry is filed in, gasp, 2000(!?).
> A dataloss bug still unfixed more than a dozen years later is a world-record
> holder, isn't it?
> I was just checking the related bugzilla entries and could not resist
> writing this. Hope it gets fixed soon. I wonder how many users were lost due
> this error/bug. Sigh.

Better resist next time: see https://bugzilla.mozilla.org/page.cgi?id=etiquette.html points 1.1 and 1.2

Comment 33

[ISHIKAWA, Chiaki]

I this bug can be closed now that
bug 928250 is fixed (to a degree.)

I tested the "save a message to write-protected directory scenario" and
it is already handled 24.4.0 (even). The message is slightly misleading, though.
The error message states that the message could not be saved [GREAT IMPROVEMENT], 
but then asks the user to "check the filename" 
(instead of saying clearly that the directory was write-protected and
that is why the save failed. I think checking errno would give us that info [under POSIXen at least].)

In any case, there *IS* an error message now :-)

TIA

Comment 34

[Wayne Mery (:wsmwk)]

WFM per comment 33