Last Comment Bug 670333 - Content-Disposition parser does not require presence of "=" in params
: Content-Disposition parser does not require presence of "=" in params
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Networking (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla8
Assigned To: Julian Reschke
:
Mentors:
http://greenbytes.de/tech/tc2231/#att...
Depends on:
Blocks: 609667
  Show dependency treegraph
 
Reported: 2011-07-09 03:09 PDT by Julian Reschke
Modified: 2011-08-09 01:10 PDT (History)
3 users (show)
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
test case and proposed patch (4.49 KB, patch)
2011-07-10 08:27 PDT, Julian Reschke
bzbarsky: review+
Details | Diff | Review
proposed patch (4.66 KB, patch)
2011-07-11 11:06 PDT, Julian Reschke
no flags Details | Diff | Review

Description Julian Reschke 2011-07-09 03:09:58 PDT
When parsing C-D header fields, the code apparently accepts params without no equals characters and tolerates whitespace as well.

Test case at <http://greenbytes.de/tech/tc2231/#attwithfn2231ws1>

Header field:

  Content-Disposition: attachment; filename *=UTF-8''foo-%c3%a4.html

Extracted filename:

  _=UTF-8''foo-%c3%a4.html

which appears to be the next element in the field, with "*" replaced by "_" in order to produce a safe filename.
Comment 1 Julian Reschke 2011-07-10 08:27:56 PDT
Created attachment 545075 [details] [diff] [review]
test case and proposed patch

test case and mimimal patch; checking that we indeed saw a "=" between name and parameter, otherwise skipping
Comment 2 Boris Zbarsky [:bz] 2011-07-11 09:06:54 PDT
Comment on attachment 545075 [details] [diff] [review]
test case and proposed patch

Can you also add a test that "filename = foo-A.html" still works?

Also, wouldn't it make sense to make "actual bug" and "sanity check" identical except for the space before '*'?

r=me with those changes.
Comment 3 Julian Reschke 2011-07-11 11:06:29 PDT
Created attachment 545215 [details] [diff] [review]
proposed patch

test cases (improved as suggested by Boris) and proposed patch
Comment 4 Boris Zbarsky [:bz] 2011-07-11 19:37:40 PDT
Thanks!

Pushed http://hg.mozilla.org/integration/mozilla-inbound/rev/5a7b496ddbae
Comment 5 Mounir Lamouri (:mounir) 2011-07-12 03:48:15 PDT
Merged:
http://hg.mozilla.org/mozilla-central/rev/5a7b496ddbae

Note You need to log in before you can comment on or make changes to this bug.