Closed Bug 393488 Opened 17 years ago Closed 17 years ago

Special characters in filename block the download function

Categories

(Toolkit :: Downloads API, defect, P2)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9

People

(Reporter: goowhen, Assigned: smontagu)

References

Details

(Keywords: intl)

Attachments

(4 files, 1 obsolete file)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.6) Gecko/20070725 Firefox/2.0.0.6 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.6) Gecko/20070725 Firefox/2.0.0.6 The file download function did not work no more since I try to download a file whose filename is "A9�(8)2.pdf". The error console shows : Error: not well-formed Source File: file:///C:/Documents%20and%20Settings/jopo/Application%20Data/Mozilla/Firefox/Profiles/pttxa21n.default/downloads.rdf Line: 54, Column: 67 Source Code: <RDF:Description RDF:about="C:\DOCUME~1\jopo\LOCALS~1\Temp\A9�(8)2.pdf" ------------------------------------------------------------------^ Reproducible: Couldn't Reproduce Steps to Reproduce: 1. 2. 3. Nothing warns there is a problem or the downloading of new file do not start.
Stephen - can you check this with trunk? I'm betting this is fixed on trunk by using sqlite
Keywords: qawanted
Gwen, do you have a public URL to a download with that name that you could put into either the URL field of this bug, or paste inline here? Thanks!
(I tried both searching Google and locally renaming other PDF files to "A9�(8)2.pdf", with no success; XP SP2 and Mac OS X 10.4 wouldn't let me name them as such.
It concerns a filename using Korean characters but I did not manage to reproduce the bug. The same kind of problem appear with other links (found on the same server where i got the bug i reported). Here is an example : The direct link is http://www.mic.go.kr/secureDN.tdf?seq=940&idx=1&board_id=P_03_01_05 (obtained on the board http://www.mic.go.kr/user.tdf?a=user.board.BoardApp&c=2002&board_id=P_03_01_05&seq=940&mc=P_03_01_05 , click on the PDF link) The filename should be "IT839전략(국문)2.pdf" But it is recognized as "IT839?(m8)2.pdf" and cannot be download by Firefox. No error message appears but I checked the error console and it shows the following message : "Error: [Exception... "Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsILocalFile.isDirectory]" nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)" location: "JS frame :: file:///C:/Program%20Files/Mozilla%20Firefox%202%20Beta%202/components/nsHelperAppDlg.js :: anonymous :: line 285" data: no] Source File: file:///C:/Program%20Files/Mozilla%20Firefox%202%20Beta%202/components/nsHelperAppDlg.js Line: 285" Other Korean files can be downloaded, you can check with this example : Direct link to the PDF file "인도시장진출전략-LG(96년).pdf" : http://pds16.cafe.daum.net/download.php?grpid=xWM5&fldid=4365&dataid=1&fileid=1&regdt=20050428231226&disk=19&grpcode=lgindo&dncnt=Y&.pdf&filename=%C0%CE%B5%B5%BD%C3%C0%E5%C1%F8%C3%E2%C0%FC%B7%AB-LG%2896%B3%E2%29.pdf&nenc=uJYY2uU6SKFqrB2TUW-itg00&fenc=gEX31rE7tkk0 Korea Web page containing the link the PDF file "인도시장진출전략-LG(96년).pdf" : http://cafe337.daum.net/_c21_/pds_v3check?grpid=xWM5&fldid=4365&dataid=1&fileid=1&regdt=20050428231226&nenc=uJYY2uU6SKFqrB2TUW-itg00&fenc=gEX31rE7tkk0
Flags: blocking-firefox3?
Target Milestone: --- → Firefox 3 M9
Version: unspecified → Trunk
Attached image Screenshot; 'works' in Fedora Core 7 (deleted) —
Just a datapoint that this works in Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a8pre) Gecko/2007082604 Minefield/3.0a8pre.
Confirming the bug on Windows Vista with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a8pre) Gecko/2007082618 Minefield/3.0a8pre. Mac saves it as "IT839" So, broken on Windows, but works on Mac and Linux.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: in-litmus?
Keywords: qawanted
Flags: blocking-firefox3? → blocking-firefox3+
Severity: critical → major
Keywords: intl
Assignee: nobody → dmose
In the course of figuring out what was going on here, I spun off a patch to fix some minor API problems internal to nsLocalFileWin and put that in bug 216821. It looks like ::CreateFileW is aborting with ERROR_INVALID_NAME. The path itself looks like this "C:\Documents and Settings\Administrator\Desktop\IT839µ(m8)2.pdf" All the parent directories are valid and extant. jshin, any theories on why Windows might be getting upset?
Status: NEW → ASSIGNED
After reading through a bunch of stuff on MS dev site, it seems pretty clear that there are going to be cases where we'll have characters in our filename that the underlying filesystem is going to barf on, and no way to tell that a priori. So we are going to need to handle that case better in general, rather than just fixing the specific problem here. I've pulled the patch mentioned previously back into this bug, added a test for it, added a patch for toolkit, and a test-in-progress for toolkit as well. There are multiple problems with the toolkit test, including bug 299372. So I'm going to shelve this for now and come back to it after bug 299372. Work-in-progress patch attached.
OS: Windows XP → All
Hardware: PC → All
Target Milestone: Firefox 3 M9 → Firefox 3 M10
Priority: -- → P3
Target Milestone: Firefox 3 M10 → ---
Target Milestone: --- → Firefox 3 M11
Whiteboard: [needs bug 299372]
Should see if this could be a regression from bug 278161 (as bug 409796 appears to be).
qawanted added, as per comment 11.
Keywords: qawanted
Ignore comments 11 & 12; I was confused.
Keywords: qawanted
talking to dmose - don't think there is a dependency on the previously mentioned bug (should only be the case for save page as downloads). However, he's kinda sorta busy with some other important work. Jimm - do you have any spare cycles to look at this by chance?
No longer depends on: 299372
Whiteboard: [needs bug 299372]
Can't reproduce this. multiple saves preserve the filename and append (x) on the end. This is on Vista Home Premium.
(That was using trunk from today)
Maybe this is XP only? I was definitely able to reproduce it on XP.
Maybe Stephen can confirm. I don't have access to my xp images this week.
Keywords: qawanted
I couldn't reproduce with: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2008012804 Minefield/3.0b3pre Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b3pre) Gecko/2008012904 Minefield/3.0b3pre Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008012904 Minefield/3.0b3pre Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008012904 Minefield/3.0b3pre I have screenshots, which I can attach if needed, but downloads on all of these OSs worked, and I could also successfully open the file.
Keywords: qawanted
I'm wondering if maybe it matters if you have a non-en windows build?
Attached image Screenshot with German WinXP (deleted) —
It happens for me with a German XP and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008020104 Minefield/3.0b3pre ID:2008020104 I even don't see a failure. But Minefield wants to save the file with a strange character encoding. I think that I don't have installed the unicode fonts. Perhaps the following error appears in the Error Console? Error: uncaught exception: unknown (can't convert to string)
The real filename from comment 4 is IT839\uC804\uB7B5(\uAD6D\uBB38)2.pdf (using javascript escapes for the non-ASCII characters). Comment 8 and the screenshot in attachment 301009 [details] shows that the name is going through a lossy 16-bit to 8-bit conversion. Unfortunately there are still a number of places where we do that with filenames. Bug 411511 will fix some of them but I'm not sure whether what will help this bug or not.
Depends on: 411511
Whiteboard: [needs bug 411511?]
With a few exceptions, I'm mostly focused on MailCo-related hacking now. Reassigning a bunch of bugs to default component owners. I'm happy to help with brainstorming/advice as needed, however. Search for the string MAILMONKEY to delete any bugmail generated by this change.
Assignee: dmose → nobody
Status: ASSIGNED → NEW
It turns out that this isn't us doing a lossy conversion: the filename is already incorrect in the Content-Disposition header. I think we can fix this just by extending the FILE_ILLEGAL_CHARACTERS macro to include all characters < 0x20.
Assignee: nobody → smontagu
Blocks: 419157
Status: NEW → ASSIGNED
No longer depends on: 411511
Whiteboard: [needs bug 411511?]
Attached patch Patch (obsolete) (deleted) — Splinter Review
dmose, what were the problems with your toolkit test apart from bug 299372?
Priority: P3 → P4
Might be what Marcia sees from time to time on the debug build in the lab: marcia_debug: WARNING: Illegal character in window name Download:Manager: file /Users/debug/mozilla/embedding/components/windowwatcher/src/nsWindowWatcher.cpp, line 1372; but that assertion's filename doesn't match the patch in comment 25
I may be missing something, but that warning looks kind of bogus to me. Steps to reproduce and/or a stack would be helpful.
That warning has nothing to do with it. It'll happen anytime the download manager is opened (addons manager too!)
For an explanation just take a look at bug 258480. Could be the same reason.
Right, so either the name "Download:Manager" should be changed, or the warning should be changed. Either way it's not relevant to this bug.
Sorry Simon/Shawn: I was just grasping for straws, and saw that warning come up when Marcia was downloading a PDF at work in her debug build.
Simon - is this patch good to go?
Yeah, except for lack of testcase. The problems are the same as bug 419157. I tried doing it another way for this bug by just creating a file in xpcshell with this filename and then checking the name on disk, but that doesn't go through any codepath that filters illegal characters.
BTW, is P4 the right priority here? It seems too low to me.
Probably at least P3, maybe P2.
Priority: P4 → P3
Target Milestone: Firefox 3 beta3 → Firefox 3 beta5
Depends on: 299372
Shawn, right now P3 means that we won't block final release on it. If you think it needs to be in final, move to P2. Hard for me to tell how common a case this will be, but it feels like we should probably cover the bases.
Target Milestone: Firefox 3 beta5 → Firefox 3
This won't be an issue for english, but I bet it'll come up more for non-english. Given our market share stats, I think it's a good idea to block on.
Priority: P3 → P2
Comment on attachment 306322 [details] [diff] [review] Patch Shawn, can you review? This is in litmus at least, and we need to get it landed.
Attachment #306322 - Flags: review?(sdwilsh)
Whiteboard: [has patch][needs review sdwilsh]
Comment on attachment 306322 [details] [diff] [review] Patch r=sdwilsh, but I'm not a suitable reviewer for this. Asking bsmedberg to take a look as well.
Attachment #306322 - Flags: superreview?(benjamin)
Attachment #306322 - Flags: review?(sdwilsh)
Attachment #306322 - Flags: review?(benjamin)
Attachment #306322 - Flags: review+
Flags: in-testsuite?
Whiteboard: [has patch][needs review sdwilsh] → [has patch][needs review bsmedberg]
No longer blocks: 419157
Comment on attachment 306322 [details] [diff] [review] Patch Why is CONTROL_CHARACTERS a multiline string? I think I'd prefer a concatenated string of the form "\001\002\003\017" \ "\020\021\022" r=me with that change
Attachment #306322 - Flags: superreview?(benjamin)
Attachment #306322 - Flags: review?(benjamin)
Attachment #306322 - Flags: review+
Whiteboard: [has patch][needs review bsmedberg] → [needs new patch[has review]
Whiteboard: [needs new patch[has review] → [needs new patch][has review]
Attached patch Patch for checkin (deleted) — Splinter Review
Attachment #306322 - Attachment is obsolete: true
Attachment #313060 - Flags: review+
Checked in
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Whiteboard: [needs new patch][has review]
With trunk builds on Windows, Linux, and Mac OS X 10.4, I see: IT839_µ(m8)2.pdf; is that correct?
(In reply to comment #44) > IT839_µ(m8)2.pdf; is that correct? Yes :) (see bug 419157 comment 7 -- the filename should really be IT839전략(국문)2.pdf, but the Content-Disposition header gives it as IT839µ(m8)2.pdf, so IT839_µ(m8)2.pdf is the expected result, with the 0x04 character replaced by "_")
Thanks, Simon. Verified FIXED using: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko/2008042704 Minefield/3.0pre Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008042705 Minefield/3.0pre -and- Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9pre) Gecko/2008042704 Minefield/3.0pre
Status: RESOLVED → VERIFIED
This seems to be broken in RC2 on Mac OS X 10.4PPC. On beta 5, [/] or [:] characters were silently replaced with [_] (even replacing [::] with [_] rather than [__], which was confusing) -- maybe not the ideal functionality, but at least nothing was lost. On RC2, the download silently fails if the file name contains [/] or [:]. The .part file remains in the download directory, but the complete file does not seem to be downloaded.
Works excellent for me with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9) Gecko/2008053008 Firefox/3.0 Do you have an example URL of such a broken download manager behavior?
(In reply to comment #48) > Works excellent for me with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; > en-US; rv:1.9) Gecko/2008053008 Firefox/3.0 > > Do you have an example URL of such a broken download manager behavior? It seems unlikely that this would explain the difference, but I'm on a PPC Mac, not an Intel Mac. The behaviour isn't URL-specific -- if I try to save any downloaded file (whatever its original name) with a [/] in the file name, then the download fails in the manner described in <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=393488#c47">comment #47</a>. (The mention of [:] in that comment was wrong -- that character is translated in file dialogues by the OS, not Firefox, into a [-].)
Hmm, I'm sorry. On further testing, what I wrote isn't true -- if I try to save this page, for example, it works fine. I can reproduce the error when trying to save a PDF -- for (a random) example, http://www.google.com/press/guides/desktop_mac_overview.pdf, trying to save it as (say) a/b.pdf.
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: