New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 716994 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
OoO until Feb 4th
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 749086



Sign in to add a comment

XHR: Make getAllResponseHeaders() return lowercased header names

Project Member Reported by raphael....@intel.com, May 1 2017

Issue description

Spun 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
 
From https://groups.google.com/a/chromium.org/d/msg/blink-dev/_oxlCPNsrck/Aa0--oCsCAAJ:

> I think there are two things worth doing that might give better insights
> than static HTTPArchive analysis, which seems tricky in this case.
>
> First, take the top sites identified by tyoshino@ and try them with a
> build that has the change. Looking visually for breakage is hard if you're
> not a regular user of the site, so instead set a breakpoint in DevTools
> where getAllResponseHeaders is called and try to follow the return value to
> where it's used.
>
> Second, figure out how far in the Safari release channel
> https://bugs.webkit.org/show_bug.cgi?id=169286 has gotten and search for
> regressions in their bug database. I assume it's not in stable Safari yet,
> but once it is, that's a good signal that the change is web compatible.

Yoshino-san, your help would be appreciated with item 1, as at the moment I don't have access to those top sites you mentioned.
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
results-20170501-202707.csv
15.5 KB View Download
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.
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.
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?

Great analysis.

Please share this at the blink-dev thread maybe by pointing to the comments.
Labels: M-60
Status: Fixed (was: Started)
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!
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.
Great. Thank you for taking care of the tough process, Raphael!
Blocking: 749086

Sign in to add a comment