Closed Bug 419157 Opened 14 years ago Closed 14 years ago

Filename from HTTP headers with 8bit characters and no charset information truncated on Mac

Categories

(Toolkit :: Downloads API, defect, P2)

x86
Windows XP
defect

Tracking

()

VERIFIED FIXED
mozilla1.9

People

(Reporter: donatpub, Assigned: smontagu)

References

()

Details

Attachments

(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008022204 Minefield/3.0b4pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008022204 Minefield/3.0b4pre

If this is not a bug, please revise FX 3.0 FFT - Downloading #45.

Reproducible: Always

Steps to Reproduce:
1. Set "Always ask me where to save files".
2. Click on http://www.mic.go.kr/secureDN.tdf?seq=940&idx=1&board_id=P_03_01_05 and save the file.

Actual Results:  
Note that when the save as dialog appears, the filename is blank.


Expected Results:  
A suggested filename should appear in the dialog.
Sorry; it helps to read the steps to reproduce.

Yeah, I can confirm this with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008022204 Minefield/3.0b4pre.

Michael: do you have any idea when this regressed?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-firefox3?
Version: unspecified → Trunk
Comment on attachment 305174 [details]
Screenshot

Ignore this!
Attachment #305174 - Attachment is obsolete: true
Stephen: No idea when this regressed - sorry.

The test results for Testcase ID #4607 suggest this is a platform issue. I'm surprised there's a failure recorded for the Mac and Solaris given the positive results on Linux.
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P2
Are the positive results reported for Linux correct? It fails for me on all platforms. Note that the correct file name is IT839전략(국문)2.pdf (with Korean characters) -- saving the file as IT839µ(m8)2.pdf should be considered a failure even if the download succeeds.
I need to back down from comment 5: the filename maybe *should* be IT839전략(국문)2.pdf, but it's IT839µ(m8)2.pdf already in the Content-Disposition header, so saving it that way is not a failure after all.
Depends on: 393488
Assignee: nobody → sdwilsh
I really have *no* idea about this bug.  I'm probably not the best assignee for it.
Whiteboard: [needs patch][swag ?]
This should be fixed by bug 393488.
Assignee: sdwilsh → smontagu
Attached patch Patch (obsolete) — Splinter Review
On Mac 0x04 is a legal character in a filename, so we can fix this even without bug 393488.

Shawn, can you give me some pointers on how to write a testcase for this?
32-bit or 64-bit?

In all seriousness though, I thought about this and wasn't so sure.  You'd have to use a mochitest I think that redirected to a file with the right headers.  I haven't actually done this before, but I think it exists elsewhere in the codebase.  Waldo might be useful to consult here, or sayrer.
I know how to trigger a download with the right headers from a mochitest. The part I don't know how to do is (a) get past the "what do you want to do with this file" dialog and (b) retrieve the suggested filename from the filepicker.
(a) should be easy with the window watcher, but I have no idea how to do (b)
Comment on attachment 308725 [details] [diff] [review]
Patch

Our file pickers are native, so I don't think we can retrieve that information with a test.  I'm OK with a in-testsuite? on this bug.
Attachment #308725 - Flags: superreview?(cbiesinger)
Attachment #308725 - Flags: review?(cbiesinger)
Whiteboard: [needs patch][swag ?] → [has patch][needs review biesi][needs sr biesi]
No longer depends on: 393488
Flags: in-testsuite?
Target Milestone: --- → Firefox 3
that patch does not touch the file picker at all, why do you think it can't be tested/is hard to test?

and why does this fix the bug? surely that patch can't produce the filename in comment 5?
First of all, this bug has morphed and I didn't really realize until rereading it how unclear it is -- sorry about that. Comment 5 is also wrong: see comment 7. The Content-Disposition header from the server has already been through a lossy 16bit to 8bit conversion, so there's no chance of restoring the original Korean filename and the best we can hope for is to save the corrupted name without failing.

The issue originally reported has been fixed in bug 393488; the issue I'm fixing here is that on Mac the filename gets truncated as in the last 3 links in comment 6. This happens because after we check that the header isn't UTF-8 we try to fall back to the native charset, but on Mac NS_CopyNativeToUnicode just does CopyUTF8toUTF16 without any error checking. The patch just adds another level of fallback.

About testing, if there is a scenario where we can do something with the filename from the Content-Disposition header without even triggering any dialogs, we will be fine. 
Summary: saving to filename with special chars and picking where to save files loses filename → Filename from HTTP headers with 8bit characters and no charset information truncated on Mac
well it seems like you could just test the nsIMIMEHeaderParam behaviour.
Comment on attachment 308725 [details] [diff] [review]
Patch

r+sr=biesi if you update the IDL for this interface to document this new behaviour and add a unit test as mentioned in my comment above.
Attachment #308725 - Flags: superreview?(cbiesinger)
Attachment #308725 - Flags: superreview+
Attachment #308725 - Flags: review?(cbiesinger)
Attachment #308725 - Flags: review+
Attachment #308725 - Attachment is obsolete: true
Attachment #314113 - Flags: superreview+
Attachment #314113 - Flags: review+
Whiteboard: [has patch][needs review biesi][needs sr biesi] → [has patch][has reviews][can land]
Checked in
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [has patch][has reviews][can land] → [has patch]
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008050718 Minefield/3.0pre

Verified.
Status: RESOLVED → VERIFIED
Whiteboard: [has patch]
Product: Firefox → Toolkit
Depends on: 484015
You need to log in before you can comment on or make changes to this bug.