undici icon indicating copy to clipboard operation
undici copied to clipboard

Provide a way to access raw headers

Open blittle opened this issue 3 years ago • 5 comments

This would solve...

As outlined at WHATWG/fetch there is often the need in a server environment to access cookie headers.

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

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

The implementation should look like...

There are two solutions currently proposed:

  1. Node-fetch provides a non-standard Headers.prototype.raw method that returns entries for each (including duplicate) header.
  2. Re-introduce Headers.prototype.getAll to return multiple header values.

I have also considered...

  1. Not use undici primitives within NodeJS.
  2. Attempt to monkey patch undici

Additional context

  1. Cloudflare implemented .getAll().
  2. Node-fetch .raw() implementation
  3. Tracking at WinterCG

blittle avatar May 12 '22 14:05 blittle

The current headers implementation would need an overhaul to support this. It stores headers as strings, rather than arrays (which I believe goes against the spec) and doesn't concat values when using .get() (as, well, they're already strings).

I can see adding .getAll(), and like cloudflare, making it only work on set-cookie headers.

edit: Unlike a header list, a Headers object cannot represent more than one Set-Cookie header. In a way this is problematic as unlike all other headers Set-Cookie headers cannot be combined, but since Set-Cookie headers are not exposed to client-side JavaScript this is deemed an acceptable compromise. Implementations could choose the more efficient Headers object representation even for a header list, as long as they also support an associated data structure for Set-Cookie headers.

It looks like undici only needs to store set-cookie headers as arrays and instead allowing .getAll('set-cookie').

KhafraDev avatar May 12 '22 15:05 KhafraDev

Some more additional context, it looks like Deno's implementation is also a bit different:

Deno:

Deno 1.22.2
> Array.from(new Headers([['set-cookie', 'a=b'], ['set-cookie', 'b=c']]))
[ [ "set-cookie", "a=b" ], [ "set-cookie", "b=c" ] ]

Node:

Welcome to Node.js v18.3.0.
> Array.from(new Headers([['set-cookie', 'a=b'], ['set-cookie', 'b=c']]))
[ [ 'set-cookie', 'a=b, b=c' ] ]

Not sure what the "right" thing to do is, just pointing out an implementation detail.

I've been looking at creating an http server library that translates node's http.IncomingMessage to a Request object, and in turn takes a Response object and translates that to node's http.ServerResponse (similar to how Deno's http server works). Perhaps this is a fools errand as it seems the fetch API was not meant to be used in a Server context.

ksmithut avatar Jun 05 '22 15:06 ksmithut

Undici is the spec-compliant way, Deno purposefully doesn't concatenate set-cookie headers.

https://github.com/denoland/deno/blob/bb3387de17953ea1f0558f5f9863b0cd6c59d48c/ext/fetch/20_headers.js#L212-L215

KhafraDev avatar Jun 05 '22 15:06 KhafraDev

Right, they seem to do that because they reuse the Request and Response classes in a server context, not just for usage with fetch. If that's not a goal of undici, then it should probably just stick with the spec.

ksmithut avatar Jun 05 '22 15:06 ksmithut

Headers.prototype.getSetCookie will solve this use case. I don't think another solution would be accepted unless something is decided in the wintercg fetch group.

Deno most likely does this just because it's better for the user; otherwise the array of set-cookie headers wouldn't be exposed.

KhafraDev avatar Jun 05 '22 15:06 KhafraDev