Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/chilled-turkeys-leave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@solidjs/start": patch
---

Fix multiple Set-Cookie headers being lost on redirect responses
25 changes: 24 additions & 1 deletion apps/tests/src/e2e/api-call.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});
});
17 changes: 17 additions & 0 deletions apps/tests/src/routes/api/multi-set-cookie-redirect.ts
Original file line number Diff line number Diff line change
@@ -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)
Comment thread
birkskyum marked this conversation as resolved.
setHeader("Set-Cookie", "event_cookie=from_event; Path=/");

// This tests cloning redirect responses with multiple cookies
Comment thread
birkskyum marked this conversation as resolved.
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
});
}
21 changes: 19 additions & 2 deletions packages/start/src/server/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading