- Use the link above to d/l the file Expected: - file on desktop should be MacCVS_Pro_2.6d3.sit Actual: - filename is garbage, yet has correct extension (yesterday, it was wuvsdinr.sit, today it is n6tvl04v.sit)

When I download this file, it is given a temporary title while downloading but, when the download is complete, the file is saved as "MacCVS_Pro_2.6d3.sit" - this only started occuring within the past few weeks (see bug 55690 for the problem present when there is no temporary naming convention).

This seems to depend on how you download: if you save to disk, the archive is given the correct title once the download is complete. If you choose to open it with StuffIt Expander, the .SIT archive is left with the garbage filename.

yeah, you're right adam. that's what i see.

Is this at all related to all of the other "garbage filename" problems (password manager file saved as "4283607548.s", user data folder named "8wbptldq.slt", etc.)? Or a symptom of a larger problem (or intentional)? I was going to write up a separate bug for the problem where a garbage (8wbptldq.slt) folder is created (for no apparent reason) in your 'HD:Documents:Mozilla:Users50:Username:' directory, but four problems of the same type (albeit different occurances) seems a little much. (See also: bug 60542)

Adam, the second problem you mention is intentional and was done for security reasons. See bug 56002. The problem of this bug looks to be the same thing. See http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/nsExternalHelperAppService.cpp#697. It looks like if the file is supposed to be opened instead of saved, it gets the "salted" name. I would like to know how saving a file with the correct name is a security risk.

Assign to mscott who put in the name obfuscation code. That was checked in as a fix for bug 58774.

This is the intended behavior. We are spooling the helper app content into a temp file while we wait for the user to decide if they want to save or open (or cancel) the download. We cannot generate this temp file using a known file name because it introduces a security risk. Instead the name is "salted" producing the name you see today. As soon as the transfer is complete the file is renamed to the name the user specified anyway.

actually, in the case where they are passed to a helper app, they are not correctly renamed.

right because we don't ask the user for a name for this temp file. We are handing a temp file to the helper app. That's by design.

ok, then we leave a file with a garbage filename on the user's machine. that's not very nice at all.

Would someone please explain the "security risk" associated with downloading to a file of known name? No other browser or internet utility does this.

This bug causes problems in 9.04. The randomness of the filename is not enough that it changes the extension away from something known on the computer. It appears that the extension part is picked once and then kept forever. On my system the files land as randompart.pqa .pqa is associated with palm desktop and so the files become type **** creator Gld1 When they should become QuicktimeInstaller.sit On another machine they are assigned .wnd and attributed to Interarchy.

Just discovered there is a difference in behavior based on the type of connection. If the download is an ftp call it is fine. The temporary file has the correct extension and gets properly renamed but the QuicktimeInstall.sit download is a redirect to and http style transfer. Also discovered the entering the mimetype entry for sit in the browseer make the file land as a .sit but it will not automatically kick off stuffit expander.

nominate for mozilla 0.9

There's a lot of interesting discussion in this bug. Could someone summarize what we currently think is broken (if anything?) Seems like the summary is out of date too.

The bug is obvious -- you click on a download link for a file like Foopy.sit.hqx, and end up with a file on disk with a totally different name (sometimes even with a different file extension). This makes tracking downloaded files confusing, and can break mac file extension -> type mapping). It was done for supposed security reasons, but no-one has given a strong argument as to why it is necessary.

cc'ing mstoltz because he may be able to answer the security risk question. Mitch?

The reason for the random filenames is the same as the reason given in 56002, but I'll restate it. In general, it is a bad idea to allow an attacker a way to place content of the attacker's choosing at a known location on the user's hard drive. Combined with a way to cause that file to be parsed or run by the browser, this allows executing code (JS, perhaps) with the privileges of the local disk, rather than of the attacker's page. This is the reason why files in the disk cache are given names in numerical sequence, not the name of the cached document. I'm not clear on what the problem is here, but I don't think it's the randomization of the name. Is it that the temp files are left on disk after unstuffing or whatever? If the file is meant to stick around permanently, the name should not be mangled, but if it's meant to be deleted, it should be randomized. Please don't remove the filename randomization; I'm sure this problem can be cleared up through a combination of better implementation and maybe some user education.

I understand what the security risk is behind this, however, if the risk is so great, why does it not appear that other browsers do this?

On my powerbook under 904 the problem is that downloads function differently if the stream is http vs if the stream is ftp. I will re state. if it is http the file name is garbled and the extension is one that is registered with the system. NOT the extension of the file that is being downloaded. In the case of QuicktimeInstaller.sit it is named somerandomstring.pqa On another machine here it lands as randomstring.wnd If the transfer is ftp the same file is named randomstring.sit until is is all present then the name becomes QuicktimeInstaller.sit and the install proceeds normally. If I define mimetypes for .sit, .hqx and .bin the file starts out as randomstring.sit, becomes QuicktimeInstaller.sit when all present and then stops. N6 does not kick off stuffit expander even though the mimetype has it defined. This is a real pain in the @**. At this point about 2% of all downloads of quicktime 5 preview 2 are coming from netscape6 browsers on Macs. Our customers are not being served by this thing and we have put up a warning on http:// www.apple.com/quicktime/preview.

