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

RESOLVED WORKSFORME

Status

--
critical
RESOLVED WORKSFORME
19 years ago
6 months ago

People

(Reporter: rzach, Unassigned)

Tracking

({dataloss})

Trunk
x86
Linux
dataloss

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(1 attachment)

(Reporter)

Description

19 years ago
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

19 years ago
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
(Reporter)

Comment 2

19 years ago
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

Comment 3

19 years ago
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

Updated

19 years ago
Summary: to bogus location, no error msg → Save attachment, copy message to bogus location, no error msg

Comment 4

19 years ago
[PDT+] w/b minus on 3/8
Whiteboard: [PDT+] w/b minus on 3/8

Comment 5

19 years ago
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

Comment 6

19 years ago
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

19 years ago
Created attachment 6399 [details] [diff] [review]
A fix

Comment 8

19 years ago
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED

Updated

19 years ago
QA Contact: lchiang → laurel
(Reporter)

Comment 9

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

Comment 10

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

Comment 11

19 years ago
Note: when you see the error message it ends with ">>>>>>1.6" this will be fix 
when I fix bug 31929.
(Reporter)

Comment 12

19 years ago
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.
(Reporter)

Comment 13

19 years ago
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

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

Comment 15

19 years ago
Reopen for directory case.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 16

19 years ago
No, I haven't noticed that weird behavior. I'll do some more testing.
Status: REOPENED → ASSIGNED

Comment 17

19 years ago
M16 ...
Target Milestone: M14 → M16

Comment 18

19 years ago
Not beta2 stopper, marking M18.  Please add beta2 keyword if you disagree.
Target Milestone: M16 → M18

Comment 19

18 years ago
seems like this should be fixed.
Keywords: beta1 → nsbeta3

Updated

18 years ago
Keywords: mail2

Comment 20

18 years ago
- 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

Comment 21

18 years ago
sorry for the extra email. Removing mail2 keyword.
Keywords: mail2

Comment 22

18 years ago
reassigning jefft's bugs to naving
Assignee: jefft → naving
Status: ASSIGNED → NEW

Comment 23

17 years ago
over to ducarroz
Assignee: naving → ducarroz

Updated

17 years ago
Status: NEW → ASSIGNED

Comment 24

16 years ago
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

Comment 25

11 years ago
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 → ---
(Assignee)

Updated

10 years ago
Product: Core → MailNews Core

Comment 27

9 years ago
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.

Updated

9 years ago
Keywords: dataloss

Comment 30

7 years ago
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

Updated

5 years ago
Depends on: 928250
No longer depends on: 567585

Comment 31

5 years ago
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

Comment 33

5 years ago
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

6 months ago
WFM per comment 33
Status: NEW → RESOLVED
Last Resolved: 19 years ago6 months ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.