Closed Bug 60203 Opened 23 years ago Closed 22 years ago

Downloaded files should not have garbage filenames

Categories

(Core Graveyard :: File Handling, defect, P3)

Product:
Core Graveyard
Component:
File Handling
Platform:
PowerPC
Mac System 9.x
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.8

People

(Reporter: mikepinkerton, Assigned: sdagley)

References

(Blocks 1 open bug,
URL
)

Details

(Whiteboard: patches attached)

Attachments

(3 files, 3 obsolete files)

Turn off salting of DL file names on Mac
1.22 KB, patch
Details | Diff | Splinter Review
Revised patch that un-salts file name after DL on all platforms
3.79 KB, patch
mikepinkerton
: review+
sfraser_bugs
: superreview+
Details | Diff | Splinter Review
Patch to put back deletion of temp files
1.21 KB, patch
mikepinkerton
: review+
Details | Diff | Splinter Review 
- 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.
Assignee: gagan → mscott
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.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → WONTFIX
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
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.
Status: REOPENED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → WONTFIX
ok, then we leave a file with a garbage filename on the user's machine. that's
not very nice at all.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
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
Keywords: mozilla0.9, nsmac2
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.

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.
Depends on: 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."
Attached patch proposed patch file (obsolete) — — Splinter Review 

    
    

    
  
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.
QA Contact: tever → benc

    
    

    
  
+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?

Keywords: qawanted

    
    

    
  
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?
Keywords: mozilla0.9, 
          
            nsmac2, 
          
            patch → mozilla0.9.2, 
          
            nsmac1, 
          
            rtm
Whiteboard: strongly desired for RTM

    
    

    
  
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.
Keywords: qawanted

    
    

    
  
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?
Keywords: mostfreq

    
    

    
  
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.
Keywords: mozilla0.9.2 → mozilla0.9.5

    
    

    
  
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.

    
  
Keywords: mozilla0.9.5 → mozilla1.0

    
  
Keywords: patch
Whiteboard: strongly desired for RTM → Has band-aid solution, untested by author

    
    

    
  
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.

    
  
Summary: Downloaded files have garbage filenames → Downloaded files should not have garbage filenames

    
    

    
  
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.

        Attachment #26516 -
        Attachment is obsolete: true

        Attachment #56228 -
        Attachment is obsolete: true

    
    

    
  
Taking bug, updating status, setting TFV

need r/sr

Assignee: mscott → sdagley
Status: REOPENED → NEW
Whiteboard: Has band-aid solution, untested by author → Has patch to turn off salting of DL file names on Mac
Target Milestone: --- → mozilla0.9.8

    
    

    
  
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.

    
  
Status: NEW → ASSIGNED
Whiteboard: Has patch to turn off salting of DL file names on Mac → patches attached

    
    

    
  
+#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.

        Attachment #63715 -
        Attachment is obsolete: true

    
    

    
  
> 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

        Attachment #64302 -
        Flags: review+

    
    

    
  
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

        Attachment #64302 -
        Flags: superreview+

    
    

    
  
Fix checked in

Status: ASSIGNED → RESOLVED
Closed: 23 years ago → 22 years ago
Resolution: --- → FIXED

    
    

    
  


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

        Attachment #64549 -
        Flags: review+

    
    

    
  
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. ***

    
  
Blocks: 129923

    
  
Component: Networking → File Handling
QA Contact: benc → petersen

    
    

    
  
Verified
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard

          You need to log in
          before you can comment on or make changes to this bug.