fix: preserve multiple Set-Cookie headers on 304 responses#15902
Conversation
`Headers.get('set-cookie')` collapses multiple values into a single
comma-joined string that browsers cannot parse. Iterate
`Headers.getSetCookie()` and append each cookie individually so all
cookies survive an `If-None-Match` revalidation.
Fixes sveltejs#15527
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: a3c51a2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rich-Harris
left a comment
There was a problem hiding this comment.
Thanks, this makes sense. One wrinkle: getSetCookie was only added to Node 19.7, but SvelteKit supports >=18.13, making this a breaking change.
Should we fall back to splitCookiesString? Or better still, replace all existing occurrences of splitCookiesString with a central (headers: Headers) => string[] utility that uses getSetCookie if available but otherwise falls back to splitCookiesString, with a // TODO reminding us to remove it in v3?
`Headers.getSetCookie` is only available in Node 19.7+, but SvelteKit supports >=18.13. Introduce a central `get_set_cookies` helper that uses `getSetCookie` when present and falls back to `splitCookiesString` otherwise, and route the existing call sites through it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
good idea! updated with a get_set_cookie util now! |
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @sveltejs/kit@2.62.0 ### Minor Changes - feat: support passing Svelte(Kit) config via Vite plugin ([#15944](#15944)) ### Patch Changes - fix: preserve multiple `Set-Cookie` headers on 304 responses ([#15902](#15902)) - fix: preload for anchor elements that were just previously preloaded ([#15915](#15915)) - fix: catch load function streaming errors on the client ([#15929](#15929)) - fix: avoid generating the `_app/env.js` module if public dynamic environment variables are not used by the app ([#15940](#15940)) ## @sveltejs/package@2.5.8 ### Patch Changes - chore: bump `svelte2tsx` dependency to support TypeScript 6 ([#15896](#15896)) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
happy to help! |
Summary
Fixes #15527.
Headers.get('set-cookie')collapses multiple values into a single comma-joined string that browsers cannot parse, so the 304 path was silently dropping all cookies after the first (most visible on non-Node adapters, wheresplitCookiesStringrecovery doesn't exist).Iterates
Headers.getSetCookie()and appends each cookie individually, matching the pattern already used byadd_cookies_to_headersincookie.js.Test plan
etag forwards multiple cookiesexercises the 304 path with three cookiesetag forwards cookiestest still passespnpm -F @sveltejs/kit test:unitpasses