That's certainly not good if Mozilla users have trouble downloading files... It seems to me that a randomly named file being left on the desktop (either before or after install) might be confusing to novice users... Also, what security is being achieved if only http downloads are salted?

There are at least two problems that cause the problem as lloyd describes when trying to download QuickTime 5. Neither have to do with a salted name. (1) We don't have any MIME type mapping info for application/x-macbinary by default and none is gotten from IC. (2) After adding one myself in the helper apps pref panel, it still didn't work because my stuffit expander has a trademark symbol in it and, of course, that was botched - both in the dialog and when making the file. This caused nsLocalFileMac::LaunchAppWithDoc to fail miserably. If that file was initialized from the string in the dialog, grrrrr

We are using IC to fill in the nsIMIMEInfo. Problem is, for that MIME type, on my system, IC comes up with rubbish for that type. The file type is wrong, there is no post handler app, and worse, its MIME type field is blank. At http://lxr.mozilla.org/seamonkey/source/xpfe/appshell/src/nsInternetConfigService.cpp#208, we fill in the MIME type from what comes back in the ICMapEntry. In this case, the empty string causes a js exception in helperAppLauncher.js. This causes the dialog to never come up. Because of this, nsExternalAppHandler never gets the disposition info, nothing is done with the downloaded file, and we end up with the file still with the garbage name and the wrong file type. I think that nsIInternetConfigService::FillInMIMEInfo, when given a valid MIME type, should not allow the MIME type in what it fills in to become blank. There are several things wrong here. Separate, more specific bugs should be filed.

Back to the salted name issue: The problem with saving the download to a temporary file with a salted name is that the user cannot easily associate the local file with the download, while the download is in progress. We rename the temp file to the real name after the download is complete (doesn't this defeat the whole purpose of salting the name anyway?), but this does not help for long downloads where the user is seeing the salted name for a long time. If we really do use salted names, then we should save download-in-progress files, with salted names, into the Temporary Items folder, so that the user never sees them. Then when the download is complete, we move the file to the final destination with the correct name.

I was able to download and install QT5 on both of my Macs at home. Both have system 9.04. I did not see the behaviour you mentioned (other than the temp filename). Netscape 6 offered me Stuffit as a helper app. What could be different about my machines?

What could be different? The Internet Preferences file, or whatever file IC uses to keep MIME type info.

I agree with Simon that a temporary files folder is the way to go...the only problem is that the file must associate with the program to open/unstuff it every time, or else the user will think the download has failed for no reason.

A second problem with the Temporary Items approach is that the user won't see their download file until the download completes. They will get very confused, wondering where the file is.

The discussion about moving to a temporary file location should be a separate bug. This bug appears to be solely about HTTP not successfully renaming the file at the end of the download. Comments indicate that IC could be the root of the problem by returning incorrect information. We could fix that in our code with some bullet proofing. Can someone attach the patch with the bullet proofing?

No, this bug is about the salting of downloaded filenames. The HTTP download problem should be a separate bug.

It's filed as bug 62939.

Well, discussion in this bug has stalled. Let me start it again: How about if we turn off salting of the names of downloaded files? :)

Well, there's a tradeoff here. The security benefit of salted filenames exists in those cases where a file will be saved to a well-known location and stay there indefinitely. If this is the case, an attacker could place arbitrary content in a known location, which is a good opening for attacks. We need to weigh that against the confusion these salted names are causing. Can we change how we're doing this, instead of eliminating the feature?

