mirror of
https://github.com/oven-sh/bun
synced 2026-02-09 18:38:55 +00:00
Fixes #23474 ## Summary When `request.cookies.set()` is called before `server.upgrade()`, the cookies are now properly included in the WebSocket upgrade response headers. ## Problem Previously, cookies set on the request via `req.cookies.set()` were only written for regular HTTP responses but were ignored during WebSocket upgrades. Users had to manually pass cookies via the `headers` option: ```js server.upgrade(req, { headers: { "Set-Cookie": `SessionId=${sessionId}`, }, }); ``` ## Solution Modified `src/bun.js/api/server.zig` to check for and write cookies to the WebSocket upgrade response after the "101 Switching Protocols" status is set but before the actual upgrade is performed. The fix handles both cases: - When `upgrade()` is called without custom headers - When `upgrade()` is called with custom headers ## Testing Added comprehensive regression tests in `test/regression/issue/23474.test.ts` that: - Verify cookies are set in the upgrade response without custom headers - Verify cookies are set in the upgrade response with custom headers - Use `Promise.withResolvers()` for efficient async handling (no arbitrary timeouts) Tests confirmed: - ❌ Fail with system bun v1.2.23 (without fix) - ✅ Pass with debug build v1.3.0 (with fix) ## Manual verification ```bash curl -i -H "Connection: Upgrade" -H "Upgrade: websocket" \ -H "Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==" \ -H "Sec-WebSocket-Version: 13" \ http://localhost:3000/ws ``` Response now includes: ``` HTTP/1.1 101 Switching Protocols Set-Cookie: test=123; Path=/; SameSite=Lax Upgrade: websocket Connection: Upgrade ... ``` --------- Co-authored-by: Claude Bot <claude-bot@bun.sh> Co-authored-by: Claude <noreply@anthropic.com>
146 lines
3.7 KiB
TypeScript
146 lines
3.7 KiB
TypeScript
import { expect, test } from "bun:test";
|
|
|
|
test("request.cookies.set() should set websocket upgrade response cookie - issue #23474", async () => {
|
|
using server = Bun.serve({
|
|
port: 0,
|
|
routes: {
|
|
"/ws": req => {
|
|
// Set a cookie before upgrading
|
|
req.cookies.set("test", "123", {
|
|
httpOnly: true,
|
|
path: "/",
|
|
});
|
|
|
|
const upgraded = server.upgrade(req);
|
|
if (upgraded) {
|
|
return undefined;
|
|
}
|
|
return new Response("Upgrade failed", { status: 500 });
|
|
},
|
|
},
|
|
websocket: {
|
|
message(ws, message) {
|
|
ws.close();
|
|
},
|
|
},
|
|
});
|
|
|
|
const { promise, resolve, reject } = Promise.withResolvers();
|
|
|
|
// Use Bun.connect to send a WebSocket upgrade request and check response headers
|
|
const socket = await Bun.connect({
|
|
hostname: "localhost",
|
|
port: server.port,
|
|
socket: {
|
|
data(socket, data) {
|
|
try {
|
|
const response = new TextDecoder().decode(data);
|
|
|
|
// Check that we got a successful upgrade response
|
|
expect(response).toContain("HTTP/1.1 101");
|
|
expect(response).toContain("Upgrade: websocket");
|
|
|
|
// The critical check: Set-Cookie header should be present
|
|
expect(response).toContain("Set-Cookie:");
|
|
expect(response).toContain("test=123");
|
|
|
|
socket.end();
|
|
resolve();
|
|
} catch (err) {
|
|
reject(err);
|
|
}
|
|
},
|
|
error(socket, error) {
|
|
reject(error);
|
|
},
|
|
},
|
|
});
|
|
|
|
// Send a valid WebSocket upgrade request
|
|
socket.write(
|
|
"GET /ws HTTP/1.1\r\n" +
|
|
`Host: localhost:${server.port}\r\n` +
|
|
"Upgrade: websocket\r\n" +
|
|
"Connection: Upgrade\r\n" +
|
|
"Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n" +
|
|
"Sec-WebSocket-Version: 13\r\n" +
|
|
"\r\n",
|
|
);
|
|
|
|
await promise;
|
|
});
|
|
|
|
test("request.cookies.set() should work with custom headers in upgrade - issue #23474", async () => {
|
|
using server = Bun.serve({
|
|
port: 0,
|
|
routes: {
|
|
"/ws": req => {
|
|
// Set cookies before upgrading
|
|
req.cookies.set("session", "abc123", { path: "/" });
|
|
req.cookies.set("user", "john", { httpOnly: true });
|
|
|
|
const upgraded = server.upgrade(req, {
|
|
headers: {
|
|
"X-Custom-Header": "test",
|
|
},
|
|
});
|
|
if (upgraded) {
|
|
return undefined;
|
|
}
|
|
return new Response("Upgrade failed", { status: 500 });
|
|
},
|
|
},
|
|
websocket: {
|
|
message(ws, message) {
|
|
ws.close();
|
|
},
|
|
},
|
|
});
|
|
|
|
const { promise, resolve, reject } = Promise.withResolvers();
|
|
|
|
const socket = await Bun.connect({
|
|
hostname: "localhost",
|
|
port: server.port,
|
|
socket: {
|
|
data(socket, data) {
|
|
try {
|
|
const response = new TextDecoder().decode(data);
|
|
|
|
// Check that we got a successful upgrade response
|
|
expect(response).toContain("HTTP/1.1 101");
|
|
expect(response).toContain("Upgrade: websocket");
|
|
|
|
// Check custom header
|
|
expect(response).toContain("X-Custom-Header: test");
|
|
|
|
// Check that both cookies are present
|
|
expect(response).toContain("Set-Cookie:");
|
|
expect(response).toContain("session=abc123");
|
|
expect(response).toContain("user=john");
|
|
|
|
socket.end();
|
|
resolve();
|
|
} catch (err) {
|
|
reject(err);
|
|
}
|
|
},
|
|
error(socket, error) {
|
|
reject(error);
|
|
},
|
|
},
|
|
});
|
|
|
|
socket.write(
|
|
"GET /ws HTTP/1.1\r\n" +
|
|
`Host: localhost:${server.port}\r\n` +
|
|
"Upgrade: websocket\r\n" +
|
|
"Connection: Upgrade\r\n" +
|
|
"Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n" +
|
|
"Sec-WebSocket-Version: 13\r\n" +
|
|
"\r\n",
|
|
);
|
|
|
|
await promise;
|
|
});
|