275110 - Content-disposition filename handling violates RFCs 2183 and 2616

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Josh Aas]

On line 587 of contentAreaUtils.js, you'll see:

fileName = fileName.replace(/^"|"$/g, "");

This strips quotes returned from getParameter(), where it is asked for the file
name suggested in the Content-disposition value of the HTTP response header.
This violates RFCs 2183 and 2616. This line appears to be a hack added in
response to bugzilla sending incorrect headers, which biesi and I discovered
today. See bug 275108. The value you get from getParameter() is the correct
suggested value - it shouldn't be messed with.

Comment 1

[Josh Aas]

The solution is to just remove that line.

Comment 2

[Phil Ringnalda (:philor)]

ccing jshin, since that "hack added in response to bugzilla sending incorrect
headers" was actually added in bug 215493 as a port of bug 158006 (where I don't
see any mention of bugzilla's headers) which Seamonkey used right up until bug
160454 pulled all that code out in October. Might be worthwhile finding out why
it was there before we rip it out as having been a bugzilla workaround.

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

Phil, that bug did not add this line. see the patch there - attachment 126311 [details] [diff] [review]
shows that the line existed before, it was just moved.

Instead, bz added it in bug 121509. it seems to me, though, that at least since
we have nsIMIMEHeaderParam we don't need it anymore.

Comment 4

[Josh Aas]

Note that this line exists in and should be removed from the code that the moz
suite uses for the same purpose...

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #4)
> Note that this line exists in and should be removed from the code that the moz
> suite uses for the same purpose...

hm... really? I can't find it; could you mention the location of that line?

Comment 6

[Boris Zbarsky [:bzbarsky]]

biesi is right -- this line can just go.  

And I also can't seem to find it in trunk seamonkey....

Comment 7

[Josh Aas]

Attachment: fix v1.0

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 172844 [details] [diff] [review]
fix v1.0

please find a firefox peer to review this patch

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 172844 [details] [diff] [review]
fix v1.0

Not that this is needed for Firefox..

Comment 10

[Josh Aas]

landed on trunk