Add special handling of `set-cookie` to Headers

[lucacasonato]

trafficstars

Towards #973

• [ ] At least two implementers are interested (and none opposed):
  • Firefox (https://github.com/whatwg/fetch/issues/973#issuecomment-954815921)
  • Deno (https://github.com/whatwg/fetch/issues/973#issuecomment-902578584)
  • Cloudflare Workers
  • Node.js
• [x] Tests are written and can be reviewed and commented upon at:
  • https://github.com/web-platform-tests/wpt/pull/31442
• [ ] Implementation bugs are filed:
  • Chrome: …
  • Firefox: …
  • Safari: …
  • Deno: …

---

Preview | Diff

---

Preview | Diff

[annevk]

Thanks for working on this, this looks quite promising to me. @yutakahirano @ricea @youennf thoughts?

[lucacasonato]

@jasnell @kentonv thoughts?

[kentonv]

I'm fine with this.

I have a weak opinion that, for server-side vendors, getAll() is slightly more flexible: If we have some big customer come along and say "We really need to access this legacy API, but it has a header that misuses commas, which we can't change," we can easily say "No problem, we'll just add your special weird header to the getAll() allow list." On the server side, implementing special requests for high-paying customers are not uncommon. Obviously, on the browser side, such special requests would be complete nonsense, since web sites don't get to choose which browser is used to open them.

But I have seen no evidence that any such headers exist in the wild, so it seems unlikely we'll ever get this exact sort of special request. So, whatever. :)

[annevk]

It's more flexible, but it would also violate HTTP, as would any header that needs it (with the exception of Set-Cookie).

[jimmywarting]

I'm not opposed to adding this into node-fetch either. I will only be glad that we can unite on one unified api that works the same across all platform

[mcollina]

Node.js is ok in implementing this too.

[shadiramadan]

bump - I upgraded node and realized I can't because .raw() was removed and I need to query multiple set cookie headers and that was the only way I was able to.

[andreubotella]

Hi. We're trying to bring this PR back to life and get it landed as part of the WinterCG effort. It seems like the semantics are clear and agreed upon, and that no more work is needed on that area. So it comes down to implementer interest.

@yutakahirano, @youennf. Is Firefox still interested, @smaug----?

[yutakahirano]

cc: @ricea for Chrome

[ricea]

Chrome supports if there is consensus. I don't want to be the only browser implementing this.

[andreubotella]

I've filed implementation bugs for all browsers:

• Chromium: https://crbug.com/1409512
• Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1812511
• WebKit: https://bugs.webkit.org/show_bug.cgi?id=251194

I've also submitted implementations for Chromium and Firefox, and I'm currently working on one for WebKit.

[youennf]

I won't object to this addition but I do not see a lot of value for web browsers to support this. Are there shims around Headers that have been deployed already?

[jasnell]

With the case of the keys() iterator here, would the intention of this PR be to make it so that set-cookie could appear multiple times? Or should the keys() iterator always only produce a unique set? The way this is currently defined, it would appear that the keys iterator could have set-cookie multiple times.

[andreubotella]

With the case of the keys() iterator here, would the intention of this PR be to make it so that set-cookie could appear multiple times? Or should the keys() iterator always only produce a unique set? The way this is currently defined, it would appear that the keys iterator could have set-cookie multiple times.

That seems intentional in the way pair iteration works in WebIDL. That way, [...headers.keys()].length === [...headers.values()].length and the indices match.

Do you think this would break some existing code, though?

[jasnell]

I do think there's a risk of breaking existing code that may be relying on the iterator producing a unique set of keys.

[andreubotella]

I do think there's a risk of breaking existing code that may be relying on the iterator producing a unique set of keys.

Deno's current implementation of headers.keys() will yield "set-cookie" repeatedly in such cases. @lucacasonato can probably speak more on it, but this doesn't seem to have caused any breakage.

[andreubotella]

I have fixed this PR's merge conflicts, and rewritten and moved the Headers implementation note to be about header lists. PTAL.