Simon wrote back on 12/14 that if we use salted names for the temp file, then we should be storing the temp file in the Temporary Items Folder on the Mac. This is actually where the file is stored for windows and Linux (in appropriate temporary directories). At the time I wrote the code someone ( I don't rememeber who now) told me on the Mac I should attempt to put the file in the mac default download directory. So I actually have a chunk of XP_MAC specific code to do this. Would folks be happier on the Mac if I ripped out that code and we put it in the temporary directory like we do on Windows and Linux? I'd be more than happy to remove that code.

That might be nice...

Um, no. Where you want to put DL'd files is in the folder the user has specified for downloads in Internet Config (aka the Internet control panel). If you put them in the Temporary Items folder the user will never see them as that folder is normally invisible.

That's a good point. What are the odds that a file is going to be named exactly the same thing, and stay in the same download location (the desktop, by default). Since most download files are either stuffed or installers anyway...

And let me add my plea for removing salting in general, at least on the Mac. Consider the following scenario which totally voids any security advantage to salting the downloaded file name: 1) User clicks on a link to a DL named MyNastyVirus.sit which is a StuffIt archive containing the application/file MyNastyVirus. 2) We convert the downloaded file name to FEJKFJGHE.sit and save the file somewhere on disk. 3) We call StuffIt Expander to process the file (after all, it's a .sit file) 4) StuffIt Expander expands the archive and leaves the application/file MyNastyVirus in the same folder as FEJKFJGHE.sit. The user now has no way to associate FEJKFJGHE.sit with MyNastyVirus but the latter is still in a potentially well know location on the user's drive. Did we really do anything to protect them by salting the DL file name?

That's completly true, a salted filename that only applies to the .sit file is useless as a security measure. Somebody just de-salt the filenames, please...

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

Lots of discussion about the random temporary file name. Seems to me the filename should be randomized but keep the extension of the actual file. randomizing the extension causes Macos file exchange extension to assign an file type and create a resource fork based on the file extension because the file comes down without one and The extension thinks it is a Mircosoft style file.

Maybe Steve is right; this may be more trouble than benefit. Scott, what do you think the possible attack scenario is in this case without the salted filenames?

First of all, the file name extension is preserved in the salting. If there's a particular case were it isn't preserved then let's take a look at that. I just had a discussion with steve dagley last night where I explained to him why this salting of the name is necessary in 6.0. In 4.x, when you went to download content that was going to be launched using a helper app (or saved to disk), we would bring up a dialog asking you if you wanted to save it or open it using a particular application. Once you decided, we would then start downloading the content. So the user has a chance to acknowledge that they want the content and to determine how they want to dispose of the content. That is not the behavior in 6.0. We've actually improved this quite a bit. Now we immediately start to download the content to a temporary file. WHILE the content is being downloaded we then bring up a dialog asking the user if they want to open this using an application (like Stuffit expander) or save the content to disk. Now while the user makes up their mind we are hard at work downloading the file. The file name needs to be salted because the user has yet to acknowledge that they even want this content. By not salting the name, an attacker could place a file in a known location with a known name without the user's knowledge or permission. Once the user dismisses the dialog that comes up it would then be safe to use a non salted name because they have now acknowledged that they want the content. In the case of save to disk, we rename the salted file name to a meaningful name the user choses. In the case of open using an application we don't rename the file and we just launch the app passing in this file. Steve and I were talking and I threw out the following possibility: before launching the external application, we can rename the file from the salted name to a name we can extract from the url. That way we won't leave a salted file name in the case of opening using an application after the download is completed. We'll rename it. Sound good?

Excellent!

So why did we decide, in 6.0, to start downloading before the user acknowledges that they actually want this download? I don't like this behaviour at all. Also, it presents the problem that if the user chooses to continue the download, but saved to a file in a different location, we have to somehow move a file which is in the progress of being downloaded to, or fall back on the less user-friendly alternative of only moving the file to teh user's selected download location when the download is done. If salting is a necessary side-effect of the 'background downloading' feature, then I say turn this feature off.

Starting the download before the user has acknowledged it is confusing. Here's what happened to me until I knew it was doing this. I know how long it takes my system to download a file of a certain size. Say I take a few moments to decide where to put the file. I finally confirm in the dialog and than nothing appears to happen! While deciding where to put it, it was already downloaded without any indication of this. I'm not so against using idle CPU cycles to pull in the file while I make up my mind but at least some indication that the download is happening would be good. Moving the file after its in would be wasteful when we spool it to one drive and the user chooses to save it to another.

That threw me for a loop too. I thought, "how cpuld that have downloaded so fast?" I think this is a great feature, and we should keep it, but maybe there should be some indication that it's happening. I agree that if we do this pre-downloading, we've got to salt it.

we can throw up the progress dialog, show 100% then take it down after a second or two in the case where we've finished the download by the time the user dimisses the open / save as dialog.

I'd be happy with what mscott suggests. Is it possible to show indeterminate progress during this time?

*sigh*. Did we do any usability testing on the background download thing?

Is it possible to rename the file to its "proper" name immediately after the user clicks "Save to Disk" or "Launch with Helper App", instead of doing so after the file completes downloading? This seems far more intuitive to me, but I don't know if the Mac filesystem allows it.

So, did we make any changes to resolve this bug?

Would it be impossible to only partially salt the name by adding either a prefix or pre-extension suffix (so you end up with something like "8BC56F_netscape6.bin" or "netscape6_8BC56F.bin", etc.)? This would provide the (limited) security necessary to prevent most malicious behavior, and would also leave the user with a recognizable filename (instead of something like "OU812.bin"). The best of both worlds. BTW, starting a download before the user selects a location or filename (bad behavior, IMO), is covered in bug 55690.

FWIW: changing the filename/pathname to something random is totally annoying (I always hate this "feature" of IE when I am forced to use my Windows box) -- its nice to be able to either view or even 'mv' the file while its being downloaded, plus you can see the file size changing from a telnet session (if you are downloading something really big for example). I'm not sure what the security implications are (but the start auto download sounds particularly lame -- I don't want that to happen at all under any circumstances) but rewritting the filename and doing an 'mv' or 'cp' at the end is totally wacked, IMO.

can't we just put parens around the file while we're downloading, or something? such as (MacCVS_Pro_2.6d3).sit for the example url above.

In terms of security, that kind of static naming convention wouldn't be any different than not salting the name at all, would it? Once the theoretical attacker knows that Mozilla places parantheses around filenames, they'd be able to work their magic, no? FWIW, I'm not convinced that these "security" measures are really warranted - you can't protect every single user from every single type of attack. Unsalting the filenames and eliminating this auto-download "feature" gets rid of the need to spool downloads into a temp file and move it once finished, and will help reduce user confusion.

I think the background downloading can be made transparent to the user... just download the salted filename to the temporary items folder, and as soon as the user picks a filename and path, then copy the file there immediately. The only problem I can see is that moving files whilst they're still downloading can be a bit dangerous, cause filesystem corruption, etc. I'm working on a patch to bring the current behaviour to match what I've described, but I can't make any promises... I'm just a second-year software engineering student, it's not like I'm any good with this stuff... (I'll have to look further into the Moz routines to see whether they actually move the file, or just make a copy in the new location and erase the original. My initial testing seems to indicate the former.)

The problem I have with that and other proposed patches/behaviors is that it's all so complicated, especially for something as simple as downloading a file. The best way to fix this is to eliminate the behavior described in bug 55690. That way, you get rid of the need to salt filenames, create temporary "spool" files, create temporary progress dialogs, etc. Mike said it best in 55690 - we need to get rid of auto-downloads, rather than bend over backward to accomodate this dubious "feature."

A few things which bear saying: This is my first patch to Mozilla, so go easy on me. In addition, I'm just starting my second year of my software engineering degree... most other kids in my class are starting to learn to code in C. So go easy on me :-) At least initially, I'm not looking to get this checked in to the tree, I'm mostly looking for feedback. It's not a big diff, all I've done is changed some logic around. Even though this is a Mac bug, the code I'm touching is XP. Now I don't have easy access to build environments which aren't Mac, so I'm hoping that someone else build it and hold my hand through this (and most of the rest of the process). From my initial testing, it works well on Mac. Now, with this code I'm moving a file whilst it's in the middle of downloading. This doesn't seem like a very healthy exercise to me, but my cursory inspection of the code shows that it should be OK. But I haven't tested on many platforms. And I don't have deep knowledge of the Mozilla file handling code. So feedback is welcome. Finally, I think the behaviour I've coded is how it "should be"... the salted filenames are hidden in Temporary Items and the user should never see them. All they should notice is that the file seems to get a headstart when they start downloading. Which is as it should be. (On other platforms, I've set it to download to the home directory when set to open with a helper app... is this the right place? Or should they go to the temp directory?) Testing this: 1. Apply the patch and build up. 2. Click a downloadable file. Wait half a minute for it to start downloading in the background. Click Save to Disk, pick a spot and save it. When the progress dialog shows up, it should show the file as partially downloaded already... and the partial download file will be where you saved it (try a Get Info on it). 3. Click a downloadable file. Wait for ten minutes, or however long it would take to download completely. Click Save to Disk, and save it. The progress dialog should show that the download is completed, and it should be where you saved it. 4. Click a downloadable file. Wait half a minute for it to start downloading in the background. Click Open Using, pick an app if you like, and go. It should show the file as partially downloaded, and the partial download should be in the Internet Config downloads folder. When the download finishes, the helper app should open. 5. Click a downloadable file. Wait for ten minutes. Click Open Using, and go. The helper app should fire immediately. Those who can, should check the Temporary Items folder before either Save as File or Open Using. BTW, the testcase URL given doesn't work. It's nearing midnight and I'm much too tired at the moment to find an alternate URL.

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

Could some people please try applying this patch and telling me what they think of it? I really hope I haven't broken anything. Also I've uninstalled my build environment temporarily (to make room for OS X), so I'm out of the code loop now. If anyone wants to claim this patch to tweak for themselves, please feel free to.

> // At this point, we move the file to the final destination, whether it's > // completed its downloading or not. If not, it'll just continue to download > // to its final location. > rv = MoveFile(mFinalFileDestination); While this works on the Mac, does anybody know for sure how safe this is on other platforms?

