How to set multiple `Set-Cookie` headers in a single response?

[kentcdodds]

I tried copying concatSetCookieHeaders from remix but that... eh, doesn't work... Doing that I end up with a single set-cookie header.

Here's the relevant code:

const sessionIdCookie = await getUserSessionCookieFromMagicLink(request)
if (sessionIdCookie) {
  const destroyCookie = await loginInfoSession.destroy()
  return redirect('/me', {
    headers: {
      'Set-Cookie': concatSetCookieHeaders(destroyCookie, sessionIdCookie),
    },
  })
}

What I need is two Set-Cookie headers, and this gives me a single Set-Cookie header that doesn't end up updating either one.

If I use either of these cookie values by itself then they each work as expected. But I need to set them both in the same response.

[kentcdodds]

I get an interesting result if I try this:

// import {Headers} from 'remix'

const headers = new Headers()
headers.append('Set-Cookie', destroyCookie)
headers.append('Set-Cookie', sessionIdCookie)
return redirect('/me', {headers})

I still only get a single set-cookie header, and it looks like this:

set-cookie: KCD_login=; Max-Age=1800000; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; SameSite=Lax, KCD_root_session=eyJfX3Nlc3Npb25faWRfXyI6IjYxMTllMDA4LWJhMWMtNDA1Zi05MmViLTdkYmUwYTY2NzY1NCJ9.bbbrkMWekmxsFv1GOP1t%2FIK12L4ZBQcVUnwS54p9nkE; Max-Age=2592000000; Path=/; SameSite=Lax

I'm pretty sure that's exactly the same result as using concatSetCookiesHeaders which is good news at least. Means I don't need to vendor concatSetCookieHeaders :)

But I'm thinking remix has a bug in handling this situation. It should be sending multiple Set-Cookie headers. I'm using the express adapter.

[kentcdodds]

I got it working. Now we're properly setting two cookies :)

However, to make it work, I needed to create these patches 😬

diff --git a/node_modules/@remix-run/express/server.js b/node_modules/@remix-run/express/server.js
index 4fd2142..246a32e 100644
--- a/node_modules/@remix-run/express/server.js
+++ b/node_modules/@remix-run/express/server.js
@@ -73,8 +73,10 @@ function createRemixRequest(req) {
 function sendRemixResponse(res, response) {
   res.status(response.status);
 
-  for (let [key, value] of response.headers.entries()) {
-    res.set(key, value);
+  for (let [key, values] of Object.entries(response.headers.raw())) {
+    for (const value of values) {
+      res.append(key, value);
+    }
   }
 
   if (Buffer.isBuffer(response.body)) {

And one in @remix-run/node/server.js. This patch also has support for build.entry.module.handleDataRequest and I was too lazy to try and figure out how to update the patch content to remove that.

diff --git a/node_modules/@remix-run/node/server.js b/node_modules/@remix-run/node/server.js
index af7406e..93cf72d 100644
--- a/node_modules/@remix-run/node/server.js
+++ b/node_modules/@remix-run/node/server.js
@@ -58,7 +58,7 @@ async function handleDataRequest(request, loadContext, build, routes) {
   try {
     response = isActionRequest(request) ? await data.callRouteAction(build, routeMatch.route.id, clonedRequest, loadContext, routeMatch.params) : await data.loadRouteData(build, routeMatch.route.id, clonedRequest, loadContext, routeMatch.params);
   } catch (error) {
-    return responses.json(errors.serializeError(error), {
+    response = responses.json(errors.serializeError(error), {
       status: 500,
       headers: {
         "X-Remix-Error": "unfortunately, yes"
@@ -66,17 +66,29 @@ async function handleDataRequest(request, loadContext, build, routes) {
     });
   }
 
+  if (build.entry.module.handleDataRequest) {
+    try {
+      response = await build.entry.module.handleDataRequest(clonedRequest, response, loadContext, routeMatch.params);
+    } catch (error) {
+      return responses.json(errors.serializeError(error), {
+        status: 500,
+        headers: {
+          "X-Data-Request-Handler-Error": "Handle your errors, yo."
+        }
+      });
+    }
+  }
+
   if (isRedirectResponse(response)) {
     // We don't have any way to prevent a fetch request from following
     // redirects. So we use the `X-Remix-Redirect` header to indicate the
     // next URL, and then "follow" the redirect manually on the client.
     let locationHeader = response.headers.get("Location");
     response.headers.delete("Location");
+    response.headers.set("X-Remix-Redirect", locationHeader);
     return new nodeFetch.Response("", {
       status: 204,
-      headers: { ...Object.fromEntries(response.headers),
-        "X-Remix-Redirect": locationHeader
-      }
+      headers: response.headers,
     });
   }

As discussed on discord, response.headers.raw() is a node-fetch-only API and not standard. There doesn't appear to be any way to get the individual values of headers. Every way to get values from a header has the values concatenated by ,. Unfortunately we can't simply .split(',') because the Expires value on a cookie can have a , in it (as in ... Expires=Thu, 01 Jan 1970 00:00:00 GMT...).

[kentcdodds]

Another thing that will need to happen is all the templates and docs should update so instead of this:

headers: {
  ...Object.fromEntries(responseHeaders),
  // ...etc
}

They simply set values on the responseHeaders instead. Otherwise multiple header values will be lost.

I just looked and noticed that this doc is correct, so maybe you've already made this change.

responseHeaders.set("Content-Type", "text/html");

  return new Response("<!DOCTYPE html>" + markup, {
    status: responseStatusCode,
    headers: responseHeaders
  });

👍

[kentcdodds]

FYI, I had to add another patch for the @remix-run/node module to support preserving multiple header values in redirects. I updated my comment above.

[mjackson]

Thanks for all the detailed info here, @kentcdodds. I think you've uncovered a significant bug in the way we currently handle headers in Remix. We'll get this fixed very soon 👍

[mcansh]

this is fixed and included once 0.18 ships!

[shripadk]

This doesn't seem to be working when you have a parent loader and nested loader both trying to Set-Cookie. Since according to Remix docs, the nested loader headers override the parent loader headers, it can only be merged in the headers function that is exported. However, there is no way to merge both Set-Cookie headers in there either as the result expected by the function is a Javascript object or instance of Headers Fetch API.

How do I handle this situation? Or am I missing something?

[kiliman]

@shripadk This is a closed issue. I don't think your issue is a bug, but a support question. Can you create a new discussion?

[shripadk]

@kiliman Wasn't sure if this is a bug or a discussion so I posted both. A discussion in Discord as well as a report here. But I am now inclined to think this is definitely a bug.

Tested it out in the root loader (the Response headers contains only 1 Set-Cookie that is concatenating all values instead of setting a Set-Cookie for each cookie):

Network tab: