Stop lowercasing header names |
|||||||||||
Issue descriptionSee https://github.com/whatwg/fetch/pull/476 and referenced issues and please provide input if you have any. It seems Chrome is currently the only browser that lowercases so most likely to run into issues.
,
Mar 16 2017
,
Mar 30 2017
,
Apr 7 2017
It looks like the WPT tests for this spec change are under XMLHttpRequest, which we don't currently import: https://github.com/w3c/web-platform-tests/commit/0a3f56149526619aaf9bd60af66c008df9eab0c5 Could we pull in those tests?
,
Apr 7 2017
There's also https://github.com/w3c/web-platform-tests/pull/5115, but that one also requires issue 700434 . W3CImportExpectations shows jsbell@ is the owner for external/wpt/XMLHttpRequest, so I guess it's up to him to import them? I also mentioned in issue 705490 that we need at least part of that directory for some Fetch tests to pass as well.
,
Apr 7 2017
,
Apr 7 2017
,
Apr 7 2017
The # Owners annotation just applies to the #...[Skip] line right below it. So it's not me. :) Anyone can be the "owner", just need to comment out the [Skip] line and wait for the next import. Imported tests that fail should automatically get appropriate expectations added.
,
Apr 12 2017
Sorry for the confusion :-) I've finally uploaded a CL to import the XHR tests: https://codereview.chromium.org/2808403003/
,
Apr 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5bc8d44f3ab226674d0dd2c52ef631d6d52dc464 commit 5bc8d44f3ab226674d0dd2c52ef631d6d52dc464 Author: raphael.kubo.da.costa <raphael.kubo.da.costa@intel.com> Date: Wed Apr 12 15:06:27 2017 Import XMLHttpRequest wpt tests. Some Fetch API tests require bits from the /XMLHttpRequest directory. Some of the tests are also required for correctly testing behavior after changes to the Fetch and XHR specs. BUG= 687155 ,705490 R=falken@chromium.org,tyoshino@chromium.org,yhirano@chromium.org Review-Url: https://codereview.chromium.org/2808403003 Cr-Commit-Position: refs/heads/master@{#464024} [modify] https://crrev.com/5bc8d44f3ab226674d0dd2c52ef631d6d52dc464/third_party/WebKit/LayoutTests/W3CImportExpectations
,
Apr 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f8ebda9548f2a4bfc00d414f48512327b2d656d7 commit f8ebda9548f2a4bfc00d414f48512327b2d656d7 Author: raphael.kubo.da.costa <raphael.kubo.da.costa@intel.com> Date: Tue Apr 18 13:36:04 2017 Fetch API: Stop lowercasing header names. Adapt to https://github.com/whatwg/fetch/pull/476, which basically says a header list should not lowercase the header names it is passed, but rather only do that in the "sort and combine" algorithm. This means several test expectations had to be updated because casing _will_ be preserved when a header is set. To accommodate the change, turn FetchHeaderList::header_list_ into an std::multimap with a custom comparison function in order to implement the concept of multiple headers with possibly different casing that should all be treated as the same header and kept sorted. Special care has been taken not to change FetchHeaderList's API unless absolutely necessary, but simply to avoid making the diff needlessly large. BUG= 687155 R=haraken@chromium.org,tyoshino@chromium.org,yhirano@chromium.org Review-Url: https://codereview.chromium.org/2787003002 Cr-Commit-Position: refs/heads/master@{#465214} [modify] https://crrev.com/f8ebda9548f2a4bfc00d414f48512327b2d656d7/content/browser/service_worker/service_worker_browsertest.cc [modify] https://crrev.com/f8ebda9548f2a4bfc00d414f48512327b2d656d7/third_party/WebKit/LayoutTests/http/tests/fetch/resources/thorough-util.js [modify] https://crrev.com/f8ebda9548f2a4bfc00d414f48512327b2d656d7/third_party/WebKit/Source/modules/BUILD.gn [modify] https://crrev.com/f8ebda9548f2a4bfc00d414f48512327b2d656d7/third_party/WebKit/Source/modules/cachestorage/Cache.cpp [modify] https://crrev.com/f8ebda9548f2a4bfc00d414f48512327b2d656d7/third_party/WebKit/Source/modules/fetch/FetchHeaderList.cpp [modify] https://crrev.com/f8ebda9548f2a4bfc00d414f48512327b2d656d7/third_party/WebKit/Source/modules/fetch/FetchHeaderList.h [add] https://crrev.com/f8ebda9548f2a4bfc00d414f48512327b2d656d7/third_party/WebKit/Source/modules/fetch/FetchHeaderListTest.cpp [modify] https://crrev.com/f8ebda9548f2a4bfc00d414f48512327b2d656d7/third_party/WebKit/Source/modules/fetch/FetchManager.cpp [modify] https://crrev.com/f8ebda9548f2a4bfc00d414f48512327b2d656d7/third_party/WebKit/Source/modules/fetch/FetchResponseData.cpp [modify] https://crrev.com/f8ebda9548f2a4bfc00d414f48512327b2d656d7/third_party/WebKit/Source/modules/fetch/Headers.cpp [modify] https://crrev.com/f8ebda9548f2a4bfc00d414f48512327b2d656d7/third_party/WebKit/Source/modules/fetch/Request.cpp
,
Apr 18 2017
,
May 2 2017
Is this fixed? external/wpt/XMLHttpRequest/getallresponseheaders-cl.htm is still failing: FAIL Casing of known headers assert_equals: expected "content-length: 0\r\n" but got "CONTENT-LENGTH: 0\r\n" PASS Casing of known headers 1 PASS Casing of known headers 2 Harness: the test ran to completion.
,
May 2 2017
That is a test for how response headers are exposed through the API. This bug is about not lowercasing request header names.
,
May 2 2017
Well I'm confused, is there a web platform test that Chrome previously failed on and then started passing after comment 11 landed?
,
May 2 2017
The two subtests that PASS didn't both PASS I think. In particular the last one used to fail due to fetch() lowercasing request header names. I was just explaining the FAIL case.
,
May 2 2017
I see, an additional infra change was needed to get those test cases to pass: https://chromium-review.googlesource.com/c/481760/ I think comment 11 plus that change got the tests to pass.
,
May 2 2017
,
May 2 2017
As for the remaining failure: see issue 716994 . |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by yhirano@chromium.org
, Jan 31 2017