[miniflare] fix: preserve multiple Set-Cookie headers on WebSocket 101 upgrade responses#14330
[miniflare] fix: preserve multiple Set-Cookie headers on WebSocket 101 upgrade responses#14330matingathani wants to merge 7 commits into
Conversation
…1 upgrade responses Iterating a `Headers` object with `for…of` collapses all `Set-Cookie` values into a single comma-joined string. This corrupted cookies whose attributes contain commas (e.g. `Expires=Wed, 09 Jun 2026 …`), meaning clients received a single mangled `Set-Cookie` line instead of two separate cookies. Fix: use `extra.getSetCookie()` first to emit each cookie as its own header line, then iterate the rest of the headers with `set-cookie` skipped. Mirrors the approach already in packages/vite-plugin-cloudflare/src/websockets.ts. Fixes cloudflare#14145
🦋 Changeset detectedLatest commit: 146471d The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
workers-devprod
requested review from
a team and
penalosa
and removed request for
a team
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Fix TS2322: use block body in onTestFinished callback so server.close() return value is not implicitly returned as Awaitable<void> - Also close WebSocketServer in cleanup to release all handles - Fix comment: first cookie (not second) has the Expires comma - Drop `fix:` prefix from changeset title per AGENTS.md convention
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
`getSetCookie()` is not part of the standard Fetch API. Guard with `typeof extra.getSetCookie === "function"` before calling it, matching the same defensive check in packages/vite-plugin-cloudflare/src/websockets.ts.
| // Preserve multiple Set-Cookie values verbatim — iterating `Headers` | ||
| // with `for…of` collapses them into a single comma-joined string | ||
| // (web-platform limitation), which corrupts cookies containing commas | ||
| // (e.g. `Expires=Wed, 09 Jun 2026 …`). `getSetCookie` is not part of | ||
| // the standard Fetch API, so guard before calling it. |
There was a problem hiding this comment.
🚩 Undici 7.x may already handle Set-Cookie separately in iteration
The changeset and code comment state that iterating a Headers object with for...of collapses Set-Cookie values into a single comma-joined string. This was true for older implementations, but undici 7.x follows the updated Fetch spec (since March 2023) where Headers[Symbol.iterator]() yields each Set-Cookie value as a separate entry. This means the bug being fixed might only have manifested with older undici versions or specific workerd Headers implementations. The fix using getSetCookie() is still the most robust approach and explicitly correct regardless of iteration behavior, so this doesn't affect correctness — but the comment's characterization of the problem may not accurately describe the current runtime behavior.
Was this helpful? React with 👍 or 👎 to provide feedback.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…' into fix/miniflare-websocket-set-cookie
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Fixes #14145.
Iterating a
Headersobject withfor…ofcollapses allSet-Cookievalues into a single comma-joined string — a known web-platform limitation. When a Worker (or custom service binding) returned multipleSet-Cookieheaders on a 101 upgrade response, the cookies were emitted as a single mangled line instead of separateSet-Cookielines. This corrupts cookies whose attributes contain commas (e.g.Expires=Wed, 09 Jun 2026 10:18:14 GMT), causing clients to receive an unparseable cookie.Fix: use
extra.getSetCookie()to emit each cookie as its own header line, then iterate the remaining headers withset-cookieskipped. This mirrors the approach already inpackages/vite-plugin-cloudflare/src/websockets.ts(landed in #14117).