Use case for Headers getAll

[xtuc]

Since https://github.com/whatwg/fetch/commit/42464c8c3d2fd3437a19fc6afd2438a0fd42dde8 Headers.prototype.getAll has been deprecated/removed from the spec and implementations.

I understand that in browsers (including service workers) filtered responses don't include headers that could benefit from the getAll method. However, some JavaScript environment doesn't need to filter response/request headers, like serverless plateforms (for instance Cloudflare Workers) or maybe Nodejs at some point.

For example editing Cookie headers:

const h = new Headers;
h.append("Set-Cookie", "a=1; Expires=Wed, 21 Oct 2015 07:28:00 GMT");
h.append("Set-Cookie", "b=1; Expires=Wed, 21 Oct 2015 07:28:00 GMT");

h.get("Set-Cookie")
// a=1; Expires=Wed, 21 Oct 2015 07:28:00 GMT, b=1; Expires=Wed, 21 Oct 2015 07:28:00 GMT

Spliting the combined header value by , will give an invalid result. Instead getAll could be used to retrieve the individual headers.

[annevk]

Set-Cookie is the only such header, see also HTTP and #506.

It might make sense to expose it properly for code that wants to use this class generically, but I think we ought to allow for implementations of this class to eagerly combine headers generally and store Set-Cookie separately as a special case. That would suggest something like h.getSetCookie() to me.

cc @youennf @ddragana @yutakahirano

[annevk]

It's probably also worth investigating if https://www.npmjs.com/package/set-cookie-parser#splitcookiestringcombinedsetcookieheader is a reasonable alternative to expose. Though if that actually works HTTP would not have to consider cookies a special case...

[xtuc]

Maybe to clarify the environment a bit, Cloudflare Workers boils down to a proxy written in JavaScript and Node.js a HTTP client.

Set-Cookie is the only such header

For such use-cases, it make sense to not filter headers in the response and let the user define its own headers. Any custom headers may contain , .

[annevk]

I disagree, HTTP doesn't guarantee intermediaries won't combine such headers so we shouldn't either. I'd want HTTP to call out more than Set-Cookie as requiring a special data model before going there.

[domenic]

I'm a bit confused why we'd revisit this. Non-browser environments can consider adding nonstandard extensions for whatever purpose they wish, but in browser environments there's no need for this functionality, and our reasoning from the previous discussions seems to hold. (And the spec is written for browser environments.)

[annevk]

We don't forbid appending Set-Cookie unconditionally. When it's appended, there's no way to get it back out per HTTP semantics. That mismatch irks me a bit.

[xtuc]

We are considering adding the getAll method for Cloudflare Workers, I don't know Node.js' plans.

With a basic filtered response you can access almost all the headers. I think this behavior can still happen in a browser.

[annevk]

To be clear, getAll() with the semantics I think you are suggesting would be incompatible with what we'd add here, if anything, as HTTP doesn't require preserving the distinction between

h.append(name, value)
h.append(name, value2)

and

h.append(name, `${value}, ${value2}`)

for any name other than Set-Cookie and this API shouldn't either. See also the discussion in #189.

[kentonv]

Non-browser environments can consider adding nonstandard extensions for whatever purpose they wish, but in browser environments there's no need for this functionality, and our reasoning from the previous discussions seems to hold. (And the spec is written for browser environments.)

In Cloudflare Workers, we'd really like to stick to standard APIs, to promote code portability across a wide range of JavaScript environments. We think it would be a great thing for the JavaScript ecosystem as a whole if common functionality like making HTTP requests actually worked the same everywhere, so it would be nice to see the fetch() standard as applying to more than just browsers.

We have an immediate need to support manipulation of Set-Cookie in Cloudflare Workers. Perhaps ironically, this is actually driven by a request from @rowan-m on the Chrome team related to the default SameSite=Lax change. A lot of people are going to need to work around this soon and doing it in CF Workers will likely be easier for many people than updating their backend code.

We are proceeding with implementing getAll() because although it is deprecated, the past semantics cover our needs, and we're more comfortable implementing something that has been specified in the past than something that has never been specified. The thing we really want to avoid is specifying a new API that turns out incompatible with implementations from other vendors, fragmenting the ecosystem. So while getSetCookie() might be better, getAll() has the advantage that we're pretty confident no one is going to implement it with different semantics. (Yes, I do understand that various platforms may make different decisions about when to automatically combine headers other than Set-Cookie, but I'm comfortable with that.)

[domenic]

I think that's a really unfortunate decision. I think you'd be better served by clearly signposting nonstandard APIs (e.g. using a Cloudflare global), instead of assuming that things which existed briefly in the spec but were never implemented anywhere have some sort of semi-standard semantics.

[kentonv]

I think you'd be better served by clearly signposting nonstandard APIs (e.g. using a Cloudflare global)

I thought vendor prefixes were no longer in favor?

existed briefly in the spec but were never implemented anywhere

Eh? Seems like it was implemented by almost every major browser at one point?

[annevk]

getAll() is not deprecated, it's removed, because its design is broken as explained here and in linked issues.

[kentonv]

@annevk Yes, I understand the issue -- for headers other than Set-Cookie, the header values may be comma-concatenated anyway by any intermediate proxy, so getAll() can't keep its promise.

Do you have a recommendation for how we should proceed, in order to both solve the immediate problem and stay standards-compatible long-term?

[annevk]

Again, not just by any intermediate proxy, by an implementation of the Headers class as well, as at least one implementer strongly argued for.

getSetCookie() still seems like a reasonable solution. @mnot thoughts?

[xtuc]

getSetCookie() sounds good to me now. Could we consider adding it to the browser fetch spec?

[kentonv]

What about getAll(), but it is defined to throw an exception if passed any other header name than "Set-Cookie"?

This has the advantage that if ever there comes along another such header (probably, a non-standard header used by some badly-designed HTTP API that one of our customers really really needs to support), we can easily accommodate...

[annevk]

@youennf @yutakahirano @ddragana @whatwg/http thoughts?

[ddragana]

I looks like we cannot read Set-Cookie if we once have set it, therefore I am for adding some function for this. getSetCookie sound good. I do not like getAll, because it sound like you can use it for every header field, but you cannot. If we want to make it generic getUnmergableHeader or something, but that is only for one header and it is unnecessary, so getSetCookie its fine.

[tonyjmnz]

cross-posting a suggestion from @nfriedly on #506 - use https://www.npmjs.com/package/set-cookie-parser#usage-in-react-native to parse the mangled cookie header

The fact that this inconsistency is still around is mind boggling to me

[lucacasonato]

For future reference, this is how we solve this in Deno's fetch implementation:

1. Setting Set-Cookie headers does not concatenate headers, it will keep them seperate (both in the internal data structure, and on the wire)
2. h.get("Set-Cookie") returns a concatentated list of Set-Cookie headers.
3. Iterating over h (or using h.entries, h.values, or h.keys) will result concatentated headers, as specified except for set-cookie headers. These are returned as is, without concatenation. That means it is possible that h.entries returns multiple items where the header name is set-cookie.

This was the solution of us trying to come up with a solution for multiple years. We deem this breaks the least existing code, while also being relatively easy to understand and use.

[kentonv]

For the record, Cloudflare Workers went ahead with what I suggested in my last comment -- getAll(), but it is only allowed for the specific header name Set-Cookie.

[annevk]

I think what we should define here is a combination of https://github.com/whatwg/fetch/issues/973#issuecomment-561052450 and https://github.com/whatwg/fetch/issues/973#issuecomment-902578584. I.e., getSetCookie() which returns a sequence and the iterator which is special cased for Set-Cookie headers. The constructor already works. The internal data structure also already works.

Firefox is still supportive of adding this. Primarily to ensure that this data structure is actually suitable for all possible use cases.

@youennf @domenic thoughts?

[domenic]

I don't have any real thoughts here. The special iterator behavior seems a bit weird but I'm happy to believe https://github.com/whatwg/fetch/issues/973#issuecomment-902578584 when they say that they tried many things and found that to be the best.

I suspect @jasnell would also be interested in this issue.

[jasnell]

Thanks @domenic ... definitely interested. I'll need to review a bit of the history before I can weigh in tho. I appreciate the heads up.

[youennf]

I am not against a dedicated getter within the Headers class. That said, I'd like to mention the potential complexity for implementors: Cache API as well as exposing a request to service worker fetch event handler might need specific handling to preserve Set-Cookie headers, depending on the actual underlying implementation. It is unclear to me how valuable an addition this is to the web platform (are we talking about setting Set-Cookie to request headers?).

[annevk]

That's a very good point. One potential solution might be to add Set-Cookie to the forbidden header names, but this would have to be tested to see if it's web compatible. I agree that it's not desirable to have Request or Response instances that can contain Set-Cookie headers in their Header objects.

[lucacasonato]

I agree that it's not desirable to have Request or Response instances that can contain Set-Cookie headers in their Header objects.

But only on the web! In server side environments like Deno and Cloudflare Workers, Response objects are used to represent outgoing HTTP responses from the server to the client. These must be able to contain set-cookie headers. On the web Response objects containing a set-cookie headers can already not be created, because Response's headers use a "response" header guard, which forbids "set-cookie" headers. Both Deno and CFW work around this by not considering set-cookie a forbidden response-header name.

For Request objects I have no interest or opinion on set-cookie headers being allowed. They are basically useless, and I don't think anyone would be hurt if the header were to be blocked for requests.

I would be strictly opposed to any changes to the header representation in the spec (even just in Response objects) that would that would cause set-cookie headers to be unsupportable by otherwise conformant implementations.

It is unclear to me how valuable an addition this is to the web platform.

There is no inherent value to the web browsers specifically: set-cookie headers on responses are never exposed in the web platform, so users can not manually use these. The value lies in broader ecosystem compatibility and uniformity. Web browsers are just one of the implementors of the fetch spec: other JS runtimes use this API for HTTP fetches too (e.g. Deno, Cloudflare Workers, Node.js when using node-fetch). If the API that all of these runtimes use is standardised and uniform, it becomes much easier to write code that is portable across runtimes. Reading and setting set-cookie headers is a critical feature for server side HTTP implementations.

[annevk]

@lucacasonato sure, different flavor Request/Response objects could have a different Headers guard. My comment was strictly about what is currently defined in Fetch.

[domenic]

If there's no value in this for the web, I suggest we not merge it into the spec or implement it in browsers.

[lucacasonato]

@domenic I did not phrase that very elegantly. What I meant was that there is no value for the web if this change is looked at in total isolation, pretending that the Headers class is implemented and used only in the browser (ignoring the existence of server side JS runtimes that have the Headers class). This is obviously not the case though. Headers are used in browsers, Deno, Cloudflare Workers, Node.js and various other server side runtimes.

It is correct that the primary users of this specific feature are going to be users of server side JS runtimes. Developers developing exclusively for the web are likely not going to use this. It is critically important however that the API behaves the same in server side runtimes, and browsers. Otherwise packages written for both will need to branch behaviour on the runtime, which is not great.

[...new Headers([["set-cookie": "foo"], ["set-cookie": "bar"]])] should return the same result across browsers and server side runtimes. Not doing so is confusing, and will fragment the ecosystem.

That is the real value for browsers: keeping the API consistent between runtimes, and to avoid user confusion.