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

3.7 KiB

Raw Permalink Blame History

View Raw