XHR: Make getAllResponseHeaders() return lowercased header names |
|||
Issue descriptionSpun off issue 700434 . See https://github.com/whatwg/xhr/issues/109 and https://github.com/whatwg/xhr/commit/ecce3904ace: XHR.getAllResponseHeaders() is supposed to follow steps defined in the Fetch spec, including lowercasing all the header names it returns. An intent email was sent to blink-dev to discuss this behavior change: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/_oxlCPNsrck
,
May 1 2017
See the attached, please. 3rd column is the JS file name used by the site (2nd column) that contains getAllResponseHeaders() call. I haven't checked if the top page has it or some page belong to the origin has it. If you can't find the JS file when you load the sites, please tell me so. Sorry but I can't check that now as I need to go home now. Thanks
,
May 2 2017
Thank you for the data. I am currently busy with other tasks at work, but will try to get back to this later this week, or early next week the latest.
,
May 5 2017
I've gone through more than half of the sites on that list so far (runs_rank=[46,84] are still pending). Almost all of them reference getAllResponseHeaders() by bundling jQuery, but that specific code path is not exercised at all in any of the sites I've tried. Then there are some sites that bundle a Fetch polyfill when a native fetch() is not available, as well as libraries such as "superagent" that already lowercases header names. The unknowns so far are: - YUI (the Yahoo! UI framework), calls getAllResponseHeaders() and exposes it in the API, but it was not used by any of the sites bundling it. - Proprietary stuff like IBM Tealeaf: it looks like some customer analytics product and I couldn't get the getAllResponseHeaders() call to be triggered. - Things that may require a login to be triggered, as may be the case for ESPN's DisneyID.js. The only real user of the API I've found so far is YouTube, and it is already lowercasing header names when doing comparisons and lookups. I'll continue looking into this next week.
,
May 9 2017
I've finally finished checking the websites from comment #2, and what I said in comment #4 still holds:
* The vast majority of the websites reference getAllResponseHeaders() because ship jQuery, but no code path calling getAllResponseHeaders() is ever called in any of the sites.
* There are some other frameworks such as YUI and Naver's Jindo.js that also just forward whatever getAllResponseHeaders() returns, but none of the websites that use them reach those code paths at all.
* Angular.js code does not immediately normalize getAllResponseHeader() header names, but it provides several functions such as parseHeaders() that automatically convert all header names to lowercase.
* Google's closure-library seems to behave similarly, and none of the websites using it ever called the getAllResponseHeaders() functions.
* Many websites also ship a fetch polyfill that's obviously not used by us.
* There's some analytics stuff from IBM Tealeaf and Clicktale that call getAllResponseHeaders(), but that function was never called here.
* Some websites just call getAllResponseHeaders() for debugging purposes (ie. if something goes wrong the headers are printed somewhere).
* Most of the websites that really use getAllResponseHeaders(), like YouTube, already lowercase the header names when doing comparisons and lookups.
The only website that can genuinely break in some cases with this change is ok.ru. It contains code that looks like this:
var y = q.getAllResponseHeaders(), x;
while (x = p.exec(y)) {
j[x[1]] = x[2];
}
if (!g && j["Redirect-Location"]) {
f(j["Redirect-Location"]);
t(q, q.statusText, "Redirect");
return;
}
where p is a regular expression object. I didn't receive any payload with redirect-location in my tests, but it's clear that the check above will break when all headers are lowercased.
Given the above, how should we proceed here?
,
May 12 2017
Great analysis. Please share this at the blink-dev thread maybe by pointing to the comments.
,
May 22 2017
Oops, https://bugs.chromium.org/p/chromium/issues/detail?id=700434#c10 should have been posted here (I forgot to update the bug number in the CL). We finally got all the required LGTMs for the change and it was landed a few minutes ago!
,
May 22 2017
Nice work! Firefox fixed its behavior too a couple days ago: https://bugzilla.mozilla.org/show_bug.cgi?id=1348390. Definitely a quicker turnaround than expected.
,
May 23 2017
Great. Thank you for taking care of the tough process, Raphael!
,
Jul 27 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by raphael....@intel.com
, May 1 2017