Bug 167112 – HTTP Header values validation is too strict

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Anne van Kesteren]

See tests added in https://github.com/w3c/web-platform-tests/pull/4525.

[John Wilander]

I'd like to have information here in the bug of what we are (too) strict about, what should be relaxed, and why. Thanks!

[youenn fablet]

(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

[youenn fablet]

Created attachment 299159 [details]
Patch

[youenn fablet]

Created attachment 299160 [details]
Adding missing expectations

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[youenn fablet]

Created attachment 299246 [details]
Rebasing bogus name test

[John Wilander]

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?

[youenn fablet]

(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.

[youenn fablet]

Created attachment 340629 [details]
Patch

[EWS Watchlist]

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

[EWS Watchlist]

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

[youenn fablet]

Created attachment 340643 [details]
Patch

[Alex Christensen]

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.

[youenn fablet]

Created attachment 342114 [details]
Patch for landing

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 342114 [details]:

css3/filters/crash-filter-animation-invalid-url.html bug 186381 (authors: [e-mail] and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 342114 [details]
Patch for landing

Clearing flags on attachment: 342114

Committed r232572: <https://trac.webkit.org/changeset/232572>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/40880447>