XMLHttpRequest: make setRequestHeader() use `, ` as separator (including a space)

• Component: DOM
• Status: RESOLVED FIXED

[Anne van Kesteren]

See https://github.com/whatwg/xhr/issues/108.

web-platform-tests at:

  XMLHttpRequest/setrequestheader-case-insensitive.htm
  XMLHttpRequest/setrequestheader-header-allowed.htm

[Alexey Proskuryakov]

We just changed it to never add a space, in part because doing otherwise would be complicated for CFNetwork based ports.

[youenn fablet]

If we change XHR to use ', ', we will also change it to fetch.
Currently we are using ',' for both, except when CFNetwork is changing that behind the scene, which it should really not.

[Anne van Kesteren]

I have been trying to get your input in the issue, but am glad filing bugs is working out better.

It sounds like the preference from WebKit is that we only have a single separator style, used by:

  XMLHttpRequest
  WebSocket
  Fetch API
  CORS

Knowing that makes it easier to talk with the other browsers. I suspect what we want to do is use `, ` and also change Fetch to use that and any Fetch API tests.

`Access-Control-Request-Headers` seems tricky though, as I believe no browser uses a space after the commas there now. Is that problematic?

[youenn fablet]

(In reply to comment #3)
> I have been trying to get your input in the issue, but am glad filing bugs
> is working out better.

Sorry about that, I plan to get back to fetch/XHR soon.
I should probably improve my GitHub notification filter.

> It sounds like the preference from WebKit is that we only have a single
> separator style, used by:
> 
>   XMLHttpRequest
>   WebSocket
>   Fetch API
>   CORS

Right.

> Knowing that makes it easier to talk with the other browsers. I suspect what
> we want to do is use `, ` and also change Fetch to use that and any Fetch
> API tests.

Yes, that is probably the most interoperable choice.

> `Access-Control-Request-Headers` seems tricky though, as I believe no
> browser uses a space after the commas there now. Is that problematic?

WebKit is already using `,` here so no problem, except the small inconsistency.

[Anne van Kesteren]

It seems Mozilla is tentatively okay with making this change. I prepared three pull requests to make this setup work:

  https://github.com/whatwg/fetch/pull/504
  https://github.com/whatwg/xhr/pull/130
  https://github.com/w3c/web-platform-tests/pull/5115

If I missed tests that need updating I'd appreciate your help with that. I'd also appreciate any review.

[youenn fablet]

Sure, I'll update WebKit side and upstream any needed change to WPT

[Anne van Kesteren]

(To be clear, I don't know if Mozilla is okay with lowercasing header names and reordering them for getAllResponseHeaders(). Those two items still need to be battle-tested somehow, but aligning on `, ` seems okay.)

[youenn fablet]

Created attachment 304069 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 304069 [details]
Patch

Clearing flags on attachment: 304069

Committed r213766: <http://trac.webkit.org/changeset/213766>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.