Apparently, the parser doesn't require ";" between parameters. (Testcase to follow)

Julian, what's your proposal? That we keep the current behavior (and restore test coverage for it), or no longer permit it? The latter is of course a change on the web, so we'd want to make sure we don't break anything unless we think it's merited.

(In reply to Jason Duell (:jduell) from comment #1) > Julian, what's your proposal? That we keep the current behavior (and > restore test coverage for it), or no longer permit it? The latter is of > course a change on the web, so we'd want to make sure we don't break > anything unless we think it's merited. We should follow the spec, or recommend that the spec be changed if following the spec would break the web. My understanding is that there are other bugs with the parsing of this header and/or similar headers. I would rather have a patch that implements a "correct" parser in a sane way, that may be useful for headers with similar syntax, than a quick fix for this particular bug.

Optimally, we fix the bug. Will write a test case and compare with other UAs. We also need to keep in mind that the parser is used for other things, such as Content-Type within HTML. Mid-term, we should write a robust header field tokenizer and use it everywhere.

(In reply to Julian Reschke from comment #0) > Apparently, the parser doesn't require ";" between parameters. > > (Testcase to follow) http://greenbytes.de/tech/tc2231/#attmissingdelim

1) Modifies the parser to stop processing once a semicolon is missing *between* parameters (this aligns the behavior with other UAs, see <http://greenbytes.de/tech/tc2231/#attmissingdelim>) 2) Does not modify the behavior for space-in-token; see <http://greenbytes.de/tech/tc2231/#attmissingdelim2>; Firefox behavior is strictly better than the one observed in other browsers. 3) Does not modify the behavior for semicolon missing between disposition type and parameters, see <http://greenbytes.de/tech/tc2231/#attmissingdelim3>; will treat this as a separate bug. 4) Updates the test cases related to this bug, and fixes another one where the delimiter was indeed missing.

Comment on attachment 631277 [details] [diff] [review] Proposed patch, incl test case Review of attachment 631277 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, and Try is green: https://tbpl.mozilla.org/?tree=Try&rev=ebb09a8c6538 ::: netwerk/test/unit/test_MIME_params.js @@ +397,4 @@ > "attachment", "basic"], > > // Bug 732369: Content-Disposition parser does not require presence of ";" between params > + // optimally, this would no even return the disposition type "attachment" s/no/not/

thanks Julian! https://hg.mozilla.org/integration/mozilla-inbound/rev/fce24c917529