Speaking for linux, I don't want any temporary files with garbled names in my home directory, not even for a few seconds. I think the OS provides a function for this ("man tmpnam 3" gives "tmpnam - create a name for a temporary file"). Usually this will give you cryptic filenames like /tmp/fileHC8iNF , so applications do not need to worry what names to give for temporary files, or in which directory to store them. If you need your file to have a certain extension you can just append it to the string returned from tmpnam. However, linux or unix systems often have their temporary directory mounted to its own partition. While it may make sense to lauch a helper app on a tmp file, the user usually won't want to save downloaded files to that directory. So it may even be the default case that the target path is on a different partition. It may be possible to move a file to a different disk partition while it is open and written to, since the "mv" command did not complain in my test, but it seems that everything written to it after the move is lost. So it may be non-trivial to implement this move-while-downloading behaviour on linux, but I'm not an expert here. I wouldn't have expected this downloading-in-advance behaviour, but I don't have any problems with it so far. (Not that I'm using this feature a lot...) Also, I don't have a slow connection, and it _is_ a pleasure for me to see that a download completes a bit faster. I don't know how MoveFile works, but NS_ASSERTION(mStopRequestIssued, "uhoh, how did we get here if we aren't done getting data?"); doesn't seem to suggest that this routine was designed for moving files which are still being written to... On unix, nsLocalFile::MoveTo uses copy/delete for "cross-device" moves, so this should not work when the old file is still being written to.

Conrad, why do you think it works on Mac? Shouldn't have Macs the same problem with cross-volume moves? nsLocalFile::MoveTo uses nsLocalFile::MoveCopy on Mac: http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsLocalFileMac.cpp#1408 1463 // On a different Volume so go for Copy and then delete 1464 rv = CopyTo( newParentDir, newName ); 1465 if ( NS_FAILED ( rv ) ) 1466 return rv; 1467 return Delete( PR_TRUE );

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

Andreas: you're absolutely right with regard to cross-device file moves, and I can anticipate that being a problem. I'm getting my Mac build environment back up... I'll have a deeper look at the Mozilla file code and find out what I can... a possible solution is to 'pause' the download whilst the transfer takes place, but that is another can of worms entirely. We will see how deep the rabbit hole goes. However, keep in mind that whilst downloading files in general, using Mozilla or some other download client, people might move the files around anyway... possibly not onto a different disk (in Mac OS, moving files to different disks will not delete the original), but maybe to somewhere else in the directory tree. It's worth finding out what happens in the case of a Mozilla downloading file... when moving it around in the Finder or on the command line, it's all in the hands of the filesystem. Another problem I ran into (admittedly nothing to do with this bug) is what happens when you have low disk space in your /tmp directory (or Hard Disk:Temporary Items). On the Linux box I was on at the time, Mozilla just kept downloading even though it was out of space. Maybe I'll file another bug in due course.

mass move, v2. qa to me.

+qawanted, no test case for fix. MacOS should resolve the HFS paths (and whatever newer file system they have) to a file ID number (or some logical file reference, boy am I rusty on MacOS internals...) Anyhow, its like most OS's, as long as you don't move off the volume, you can keep renaming the file b/c that is just a path to the file ID. Can someone summarize? Are we going to fix this for a certain milestone? What is the expected, fixed behavior?

we seriously need this for rtm, and i don't think we want to take the current patch (we don't want to move while downloading, we want to not download). why is this untargetted?

Well, whether we should try to 'fix' this behaviour or just turn it off is a point of debate. But I do agree that we should stay from the patch for the moment :-) Like many other big, long-running bugs, this one's evolved quite a bit. As I see it, this could be better split into two bugs: 1) Downloaded files which are processed by helper apps (such as Stuffit Expander) have garbage filenames after the download (Mac, maybe all platforms?) 2) Files which are in the process of downloading have garbage filenames, even though the user has picked a place to save them (all platforms) If you strip out the relevant bits, my patch will still address point 1. Point 2 will require a bit more work.

> +qawanted, no test case for fix. Test case: 1. Go to <http://news.bbc.co.uk/>. 2. Click the `BBC News 24 bulletin' link on the right. 3. Click OK in the `Do you want me to do what I'm supposed to do, or do you want me to do something else' dialog. 4. Watch some or all of the video in RealPlayer. 5. Repeat steps 2 to 4 with the `World news summary' link. 6. Now (pretend it is an hour later) repeat steps 2 to 4 with the `BBC News 24 bulletin' link again. 7. Quit Mozilla. 8. Try to listen to the world news summary again, by double-clicking on the appropriate file. What should happen: * The files on your desktop are named `n24.ram', `n24 2.ram', and `summary.ram'. * You can tell that `summary.ram' is the one you want. What actually happens: * The files on your desktop are named `luxh1pj2.ram', `xxlghxfn.ram', and `ozels9vb.ram'. * You can't tell which file is the one you want.

mscott, can you fix the component and qa? I'm getting out of the download naming features bugs, aren't they xp apps?

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

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

Hey all, it's been quite some time since anyone's kicked this up to the front burner, but the bug is definately still here. Has anyone make any progress or have any thoughts on how this can be resolved? I just downloaded the OS X 0.9.2 and the file was called "v05by6pl.bin", which is pretty confusing. Thanks.

