Testcase coming up in a few minutes. Mozilla/5.0 (Windows; U; Win98; rv:1.7.3) Gecko/20040913 Firefox/0.10 Basically, I had written an article for my blog (not yet posted) whereby a short script would convert a textarea's contents into a data: URI for saving. So with a content-type of application/octet-stream, Firefox would try to save it. Unfortunately, it would never go beyond the "Starting" phase of the "download". When you hit cancel, bye-bye directory's contents, except for the first file. Repeated testcase in Mozilla 1.8a3: did NOT see the same results. Example worked perfectly.
Created attachment 159051 [details] Testcase HTML page Steps to reproduce: (1) Tools, Options, Downloads. Set the directory to a brand-new one. (2) In said download directory, manually create three or four new files, perhaps a directory or two. (3) Open attachment, scroll to bottom and click "Save File" button. (4) Mozilla Firefox asks you whether you want to open the file or save it... choose "Save to disk". (5) Download Manager appears with "Starting" message for Downloads directory (this is probably NOT what should happen. It should show an actual file being downloaded.) (6) Hit Cancel link in Download Manager. Expected result: Download cancels safely. Actual result: All but one file in your downloads directory are completely gone, unrecoverable. Reproducible: Always
blocking1.0? because of traumatic dataloss. Yes, it's a rare scenario, but it's very evil.
Further testing shows the only reason any file in the target directory survived -- for that matter, the target directory itself -- is because said file was marked read-only.
Sounds like some of the (forked) front-end code is making silly assumptions about the ability to get a non-empty filename off a URI. Since it's forked, I happen to not really care.
I've tracked the problem back to http://lxr.mozilla.org/aviarybranch/source/uriloader/exthandler/nsExternalHelperAppService.cpp . I think it unusual that we don't get a file picker to find out the preferred filename for this file.
whoa, what exactly in that file does this?
I'm betting that the temp file is being set to the directory (by the Firefox download manager breakage) and then that PR_TRUE in the delete that you fixed on trunk recently kicks in.
nsExternalAppHandler::InitializeDownload is suspicious, insomuch as it is the only code which refers to an aDownload object. For that matter, I should clarify that it very well could be nsDownloadManager.cpp . I thought nsExternalAppHandler was responsible because I didn't see it specify or request a filename right away.
(In reply to comment #7) > I'm betting that the temp file is being set to the directory (by the Firefox > download manager breakage) and then that PR_TRUE in the delete that you fixed on > trunk recently kicks in. we should make sure that the patch for bug 259890 lands on the aviary and 1.7 branches.
:( Note bug 259890 comment 7. The patch for Mozilla trunk will not apply to Firefox 1.0 branch.
Comment on attachment 160030 [details] [diff] [review] Ported patch r+sr=bzbarsky
Comment on attachment 160030 [details] [diff] [review] Ported patch Actually, clearing reviews. This patch is on the wrong bug. It may well fix this bug (in that the directory will no longer be wiped), but even with it in you won't be able to save...
Comment on attachment 160030 [details] [diff] [review] Ported patch obsoleting, as this is being covered by bug 259890
I just committed the patch for 259890, which might also fix this -- can someone confirm that it does with tomorrow's nightly, and resolve this if so?
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.3) Gecko/20040925 Firefox/0.10 is still showing this bug! ftp://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/sweetlou-0.9/Firefox-win32.zip dated 9/25/04 2:35:00 PM
Oh, I wasn't doing the right steps to reproduce the bug. I can reproduce it with my PR1.0 build, but my aviary-1.0 nightly with my patch isn't failing to download - it downloads successfully and it's so fast I can't hit cancel.
confirmed, this bug is still present in 09/25 builds. :-(
Asa suggested this become a security-sensitive bug due to possible side-effects.
Tracy, can you try to figure out when this was first introduced and which products/releases it impacts?
This bug affects releases at least back to Firefox 0.8 I can't reproduce this on Mozilla builds. Tested 1.8a4 and 1.7 On Firefox 0.8 and 0.9 I didn't have to cancel the download. It was too fast to cancel anyhow. But the folder I selected to download to was overwritten by the downloaded testfile (it kept the Folder name). Mozilla builds installed the file into the selected Folder without data loss.
Comment on attachment 160553 [details] [diff] [review] patch r=shaver.
For the record, though I may be too late, I was able to reproduce this bug also on Linux with a debug-built Firefox.
The patch does fix this bug, but I get an exception running my testcase: *** exception in validateLeafName: [Exception... "Component returned failure code: 0x80520001 (NS_ERROR_FILE_UNRECOGNIZED_PATH) [nsILocalFile.append]" nsresult: "0x80520001 (NS_ERROR_FILE_UNRECOGNIZED_PATH)" location: "JS frame :: file:///home/ajvincent/mozilla/dist/bin/components/nsHelperAppDlg.js :: anonymous :: line 211" data: no]
fixed Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.3) Gecko/20040929 Firefox/0.10 as of a sweetlou build timestamped 2004092917 We pass the testcase, and I can file a separate (non-blocking-aviary) bug for fixing the download manager (or our help dialog) to get a filepicker so we save the file properly. Question, though: are there any other protocols which can force a pseudo-download? I'm leaving this bug as ASSIGNED, in case there are such protocols or resolving as FIXED would open this bug to the public. Until I am assured of such, I think I should not file the spinoff bug for getting the filepicker.
Verified fixed on Windows branch build 2004-09-30-08-0.9 fyi: I also tried some various scenarios with sending, saving and opening the file on the Thunderbird branch and had no data loss.
Given that this bug is fixed, as tested by myself and Tracy Walker, could we please re-expose this bug to the public? I'd like to be able to talk about it again.
Alex, we're hustling to get a Firefox update in place, both an xpi patch and a full 0.10.1 build. If all goes well, we'll be shipping that tomorrow. I'd like to keep this closed until then. Is that OK with you?
Fine. :) I'm just writing out some analysis of the bug's process now for posting to my blog.
Comment on attachment 160553 [details] [diff] [review] patch ok, so this patch cathes an exception and modifies the filename if it finds one. (I do think "unnamed" should be localizable, btw). but wouldn't a better fix be _not_ to recursively delete something that is supposed to be a single file? it seems to me like there must be a remove(true) somewhere that would better be a remove(false).
Isn't the recursive delete what you fixed in bug 259890? Or did that turn out to be unrelated?
dveditz: comment 18 said that the patch there did not fix this bug; so it seems unrelated
Isn't the cause of the problem more likely to be the Remove(PR_TRUE) here? http://lxr.mozilla.org/aviarybranch/source/toolkit/components/downloads/src/nsDownloadManager.cpp#566 That checks if the file exists, and if it does, recursively deletes it.
We would like to leave this bug security-sensitive for a couple of days after the patch release to give people a chance to protect themselve before leaking exploit details.
(In reply to comment #31) > but wouldn't a better fix be _not_ to recursively delete something that is > supposed to be a single file? it seems to me like there must be a remove(true) > somewhere that would better be a remove(false). ok, fixed by http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/toolkit/mozapps/downloads/content&command=DIFF_FRAMESET&file=downloads.js&rev1=1.28&rev2=1.29&root=/cvsroot (checked in as part of bug 262478)
Opening bug at reporter's request.
I filed bug 262949 per comment 31 of this bug, to see if we can get a better filename than "unnamed".