| Summary: | HTTP Header values validation is too strict | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Anne van Kesteren <annevk> | ||||||||||||||||||||||||
| Component: | DOM | Assignee: | youenn fablet <youennf> | ||||||||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||||||||
| Severity: | Normal | CC: | achristensen, ap, buildbot, cdumez, commit-queue, esprehn+autocc, ews-watchlist, kondapallykalyan, rniwa, webkit-bug-importer, wilander, youennf | ||||||||||||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||||||
| Version: | Safari Technology Preview | ||||||||||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||||
|
Description
Anne van Kesteren
2017-01-16 23:57:20 PST
I'd like to have information here in the bug of what we are (too) strict about, what should be relaxed, and why. Thanks! (In reply to comment #1) > I'd like to have information here in the bug of what we are (too) strict > about, what should be relaxed, and why. Thanks! I introduced a while back enforcement of the ABNF for header values. This is now obsolete and no other browser is implementing it. Fetch API is temporarily defining validation rules for header values. I guess that once HTTPBis WG will carry on the changes to the corresponding RFC, fetch spec will just refer to it. The restriction I added is roughly that all characters below 0x20 (except for tab) would lead to make a header value invalid. The proposed validation rule is defined in https://fetch.spec.whatwg.org/#concept-header Created attachment 299159 [details]
Patch
Created attachment 299160 [details]
Adding missing expectations
Comment on attachment 299160 [details] Adding missing expectations Attachment 299160 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2910917 New failing tests: imported/w3c/web-platform-tests/XMLHttpRequest/setrequestheader-bogus-name.htm Created attachment 299165 [details]
Archive of layout-test-results from ews101 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 299160 [details] Adding missing expectations Attachment 299160 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2910913 New failing tests: imported/w3c/web-platform-tests/XMLHttpRequest/setrequestheader-bogus-name.htm Created attachment 299166 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 299160 [details] Adding missing expectations Attachment 299160 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2910918 New failing tests: imported/w3c/web-platform-tests/XMLHttpRequest/setrequestheader-bogus-name.htm Created attachment 299167 [details]
Archive of layout-test-results from ews113 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 299160 [details] Adding missing expectations Attachment 299160 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/2910921 New failing tests: imported/w3c/web-platform-tests/XMLHttpRequest/setrequestheader-bogus-name.htm Created attachment 299168 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Created attachment 299246 [details]
Rebasing bogus name test
The relaxation of isValidHTTPHeaderValue() affects XMLHttpRequest too. Are we expected to change legacy APIs with Fetch changes? Are we currently breaking things with our XHR behavior? (In reply to comment #14) > The relaxation of isValidHTTPHeaderValue() affects XMLHttpRequest too. Are > we expected to change legacy APIs with Fetch changes? Are we currently > breaking things with our XHR behavior? AFAIAK, we are not breaking things but we are not consistent with other browsers nor aligned anymore with the specs. XHR is not added any feature but is still evolving, mainly because of it being defined in terms of fetch. Created attachment 340629 [details]
Patch
Comment on attachment 340629 [details] Patch Attachment 340629 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/7714170 New failing tests: http/tests/xmlhttprequest/set-bad-headervalue.html Created attachment 340639 [details]
Archive of layout-test-results from ews101 for mac-sierra
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-sierra Platform: Mac OS X 10.12.6
Created attachment 340643 [details]
Patch
Comment on attachment 340643 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=340643&action=review This makes us match Chrome and Firefox. r=me > Source/WebCore/platform/network/HTTPParsers.cpp:113 > +// See https://fetch.spec.whatwg.org/#concept-header until RFC 7230 gets fixed. I'm not sure "until RFC 7230 gets fixed" is the right thing to say here. Created attachment 342114 [details]
Patch for landing
The commit-queue encountered the following flaky tests while processing attachment 342114 [details]: css3/filters/crash-filter-animation-invalid-url.html bug 186381 (authors: jhoneycutt@apple.com and simon.fraser@apple.com) The commit-queue is continuing to process your patch. Comment on attachment 342114 [details] Patch for landing Clearing flags on attachment: 342114 Committed r232572: <https://trac.webkit.org/changeset/232572> All reviewed patches have been landed. Closing bug. |