Can't set multiple cookies in response on node 18.9

[eigood]

What version of astro are you using?

1.2.7

Are you using an SSR adapter? If so, which one?

nodejs

What package manager are you using?

npm

What operating system are you using?

Linux/docker

Describe the Bug

I am calling Astro.response.headers.append('set-cookie', $cookieString') multiple times. In the browser, I only see the first call. In the network debugger panel, I see all the set-cookie calls combined together into a single comma separated string.

I found the previous patch that fixed this, however, node 18.9 this no longer works. headers is an instance of 'HeadersList', and there is no 'raw' field.

I am running astro in docker, so I downgraded node to version 16, and then I can set multiple cookies.

While researching this problem late last night, I saw a couple other issues on other projects where node upstream is "doing things" in this area, but I haven't yet had a full chance to dig into this problem further. I'm creating this issue here, as maybe someone else has some input.

I can't create a minimal reproducible problem on astro.new, as I can't do SSR using node on that site. That's why there is random text in that field.

Link to Minimal Reproducible Example

adsfas

Participation

• I am willing to submit a pull request for this issue.

[eigood]

The following SSR APIRoute ends up sending a network response(I used wget --debug in linux to verify), with a header that looks like Set-Cookie: foo=bar, bar=2. node:18.9 docker image is broken, but node:16 docker image works fine.

export async function get({ request, params }) {
  const response = new Response({
    status: 200,
    body: JSON.stringify({}),
  })
  response.headers.append('Set-Cookie', 'foo=bar')
  response.headers.append('Set-Cookie', 'bar=2')
  return response
} 

[matthewp]

Thanks for taking a look! Since you figured out where the problem is, is it possible you can submit a PR?

[eigood]

Thanks for taking a look! Since you figured out where the problem is, is it possible you can submit a PR?

Perhaps, we talked about that, but it would certainly be a few days.

Basically, node 18 is making node-fetch strictly follow the Fetch api. Fetch is a browser-based api, and the issue with multiple set-cookie headers, is that javascript isn't supposed to handle cookies, at all, for security reasons. But, since Astro SSR is a server-side technology, then Astro.response(and .header) being Fetch api means they can't set cookies correctly.

My thoughts on a proper fix, is add Astro.cookie(name, value); this is based express, doing res.cookie(). This would be a breaking change, or new feature, so I'm mentioning it here before going forward.

[matthewp]

Yeah, I'm very familiar with this problem, we have to special-case it to get it to work in a few places. So it sounds like you're saying this isn't fixable in Node 18?

[matthewp]

Is this a dev or prod issue (or both)?

[matthewp]

Note that there is an RFC for a Astro.cookies here: withastro/roadmap#275. We could probably more easily ensure that multiples are set when using this API and deprecate setting them through headers.append().

[eigood]

https://lightrun.com/answers/node-fetch-node-fetch-response-headers-set-multiple-headers-of-same-key-in-response

Yeah, in node 18, headers.raw() does not exist. From what I can see, response.headers itself is a proxy object, using setters/getters. I tried to monkey-patch .raw() onto that, but wasn't allowed to. This is an output=server mode, when src//pages/* attempt to add cookies to the response. We are using the node adapter, for initial testing, but haven't tried any of the other deployment options.

[matthewp]

Thanks @eigood! I think some other runtimes like Deno and Cloudflare hack around this limitation so it might work there. But if it won't work locally that seems like a bit of an issue. I'm going to see if I can push the Astro.cookies idea forward as that would be a better platform-dependent solution.

[matthewp]

Hoping to get Astro.cookies approved this week and if so that will fix this issue.

[matthewp]

Considering this fixed via the Astro.cookies API.