206319 - "Save Page As... Web Page, complete" saves files outside the _files directory

Status: VERIFIED FIXED

(Core Graveyard :: File Handling, defect)

Description

[Dan]

User-Agent:       Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.4b) Gecko/20030507
Build Identifier: Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.4b) Gecko/20030507

Two .gif files that the .html file references are saved in the same directory as
the .html file, not inside the _files folder.

Reproducible: Always

Steps to Reproduce:
1. Go to http://www.netmechanic.com/news/
2. File > Save Page As...
3. Choose "Web Page, complete"
4. Click the desktop icon on the left (or put in the Windows Desktop directory)
5. Click Save
Actual Results:  
Files/directories generated are...

"JavaScript tricks and more - Webmaster Tips Newsletter.html"
"JavaScript tricks and more - Webmaster Tips Newsletter_files" folder & contents
inside.
"_001.gif"
"_002.gif"

Expected Results:  
"_001.gif" and "_002.gif" should be in the _files folder.

The two files are adverts. If you load the page then you can see the two broken
image.

If you move the two .gif files into the _files directory and reload, the images
are okay.

My system is Windows ME but the option is not available.

Comment 1

[Dan]

Okay, Windows ME isn't available on the submit bug page but it is here, so I've
changed it...

Comment 2

[Bill Mason]

Verified 20030516 PC/WinXP
->Networking/File

Comment 3

[Bill Mason]

->Networking/File

Comment 4

[Jesiah S]

I also see this on LInux 2003051908

Comment 5

[Matthias Versen [:Matti]]

-> File Handling

Comment 6

[Boris Zbarsky [:bzbarsky]]

.

Comment 7

[Adam Lock]

The URLs in question that trigger this behaviour are like this:

http://ads.netmechanic.com/?AI=439&SN=4&FN=TruckSign.gif&RANDOM=2003.05.21.19.09.45
http://ads.netmechanic.com/?AI=595&SN=9&FN=Overture125.gif&RANDOM=2003.05.21.19.09.49

The persist object appears to be creating _001.gif & _002.gif files because it
can't produce a reasonable name from the URL. The wacky URL is also screwing up
how the name is appended to the local data path with the result it ends up going
outside the dir.

I would guess that MakeFilenameFromURI should always ensure that there is at
least one character in the filename rather than ignore it (see "Filename could
be empty at this point" comment).

A patch follows

Comment 8

[Adam Lock]

Attachment: Patch

Patch changes MakeFilenameFromURI to always have at least one char in the
filename.

Comment 9

[Adam Lock]

Comment on attachment 123935 [details] [diff] [review]
Patch

Requesting an r/sr on this simple fix. Ensuring filenames have at least one
character stops the nsILocalFile impl from getting confused later on.

Comment 10

[Kathleen :Brade]

Comment on attachment 123935 [details] [diff] [review]
Patch

r=brade

Comment 11

[jag (Peter Annema)]

Comment on attachment 123935 [details] [diff] [review]
Patch

Talked to Adam, sr=jag if a slightly more verbose comment is added for why
filename can't be empty.

Comment 12

[Adam Lock]

Comment on attachment 123935 [details] [diff] [review]
Patch

Requesting 1.4 approval for this small and obvious fix. It prevents strange
urls from dumping files outside of the data dir during persistence.

Comment 13

[locka]

Fix is checked in with a more meaningful comment.

Comment 14

[Chris Petersen]

Verified in the 2003-06-09-08 win32 trunk build.