From a0518dadfda9f26034b0b5f72b7f5f3f9212d133 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 18 Aug 2022 14:44:49 +0200 Subject: [PATCH 1/4] [fix] handle set-cookie in setHeaders Fixes #6032 --- .changeset/lemon-kids-double.md | 5 ++++ packages/kit/src/runtime/server/index.js | 24 +++++++++++++++---- .../headers/set-cookie/+layout.server.js | 5 ++++ .../routes/headers/set-cookie/+layout.svelte | 1 + .../headers/set-cookie/sub/+page.server.js | 5 ++++ .../headers/set-cookie/sub/+page.svelte | 0 .../kit/test/apps/basics/test/server.test.js | 8 +++++++ 7 files changed, 44 insertions(+), 4 deletions(-) create mode 100644 .changeset/lemon-kids-double.md create mode 100644 packages/kit/test/apps/basics/src/routes/headers/set-cookie/+layout.server.js create mode 100644 packages/kit/test/apps/basics/src/routes/headers/set-cookie/+layout.svelte create mode 100644 packages/kit/test/apps/basics/src/routes/headers/set-cookie/sub/+page.server.js create mode 100644 packages/kit/test/apps/basics/src/routes/headers/set-cookie/sub/+page.svelte diff --git a/.changeset/lemon-kids-double.md b/.changeset/lemon-kids-double.md new file mode 100644 index 000000000000..e98b9d4d2433 --- /dev/null +++ b/.changeset/lemon-kids-double.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +handle `set-cookie` in `setHeaders` diff --git a/packages/kit/src/runtime/server/index.js b/packages/kit/src/runtime/server/index.js index 625fc053953b..2d3c224e60f1 100644 --- a/packages/kit/src/runtime/server/index.js +++ b/packages/kit/src/runtime/server/index.js @@ -143,12 +143,28 @@ export async function respond(request, options, state) { const lower = key.toLowerCase(); if (lower in headers) { - throw new Error(`"${key}" header is already set`); + if (lower === 'set-cookie') { + if (!Array.isArray(headers[lower])) { + headers[lower] = [/**@type{string} */ (headers[lower])]; + } + const cookies = /**@type{string[]} */ (headers[lower]); + const new_cookies = /**@type{string[]} */ ( + Array.isArray(new_headers[key]) ? new_headers[key] : [new_headers[key]] + ); + for (const new_cookie of new_cookies) { + if (cookies.includes(new_cookie)) { + throw new Error(`"${key}" header already has cookie with same value`); + } + cookies.push(new_cookie); + } + } else { + throw new Error(`"${key}" header is already set`); + } + } else { + // TODO apply these headers to the response <- is this TODO outdated? + headers[lower] = new_headers[key]; } - // TODO apply these headers to the response - headers[lower] = new_headers[key]; - if (state.prerendering && lower === 'cache-control') { state.prerendering.cache = /** @type {string} */ (new_headers[key]); } diff --git a/packages/kit/test/apps/basics/src/routes/headers/set-cookie/+layout.server.js b/packages/kit/test/apps/basics/src/routes/headers/set-cookie/+layout.server.js new file mode 100644 index 000000000000..21fd24121a85 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/headers/set-cookie/+layout.server.js @@ -0,0 +1,5 @@ +export function load({ setHeaders }) { + setHeaders({ + 'set-cookie': 'cookie1=value1' + }); +} diff --git a/packages/kit/test/apps/basics/src/routes/headers/set-cookie/+layout.svelte b/packages/kit/test/apps/basics/src/routes/headers/set-cookie/+layout.svelte new file mode 100644 index 000000000000..0385342cef1b --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/headers/set-cookie/+layout.svelte @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/packages/kit/test/apps/basics/src/routes/headers/set-cookie/sub/+page.server.js b/packages/kit/test/apps/basics/src/routes/headers/set-cookie/sub/+page.server.js new file mode 100644 index 000000000000..1d7acff07996 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/headers/set-cookie/sub/+page.server.js @@ -0,0 +1,5 @@ +export function load({ setHeaders }) { + setHeaders({ + 'set-cookie': 'cookie2=value2' + }); +} diff --git a/packages/kit/test/apps/basics/src/routes/headers/set-cookie/sub/+page.svelte b/packages/kit/test/apps/basics/src/routes/headers/set-cookie/sub/+page.svelte new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/packages/kit/test/apps/basics/test/server.test.js b/packages/kit/test/apps/basics/test/server.test.js index 45615bcb5630..20c2243778b6 100644 --- a/packages/kit/test/apps/basics/test/server.test.js +++ b/packages/kit/test/apps/basics/test/server.test.js @@ -314,6 +314,14 @@ test.describe('Static files', () => { }); }); +test.describe('setHeaders', () => { + test('allows multiple set-cookie headers with different values', async ({ page }) => { + const response = await page.goto('/headers/set-cookie/sub'); + const cookies = (await response?.allHeaders())['set-cookie']; + expect(cookies.includes('cookie1=value1') && cookies.includes('cookie2=value2')).toBe(true); + }); +}); + test.describe('Miscellaneous', () => { test('does not serve version.json with an immutable cache header', async ({ request }) => { // this isn't actually a great test, because caching behaviour is down to adapters. From c90cb714594b29bce787b09794ba296337e4cece Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Thu, 18 Aug 2022 16:22:11 +0200 Subject: [PATCH 2/4] Update packages/kit/src/runtime/server/index.js Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> --- packages/kit/src/runtime/server/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/kit/src/runtime/server/index.js b/packages/kit/src/runtime/server/index.js index 2d3c224e60f1..8e4caff7b20a 100644 --- a/packages/kit/src/runtime/server/index.js +++ b/packages/kit/src/runtime/server/index.js @@ -145,10 +145,10 @@ export async function respond(request, options, state) { if (lower in headers) { if (lower === 'set-cookie') { if (!Array.isArray(headers[lower])) { - headers[lower] = [/**@type{string} */ (headers[lower])]; + headers[lower] = [/** @type{string} */ (headers[lower])]; } - const cookies = /**@type{string[]} */ (headers[lower]); - const new_cookies = /**@type{string[]} */ ( + const cookies = /** @type{string[]} */ (headers[lower]); + const new_cookies = /** @type{string[]} */ ( Array.isArray(new_headers[key]) ? new_headers[key] : [new_headers[key]] ); for (const new_cookie of new_cookies) { From 092d5454b1744deb53fec17726b21e4e269347da Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 18 Aug 2022 11:34:30 -0400 Subject: [PATCH 3/4] Update packages/kit/src/runtime/server/index.js --- packages/kit/src/runtime/server/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/kit/src/runtime/server/index.js b/packages/kit/src/runtime/server/index.js index 8e4caff7b20a..7af1e2bed208 100644 --- a/packages/kit/src/runtime/server/index.js +++ b/packages/kit/src/runtime/server/index.js @@ -161,7 +161,6 @@ export async function respond(request, options, state) { throw new Error(`"${key}" header is already set`); } } else { - // TODO apply these headers to the response <- is this TODO outdated? headers[lower] = new_headers[key]; } From 07edfd47c34daeb571c4888dbd88a38b2b0ac5a1 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 18 Aug 2022 11:46:22 -0400 Subject: [PATCH 4/4] separate out cookie handling more explicitly --- packages/kit/src/runtime/server/index.js | 56 ++++++++++++------------ 1 file changed, 27 insertions(+), 29 deletions(-) diff --git a/packages/kit/src/runtime/server/index.js b/packages/kit/src/runtime/server/index.js index 7af1e2bed208..61d4033a8e68 100644 --- a/packages/kit/src/runtime/server/index.js +++ b/packages/kit/src/runtime/server/index.js @@ -118,6 +118,9 @@ export async function respond(request, options, state) { /** @type {import('types').ResponseHeaders} */ const headers = {}; + /** @type {string[]} */ + const cookies = []; + /** @type {import('types').RequestEvent} */ const event = { get clientAddress() { @@ -141,31 +144,26 @@ export async function respond(request, options, state) { setHeaders: (new_headers) => { for (const key in new_headers) { const lower = key.toLowerCase(); + const value = new_headers[key]; - if (lower in headers) { - if (lower === 'set-cookie') { - if (!Array.isArray(headers[lower])) { - headers[lower] = [/** @type{string} */ (headers[lower])]; - } - const cookies = /** @type{string[]} */ (headers[lower]); - const new_cookies = /** @type{string[]} */ ( - Array.isArray(new_headers[key]) ? new_headers[key] : [new_headers[key]] - ); - for (const new_cookie of new_cookies) { - if (cookies.includes(new_cookie)) { - throw new Error(`"${key}" header already has cookie with same value`); - } - cookies.push(new_cookie); + if (lower === 'set-cookie') { + const new_cookies = /** @type {string[]} */ (Array.isArray(value) ? value : [value]); + + for (const cookie of new_cookies) { + if (cookies.includes(cookie)) { + throw new Error(`"${key}" header already has cookie with same value`); } - } else { - throw new Error(`"${key}" header is already set`); + + cookies.push(cookie); } + } else if (lower in headers) { + throw new Error(`"${key}" header is already set`); } else { - headers[lower] = new_headers[key]; - } + headers[lower] = value; - if (state.prerendering && lower === 'cache-control') { - state.prerendering.cache = /** @type {string} */ (new_headers[key]); + if (state.prerendering && lower === 'cache-control') { + state.prerendering.cache = /** @type {string} */ (value); + } } } }, @@ -327,19 +325,19 @@ export async function respond(request, options, state) { : await render_page(event, route, options, state, resolve_opts); } - for (const key in headers) { - const value = headers[key]; - if (key === 'set-cookie') { - for (const cookie of Array.isArray(value) ? value : [value]) { - response.headers.append(key, /** @type {string} */ (cookie)); - } - } else if (!is_data_request) { - // we only want to set cookies on __data.json requests, we don't - // want to cache stuff erroneously etc + if (!is_data_request) { + // we only want to set cookies on __data.json requests, we don't + // want to cache stuff erroneously etc + for (const key in headers) { + const value = headers[key]; response.headers.set(key, /** @type {string} */ (value)); } } + for (const cookie of cookies) { + response.headers.append('set-cookie', cookie); + } + // respond with 304 if etag matches if (response.status === 200 && response.headers.has('etag')) { let if_none_match_value = request.headers.get('if-none-match');