Fetch: Use case for Headers getAll

Created on 28 Nov 2019  路  18Comments  路  Source: whatwg/fetch

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.

additioproposal needs implementer interest api

Most helpful comment

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.)

All 18 comments

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

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...

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 ,.

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.

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.)

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.

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.

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.

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.)

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.

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?

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

@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?

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?

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

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...

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

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.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jimmywarting picture jimmywarting  路  3Comments

anforowicz picture anforowicz  路  5Comments

jimmywarting picture jimmywarting  路  10Comments

annevk picture annevk  路  9Comments

jovdb picture jovdb  路  8Comments