Multiple Content-disposition headers cause filename problem

RESOLVED DUPLICATE of bug 655389

Status

()

RESOLVED DUPLICATE of bug 655389
10 years ago
7 years ago

People

(Reporter: dennis.rosenkvist, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; da; rv:1.9) Gecko/2008052906 Firefox/3.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; da; rv:1.9) Gecko/2008052906 Firefox/3.0

If two content-disposition http headers are present - firefox appends a comma to the filename. Of course the header should not be there twice but firefox could handle it better.

Reproducible: Always

Steps to Reproduce:
Viewing the http header in fiddler (or other http debugger)


Expected Results:  
Choose one of the filenames

Comment 1

9 years ago
As far as I can tell, Firefox does the right thing here (treating multiple header instances as defined in Section 4.2 of RFC 2616).

Comment 2

8 years ago
The original poster is correct.  Firefox does not handle multiply defined Content-Disposition headers sanely.  If there are two response headers like:

Content-Disposition: application/download;filename=foo.csv
Content-Disposition: application/download;filename=foo.csv

Firefox will either:

Prompt to open a file of type "csv," or, if saving, the default file name will be "foo.csv,".  That ain't right no matter how you look at it.

Comment 3

8 years ago
This message is invalid, as the syntax for C-D does not allow repeating header, see <http://greenbytes.de/tech/webdav/rfc2616.html#rfc.section.4.2.p.5>.

So this falls into error recovery.

I'd urge you not add new workarounds for broken messages, unless you have evidence that they occur in practice (not just once), and that other UAs accept them as well.

Comment 4

8 years ago
There's no doubt that multiple definitions of the Content-Disposition is an error, but Firefox could still handle it better.

It could (a) throw out all Content-Disposition headers since it wouldn't know which one to choose; or (b) pick one; or ...

I don't what Firefox is doing now internally, but it presents a bad user interface for the end user when faced with this server error.  If it handled this problem per (a) or (b) above, it would be better.

IE8 appears to pick one. Opera 10.62 appears to pick one.  Safari 4.0.5 behaves bizarrely by giving it a "csvattachment" file suffix.  I have not tested other browsers.
Version: unspecified → 3.0 Branch

Comment 5

8 years ago
Slightly related test case at <http://greenbytes.de/tech/tc2231/#attmultinstances>.

It tests two instances, separated by a comma (which, in HTTP, is supposed to be equivalent to having multiple instances of the header).

At least in this case, there's no interoperability whatsoever.

Comment 6

8 years ago
It does sound like we should be doing something more sensible here--as comment 4 says, either (a) throw out all Content-Disposition headers since it wouldn't know which one to choose; or (b) pick one;

The server behavior is outside the HTTP spec, so we're certainly free to throw them all out.

What makes this tricky implementation-wise is that we're simply merging the headers together when AddHeader is called the second time, and our parsing logic then isn't smart enough to know that the comma is from the merge, and not part of the filename.   We could fix it by either not merging C-D headers, or by making the parser smarter when it sees them.  The former is probably a cleaner design.
Assignee: nobody → hurley
Blocks: 609667

Comment 7

8 years ago
(In reply to comment #6)
> It does sound like we should be doing something more sensible here--as
> comment 4 says, either (a) throw out all Content-Disposition headers since
> it wouldn't know which one to choose; or (b) pick one;
> 
> The server behavior is outside the HTTP spec, so we're certainly free to
> throw them all out.

Indeed, and that would be my preference.

> What makes this tricky implementation-wise is that we're simply merging the
> headers together when AddHeader is called the second time, and our parsing
> logic then isn't smart enough to know that the comma is from the merge, and
> not part of the filename.   We could fix it by either not merging C-D
> headers, or by making the parser smarter when it sees them.  The former is
> probably a cleaner design.

Understood, and by design.

Note that HTTPbis will *allow* recipients to accept multiple instances of the Content-Length header field when their values are the same, and we *could* do this here as well. However, we also should avoid adding special cases and workarounds when it's not really really necessary, and I don't believe it is.

Finally, the change for bug 588781 already has changed FF5's behavior to use the second value, which will *not* have the trailing comma.

Updated

8 years ago
OS: Windows XP → Windows 7
Version: 3.0 Branch → Trunk

Updated

8 years ago
OS: Windows 7 → All
Hardware: x86 → All

Comment 8

7 years ago
Data point: since rel 16, Chrome not even throws away the header fields but treats the whole message as broken; see <http://code.google.com/p/chromium/issues/detail?id=103618>. If they can keep this behavior, Firefox probably could just align with that, or minimally treat the header field as not present at all (option (a) from above).

Updated

7 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

7 years ago
Assignee: hurley → nobody

Comment 9

7 years ago
As of bug 655389 we fail network requests with multiple C-D headers, which fixed this as a side-effect.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 655389
You need to log in before you can comment on or make changes to this bug.