Files
bun.sh/test/regression/issue/23474.test.ts
robobun 525315cf39 fix: include cookies in WebSocket upgrade response (#23499)
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>
2025-10-11 13:34:06 -07:00

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;
});