Over the last few days I've begun to build Mozilla again. I'm fairly sure the patches have bitrotted a fair bit, not that anyone should be using 'em anyway. But I will be looking into the issue when I get the time. I've noticed that you can pause and resume the download (not sure if recent builds have this feature). I'll examine how that works more closely, but I reckon it could be a godsend because when you pick a spot to download, we could programatically pause the download, copy the file to the right spot, delete the temp file, point the program to the new file, and then resume downloading. But this is all speculation as I haven't actually seen the code yet.

What is the status of any possible fix? Salted filenames are still not renamed before launching helper apps in Milestone 0.9.3. This breaks all file-name dependant helper apps, such as automatic unzip tools (especially for .tar.gz which loses the .tar) and WinAmp skin downloads on Windows (filename becomes skin title upon automatic installation). Also, this is not just Mac platform but all OSes with helper apps; please mark as such.

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

+mostfreq. I'll try to clean up this bug later this week. Quick check, is everyone here mac-only?

I think that the bug is cross-platform, however I'd guess that Mac users are finicky and are more likely to complain about an issue like this. I've got a basic patch going that changes the "salted" filename back to its real name in the case of launching with a helper app... I'm still grappling with the file and stream classes to address the other issue of renaming/ moving the download file on-the-fly. If someone else wants to take a crack at it, by all means do so.

Suggesting new milestone target.

CC'ing myself and putting this back on somebody's radar. This is an annoying bug which shouldn't be in a shipping product IMO. Download 10 things and go to your download location, then try to figure out if that file you want to unstuff is g8sllee9, or 7ytslw21.

This bug has nothing to do with Mac users being "finicky" and everything to do with the need to properly engineer such things in Mozilla rather than shipping hacked-up stuff.

I need to know the scope for this bug. Is it Mac-only? Or does it affect all platforms (having a messed up name during downloading and maybe afterwards too)? Or does it affect all platforms, but only the Mac users care? The target platform is Mac but I have suspicious that this is cross-platform code, and I don't want everybody breathing down my neck. I don't have easy access to other platforms to test this on, you see. Also, is this bug about a) having a messed up filename during file download, where it should be converted to a proper filename once you've picked a spot for it, or b) having a messed up filename after a download because it's been handed off to a helper app? My patch addresses b), but the fact that b) occurs is just a symptom of a), which I'm not sure is a big deal.

I am dead certain that this bug is ALL-platforms. It was originally reported as Mac because that is where the most obvious symptom is (automatic unpacking leaves garbage filenames in the downloads folder). But it also affects WinAmp skin packs, WinZip .tar.gz handling, and any other helper application that relies on filenames. Files that are being passed to helper applications should be renamed before the helper is called. The security concern over a known file location is valid, but better ways to resolve that would be to have a preference for disabling background streaming, or placing the files into randomly named folders. Too many applications rely on having the base filename intact.

Attached is a five-minute band-aid patch, with the following qualifications: 1) I haven't even tested it! My build system isn't working at the moment. So the patch may not build, it may trash your system, etc. I'm posting it in the hope that somebody will test it for me! 2) This only solves problem b) above, that is, any files which go to helper apps will now have the proper filename. This does not solve a), that is, whilst they're downloading, they'll still have garbage filenames.

Yea. I have used mozilla across multiple os's (mac classic, mac os x, linux and windows). On all os's the files are hashed inside of a tmp directory (for linux /tmp, windows C:/WINDOWS/TEMP (or something like that)). At least PART of the problem (as I see it) is that macos classic [and old versions of os x?] don't have any kind of "temp" directory. So instead, the hashed filenames are saved where the file's end result is going to be. So for example, your download directory hd:download. The file lives in there the whole time, and is just renamed while on linux (and I presume win) actually moved (and renamed in the process) from the temp directory. Anyway - It is NOT a mac only problem - its just obvious on the mac ;) Specifically when using stuffit expander for windows will it expand the file in the temp directory. Although if you don't know where it is (seems random sometimes) you wouldn't be able to find it because its hashed. So it also wastes some space. Internet Explorer for windows also does this (in some non-standard place- wherever it is). Also used for windows media files - perhaps as some kind of media protection. If you do "save target as" it will manually save it where it should be. I have also had problems with the hashed file names when downloading large files (specifically those new redhat .iso files). It originally saves it in / tmp and then moves it to /export/share. It can be a BIG problem if you have a small "/" partition and a big export (which is common setup). So /tmp would get full, the download dies, and mozilla sits there. Then you have to go and dig through /tmp and guess which file is which.. So in the end - I feel its kind of pointless. True it can be more "secure" by having a random file name but in environments where that would be required, the disks should already be secure by permissions(and then some). This could then in the end be more insecure for the file exists in / tmp and is at least viewable (haven't tried reading/writing it yet). In theory, someone could see the file and edit it as you download and change the contents. This would be worse then just saving it where its going to in the first place. I encourage that the "feature" just be removed altogether. It just causes headaches. Specifically between different environments (and mozilla is multi-platform). Also is a bit redundant and there are (i'm sure) some bigger security bugs then something as insignificant as the name of the file. I could see it MAYBE being put back in,(for those who are paranoid) but ONLY after mozilla has hit 1.0 - where most of the code has calmed down and it could then be properly be put it. so the point it - Just remove the hasing/salt/randomfilename/etc from mozilla for saving file. Sorry for going a bit long and making it semi-rant. Thanks, Ian Sidle

cc'ing myself I agree that the "download in advance" functionality should be removed altogether, or at least made a preference (although the latter seems more trouble than it's worth). I would much rather have my files always named properly than save 5 seconds of downloading time. Is this going to be fixed? What's the delay? Is there anyone who still believes that downloading the background to a temp file is the way to go? Oh, and the "mozilla0.9.5" keyword would appear to be out of date.

Touché. If anybody wants a "proper" solution, here's what I suggest. (It's probably beyond my capacity to code, so if you know the Mozilla file code, please have a shot.) 1. The user chooses to download a file. The file automatically starts downloading, using a garbage name. 2. The user chooses save-to-disk or open-with-helper. If save-to-disk, the user picks a spot to save the file... else if open-with-app, the system uses the default download folder. 3. The system temporarily stops writing to the garbage file, moves it to the final destination and renames it, and continues writing. This may actually involve a new logical file, if the garbage file and the final destination are on different volumes. 4. When the file is done downloading, it's opened with the helper app if that was specified. Step 3 is the missing link. That was what my first patch attempted to fix, only it didn't take into consideration a new logical file, so it wouldn't have worked. Result: files download faster, downloaded files are named correctly, user can inspect the file whilst it's downloading, garbage file is completely invisible to the user. Other possible solutions: A. Implement my band-aid patch. Whilst downloading, garbage files are visible to user, but when finished, they are given the right filename. B. Disable the feature altogether. File always has the right filename, file doesn't download any faster because it doesn't get a headstart.

Caveat to step 1: The "garbage-filename" placeholder file should most certainly be stored in the Mac OS invisible Temporary Items directory. Unix people may have issues with available partition space and such nonsense, but we don't.

The implementation that has not been considered is to make a "Download" directory within the profile. This would allow the files to inherit the salted profile dir path, obviating the need for a salted file name.

The DL dir in the Profile dir wouldn't fly on the Mac as there are OS settings to specify the DL dir. Personally I think we should scrap the 'feature' of starting the DL before presenting the user with any UI as I think the user complaints about mysterious/unrecognizable file names far outweighs any kudos from someone that has noticed any improvment in DL times.

Under the OS X build (2001-12-03-08) build, I see the filename corruption only when selecting a helper app to display the file. For example, using Reader or Preview apps to display a pdf file will show a corrupt filename. If I choose "Save to disk" option, the proper filename remains intact.

Chris, I refer you to mscott's comment <http://bugzilla.mozilla.org/ show_bug.cgi?id=60203#c43>

My 0.02. Every other FTP program and Browser on the Mac can get this done correctly. If it means turning off this "let's begin the download before the user has decided where to put the file" garbage, then so be it. It is really embarassing that a shipping product would have such a gaping usability hole in it. Having randomly named files downloaded is far worse than waiting a few secs before starting a download. If I'm downloading britney_xxx.jpg, then for damned sure the file should be called that when I go look in my downlaod folder :) Where's the benefit in the current implementation? If the user is on a slow connection then, they are used to waiting forever to get their file and a few seconds won't make a difference. If you're on a T3, then it'll get to you soon enough either way. Can this "feature" be easily turned off for macs only? If so, then please, please, please do it. Sorry for the rant, but as an end-user and not a programmer I'm getting really sick of this bug.

Hear, hear! This has been an issue for over a year now. This bug needs to be fixed, and the quickest way to do it is by turning the pre-downloading off entirely. No reason to waste any more time.

Just for my 2 cents worth, it takes longer to find the randomised file and figure out what it is than the pre downloading takes. If I come across a file with a randomised name, it takes that long to figure out what it is rather than saving 3 seconds downloading before hand.

Actually it's faster to stop salting DL file names. Here's a patch that does that for the Mac build.

Taking bug, updating status, setting TFV need r/sr

Despite obvious Mac issues with StuffIt, etc., this is an all-platform problem. Filename salting affects any helper application which relies on an intact filename, such as WinZip's FILENAME.TAR.GZ handling and WinAmp's SkinName.wsz skin download convention. Please consider the more correct (and cross-platform) solution, which would be renaming salted files immediately on completion of download, before executing any helper app.

I don't see anything wrong with my band-aid patch, from a logical point of view (it probably won't build, since I have no way of testing it... the switch to CW Pro 7 left me behind). It's here, right now. I think it's the better solution. Someone take a look at it. It renames the file at the END of the download process. The only drawback is that during the download, Mac users have to stare at the salted filename. After the download is finished, all is fine. We keep the quicker start and the security benefits.

Since the default DL location on the Mac is the desktop I think the appearance of the salted file name is confusing immediately so I prefer naming it correctly to begin with. If there's a lot of pushback on that I'd be willing to try Terence's approach. Note to Terence - when submitting a patch use a Unified diff with a few lines of context so that the site of your change is obvious and it has a chance of being applied to a version of the file other than the one it was originally diff'ed from.

An alternate patch based on Terence Tan's un-tested bandaid patch to rename the salted DL file name to the original name after the DL completes and before passing it off to a helper app. Changes from Terence's original code are: 1) Now tested :-) (and a Unified diff based on the current nsExternalHelperAppService.cpp so it is easier to review/apply) 2) Handles the case of a file with the same name as the desired un-salted name already existing. 3) Conditionalized for the Mac build only via #ifdef XP_MAC. I'm willing to fight to get this into the Mac builds now but I leave it up to people more familiar with other platforms to decide if they want the same change. Why they wouldn't I don't know. 4) Not changed from Terence's original but note that this version of the patch incorporates the fix for 100460 - don't delete the temp file when Mozilla quits. Also note that this patch basically implements the solution agreed to by mscott in his comment of <http://bugzilla.mozilla.org/show_bug.cgi?id=60203#c43>. I'll check with him and if he was intending that to be an XP change will address #3 before checking in.

+#ifdef XP_MAC + // Make sure the suggested name is unique since in this case we don't + // have a file name that was guaranteed to be unique by going through + // the File Save dialog + mFinalFileDestination->CreateUnique(nsIFile::NORMAL_FILE_TYPE, 0644); + rv = MoveFile(mFinalFileDestination); +#endif rv = OpenWithApplication(nsnull); You've lost the rv from the MoveFile here, and that's something that may well fail (e.g. out of disk space).

Address sfraser's comment on handling any error from MoveFile(). Since the source and dest dirs are the same for the Mac we shouldn't actually be doing any file copying - it should just result in an in place rename. This patch also takes the same approach for other platforms and is no longer just limited to Mac builds via #ifdef XP_MAC. Note that this patch does expose bug #56295 - nsLocalFileMac doesn't work for file names >31 characters - which ccarlen plans to fix today.

> Address sfraser's comment on handling any error from MoveFile(). Since the > source and dest dirs are the same for the Mac we shouldn't actually be doing > any file copying - it should just result in an in place rename. Do we need to ensure that the filename is unique?

The patch does make sure the file name is unique but only when it's ready to do the rename otherwise you'd have 2 files showing up while the DL is in progress (MakeUnique() actually creates a file rather than just a unique name). If you were questioning if we really needed needed to create a unique name, I say yes. I don't think it's a good idea to replace an existing file if we're downloading one with the same name.

Comment on attachment 64302 [details] [diff] [review] Revised patch that un-salts file name after DL on all platforms r=pink

pinkerton noticed that MoveFile() never in fact would return an error of NS_ERROR_FILE_ALREADY_EXISTS since it first deletes the dest file if it exists. I'll clean up the comment and test of the MoveFile() result code before landing the patch.

Comment on attachment 64302 [details] [diff] [review] Revised patch that un-salts file name after DL on all platforms sr=sfraser

Fix checked in

Per mscott's request I've put back the code to delete the temp files when Mozilla exits on non-Mac platforms

I'm new to this bug, but why did that go back in on non-Mac platforms? I've repeatedly run into the same problem on Windows that Mac users complained about. I download a large .zip file, but select to open it in WinZip instead of saving it to disk. If I happen to close all other Moz windows while it's still downloading, then the file's deleted as soon as the download completes and WinZip can't find it. A more obvious case is if I exit Mozilla _at_all_ while the temp file's open in WinZip, WinZip says the file's been deleted and barfs.

Because the module owner requested it be changed back to the behavior of Netscape 4.x for non-Mac platforms. I _know_ Mac users didn't like that behavior so I didn't consider the 4.x behavior 'correct'. For other platforms I leave that decision to folks more familiar with those platforms. In this case, the module owner. I'm sure he can be lobbied to a different point of view :-)

I don't have it installed anymore, but I'm sure I didn't run into the problem I just described with 4.x. In other words, the temp file was sticking around after 4.x exited.

That would be weird since the behavior in Mozilla/N6 is modeled on 4.x and I can assure you that Mac 4.x deleted temp files on app exit. And AFAIK that was the norm on other platforms for 4.x as well.

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

Verified