Fix multiple Set-Cookie headers being lost on redirect responses (#2024)

Co-authored-by: Brendan Allan <brendonovich@outlook.com>

Author: birkskyum

.changeset/chilled-turkeys-leave.md

@@ -0,0 +1,5 @@
+---
+"@solidjs/start": patch
+---
+
+Fix multiple Set-Cookie headers being lost on redirect responses

apps/tests/src/e2e/api-call.test.ts

@@ -18,5 +18,28 @@ test.describe("api calls", () => {
     expect(redirectResp.headers.get("x-event-header")).toBe("value");
     expect(redirectResp.headers.get("x-return-header")).toBe("value");
     expect(redirectResp.headers.get("x-shared-header")).toBe("event");
-  })
+  });
+
+  test("should preserve multiple Set-Cookie headers on redirect (RFC 6265)", async () => {
+
+    const response = await fetch("http://localhost:3000/api/multi-set-cookie-redirect", {
+      redirect: "manual"
+    });
+
+    expect(response.status).toBe(302);
+
+    // Use getSetCookie() to retrieve all Set-Cookie headers as an array
+    const cookies = response.headers.getSetCookie();
+
+    // We expect 3 cookies:
+    // 1. session=abc123 (from response headers)
+    // 2. csrf=xyz789 (from response headers)
+    // 3. event_cookie=from_event (from event.response headers via setHeader)
+    expect(cookies.length).toBe(3);
+
+    const cookieValues = cookies.join("; ");
+    expect(cookieValues).toContain("session=abc123");
+    expect(cookieValues).toContain("csrf=xyz789");
+    expect(cookieValues).toContain("event_cookie=from_event");
+  });
 });

apps/tests/src/routes/api/multi-set-cookie-redirect.ts

@@ -0,0 +1,17 @@
+import { setHeader } from "@solidjs/start/http";
+
+export async function GET() {
+  // Set a cookie via the event headers (this tests merging event headers)
+  setHeader("Set-Cookie", "event_cookie=from_event; Path=/");
+
+  // This tests cloning redirect responses with multiple cookies
+  const headers = new Headers();
+  headers.append("Location", "http://localhost:3000/");
+  headers.append("Set-Cookie", "session=abc123; Path=/; HttpOnly");
+  headers.append("Set-Cookie", "csrf=xyz789; Path=/");
+
+  return new Response(null, {
+    status: 302,
+    headers
+  });
+}

packages/start/src/server/handler.ts

@@ -235,15 +235,32 @@ function produceResponseWithEventHeaders(res: Response) {
 
   // Response.redirect returns an immutable value, so we clone on any redirect just in case
   if(300 <= res.status && res.status < 400) {
+    const cookies = res.headers.getSetCookie?.() ?? [];
+    const headers = new Headers();
+    res.headers.forEach((value, key) => {
+      if (key.toLowerCase() !== 'set-cookie') {
+        headers.set(key, value);
+      }
+    });
+    for (const cookie of cookies) {
+      headers.append('Set-Cookie', cookie);
+    }
     ret = new Response(res.body, {
       status: res.status,
       statusText: res.statusText,
-      headers: Object.fromEntries(res.headers.entries())
+      headers,
     });
   }
 
+  const eventCookies = event.response.headers.getSetCookie?.() ?? [];
+  for (const cookie of eventCookies) {
+    ret.headers.append('Set-Cookie', cookie);
+  }
+
   for(const [name, value] of event.response.headers) {
-    ret.headers.set(name, value);
+    if (name.toLowerCase() !== 'set-cookie') {
+      ret.headers.set(name, value);
+    }
   }
 
   return ret