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)
Toolkit
Downloads API
Tracking
()
VERIFIED
FIXED
mozilla1.9
People
(Reporter: goowhen, Assigned: smontagu)
References
Details
(Keywords: intl)
Attachments
(4 files, 1 obsolete file)
(deleted),
image/png
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
image/jpeg
|
Details | |
(deleted),
patch
|
smontagu
:
review+
|
Details | Diff | Splinter Review |
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.
Comment 1•17 years ago
|
||
Stephen - can you check this with trunk? I'm betting this is fixed on trunk by using sqlite
Keywords: qawanted
Comment 2•17 years ago
|
||
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!
Comment 3•17 years ago
|
||
(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®dt=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®dt=20050428231226&nenc=uJYY2uU6SKFqrB2TUW-itg00&fenc=gEX31rE7tkk0
Updated•17 years ago
|
Flags: blocking-firefox3?
Target Milestone: --- → Firefox 3 M9
Version: unspecified → Trunk
Comment 5•17 years ago
|
||
Just a datapoint that this works in Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a8pre) Gecko/2007082604 Minefield/3.0a8pre.
Comment 6•17 years ago
|
||
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.
Comment 7•17 years ago
|
||
Flags: in-litmus? → in-litmus+
Updated•17 years ago
|
Flags: blocking-firefox3? → blocking-firefox3+
Updated•17 years ago
|
Assignee: nobody → dmose
Comment 8•17 years ago
|
||
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
Comment 9•17 years ago
|
||
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.
Updated•17 years ago
|
OS: Windows XP → All
Hardware: PC → All
Target Milestone: Firefox 3 M9 → Firefox 3 M10
Updated•17 years ago
|
Priority: -- → P3
Target Milestone: Firefox 3 M10 → ---
Updated•17 years ago
|
Target Milestone: --- → Firefox 3 M11
Updated•17 years ago
|
Whiteboard: [needs bug 299372]
Comment 11•17 years ago
|
||
Should see if this could be a regression from bug 278161 (as bug 409796 appears to be).
Comment 14•17 years ago
|
||
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]
Comment 15•17 years ago
|
||
Can't reproduce this. multiple saves preserve the filename and append (x) on the end. This is on Vista Home Premium.
Comment 16•17 years ago
|
||
(That was using trunk from today)
Comment 17•17 years ago
|
||
Maybe this is XP only? I was definitely able to reproduce it on XP.
Comment 18•17 years ago
|
||
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
Comment 20•17 years ago
|
||
I'm wondering if maybe it matters if you have a non-en windows build?
Comment 21•17 years ago
|
||
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)
Assignee | ||
Comment 22•17 years ago
|
||
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
Updated•17 years ago
|
Whiteboard: [needs bug 411511?]
Comment 23•17 years ago
|
||
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
Assignee | ||
Comment 24•17 years ago
|
||
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
Assignee | ||
Updated•17 years ago
|
Assignee | ||
Comment 25•17 years ago
|
||
dmose, what were the problems with your toolkit test apart from bug 299372?
Updated•17 years ago
|
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
Assignee | ||
Comment 27•17 years ago
|
||
I may be missing something, but that warning looks kind of bogus to me. Steps to reproduce and/or a stack would be helpful.
Comment 28•17 years ago
|
||
That warning has nothing to do with it. It'll happen anytime the download manager is opened (addons manager too!)
Comment 29•17 years ago
|
||
For an explanation just take a look at bug 258480. Could be the same reason.
Assignee | ||
Comment 30•17 years ago
|
||
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.
Comment 32•17 years ago
|
||
Simon - is this patch good to go?
Assignee | ||
Comment 33•17 years ago
|
||
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.
Assignee | ||
Comment 34•17 years ago
|
||
BTW, is P4 the right priority here? It seems too low to me.
Comment 35•17 years ago
|
||
Probably at least P3, maybe P2.
Priority: P4 → P3
Target Milestone: Firefox 3 beta3 → Firefox 3 beta5
Comment 36•17 years ago
|
||
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
Comment 37•17 years ago
|
||
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 38•17 years ago
|
||
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)
Updated•17 years ago
|
Whiteboard: [has patch][needs review sdwilsh]
Comment 39•17 years ago
|
||
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+
Updated•17 years ago
|
Flags: in-testsuite?
Whiteboard: [has patch][needs review sdwilsh] → [has patch][needs review bsmedberg]
Comment 40•17 years ago
|
||
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+
Updated•17 years ago
|
Whiteboard: [has patch][needs review bsmedberg] → [needs new patch[has review]
Updated•17 years ago
|
Whiteboard: [needs new patch[has review] → [needs new patch][has review]
Assignee | ||
Comment 41•17 years ago
|
||
Attachment #306322 -
Attachment is obsolete: true
Attachment #313060 -
Flags: review+
Assignee | ||
Comment 42•17 years ago
|
||
Checked in
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Updated•17 years ago
|
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?
Assignee | ||
Comment 45•17 years ago
|
||
(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
Comment 47•16 years ago
|
||
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.
Comment 48•16 years ago
|
||
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?
Comment 49•16 years ago
|
||
(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 [-].)
Comment 50•16 years ago
|
||
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.
Updated•16 years ago
|
Product: Firefox → Toolkit
You need to log in
before you can comment on or make changes to this bug.
Description
•