Closed Bug 30784 Opened 23 years ago Closed 4 years ago

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

Categories

(MailNews Core :: Backend, defect)

x86
Linux
defect
Not set
critical

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: rzach, Unassigned)

References

Details

(Keywords: dataloss, Whiteboard: [nsbeta3-][PDT-] w/b minus on 3/8 - nsIFileStream is ill-imlemented)

Attachments

(1 file)

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
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.
Assignee: phil → jefft
Summary: No file operations errors in MailNews → Save attachment, copy message to bogus location, no error msg
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.

Severity: normal → critical
Keywords: beta1
I completely agree we shouldn't just silently failed. Bad. Bad. Bad.
Status: NEW → ASSIGNED
Summary: Save attachment, copy message to bogus location, no error msg → to bogus location, no error msg
Target Milestone: M14
Summary: to bogus location, no error msg → Save attachment, copy message to bogus location, no error msg
[PDT+] w/b minus on 3/8
Whiteboard: [PDT+] w/b minus on 3/8
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
    }
    
}
Whiteboard: [PDT+] w/b minus on 3/8 → [PDT-] w/b minus on 3/8 - nsIFileStream is ill-imlemented
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.
Attached patch A fixSplinter Review
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
QA Contact: lchiang → laurel
Jeff, I tried this with Linux build 2000.03.14.18, and it's still broken.  Too
early?
Please, try today's build. You might be a little bit too early. Thanks,
Note: when you see the error message it ends with ">>>>>>1.6" this will be fix 
when I fix bug 31929.
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.
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?
Yes, the fix is trunk (tip) build only. I probably missed testing the directory 
as the file name case.
Reopen for directory case.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
No, I haven't noticed that weird behavior. I'll do some more testing.
Status: REOPENED → ASSIGNED
M16 ...
Target Milestone: M14 → M16
Not beta2 stopper, marking M18.  Please add beta2 keyword if you disagree.
Target Milestone: M16 → M18
seems like this should be fixed.
Keywords: beta1nsbeta3
Keywords: mail2
- per mail triage.
Whiteboard: [PDT-] w/b minus on 3/8 - nsIFileStream is ill-imlemented → [nsbeta3-][PDT-] w/b minus on 3/8 - nsIFileStream is ill-imlemented
Target Milestone: M18 → Future
sorry for the extra email. Removing mail2 keyword.
Keywords: mail2
reassigning jefft's bugs to naving
Assignee: jefft → naving
Status: ASSIGNED → NEW
over to ducarroz
Assignee: naving → ducarroz
Status: NEW → ASSIGNED
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.
Product: MailNews → Core
another one for your talents
(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.
Assignee: ducarroz → nobody
Status: ASSIGNED → NEW
Priority: P3 → --
QA Contact: laurel → backend
Target Milestone: Future → ---
Product: Core → MailNews Core
Jason, can you test?
Wayne, I'd be glad to, but I will be out for the next week. The earliest I can help is Nov. 9.
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.
Keywords: dataloss
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.
Depends on: 567585
Depends on: 928250
No longer depends on: 567585
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.
(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
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
WFM per comment 33
Status: NEW → RESOLVED
Closed: 23 years ago4 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.