From c1a02fceb9dad2819a31b2f5b408247858756260 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 8 Dec 2023 17:34:19 -0500 Subject: [PATCH 1/3] deprecate pathless cookie, warn on path: "" --- packages/kit/src/runtime/server/cookie.js | 26 +++++++++++++++ .../kit/src/runtime/server/cookie.spec.js | 32 +++++++++---------- packages/kit/src/runtime/server/utils.js | 14 ++++++++ 3 files changed, 56 insertions(+), 16 deletions(-) diff --git a/packages/kit/src/runtime/server/cookie.js b/packages/kit/src/runtime/server/cookie.js index 90fb9e7c126e..c364504f3ced 100644 --- a/packages/kit/src/runtime/server/cookie.js +++ b/packages/kit/src/runtime/server/cookie.js @@ -1,5 +1,6 @@ import { parse, serialize } from 'cookie'; import { normalize_path } from '../../utils/url.js'; +import { warn_with_callsite } from './utils.js'; /** * Tracks all cookies set during dev mode so we can emit warnings @@ -14,6 +15,27 @@ const cookie_paths = {}; */ const MAX_COOKIE_SIZE = 4129; +/** + * + * @param {import('cookie').CookieSerializeOptions} opts + * @param {'set' | 'delete'} method + */ +function deprecate_missing_path(opts, method) { + if (opts.path === undefined) { + warn_with_callsite( + `Calling \`cookies.${method}}(...)\` without specifying a \`path\` is deprecated, and will be disallowed in SvelteKit 2.0. Relative paths can be used`, + 1 + ); + } + + if (opts.path === '') { + warn_with_callsite( + `Calling \`cookies.${method}(...)\` with \`path: ''\` will behave differently in SvelteKit 2.0. Instead of using the browser default behaviour, it will set the cookie path to the current pathname`, + 1 + ); + } +} + /** * @param {Request} request * @param {URL} url @@ -107,6 +129,7 @@ export function get_cookies(request, url, trailing_slash) { * @param {import('cookie').CookieSerializeOptions} opts */ set(name, value, opts = {}) { + deprecate_missing_path(opts, 'set'); set_internal(name, value, { ...defaults, ...opts }); }, @@ -115,7 +138,10 @@ export function get_cookies(request, url, trailing_slash) { * @param {import('cookie').CookieSerializeOptions} opts */ delete(name, opts = {}) { + deprecate_missing_path(opts, 'delete'); + cookies.set(name, '', { + path: default_path, // TODO 2.0 remove this ...opts, maxAge: 0 }); diff --git a/packages/kit/src/runtime/server/cookie.spec.js b/packages/kit/src/runtime/server/cookie.spec.js index 0b703fdc6480..52f3052c281c 100644 --- a/packages/kit/src/runtime/server/cookie.spec.js +++ b/packages/kit/src/runtime/server/cookie.spec.js @@ -58,15 +58,15 @@ const cookies_setup = ({ href, headers } = {}) => { test('a cookie should not be present after it is deleted', () => { const { cookies } = cookies_setup(); - cookies.set('a', 'b'); + cookies.set('a', 'b', { path: '/' }); expect(cookies.get('a')).toEqual('b'); - cookies.delete('a'); + cookies.delete('a', { path: '/' }); assert.isNotOk(cookies.get('a')); }); test('default values when set is called', () => { const { cookies, new_cookies } = cookies_setup(); - cookies.set('a', 'b'); + cookies.set('a', 'b', { path: '/' }); const opts = new_cookies['a']?.options; assert.equal(opts?.secure, true); assert.equal(opts?.httpOnly, true); @@ -86,7 +86,7 @@ test('default values when set is called on sub path', () => { test('default values when on localhost', () => { const { cookies, new_cookies } = cookies_setup({ href: 'http://localhost:1234' }); - cookies.set('a', 'b'); + cookies.set('a', 'b', { path: '/' }); const opts = new_cookies['a']?.options; assert.equal(opts?.secure, false); }); @@ -103,7 +103,7 @@ test('overridden defaults when set is called', () => { test('default values when delete is called', () => { const { cookies, new_cookies } = cookies_setup(); - cookies.delete('a'); + cookies.delete('a', { path: '/' }); const opts = new_cookies['a']?.options; assert.equal(opts?.secure, true); assert.equal(opts?.httpOnly, true); @@ -125,15 +125,15 @@ test('overridden defaults when delete is called', () => { test('cannot override maxAge on delete', () => { const { cookies, new_cookies } = cookies_setup(); - cookies.delete('a', { maxAge: 1234 }); + cookies.delete('a', { path: '/', maxAge: 1234 }); const opts = new_cookies['a']?.options; assert.equal(opts?.maxAge, 0); }); test('last cookie set with the same name wins', () => { const { cookies, new_cookies } = cookies_setup(); - cookies.set('a', 'foo'); - cookies.set('a', 'bar'); + cookies.set('a', 'foo', { path: '/' }); + cookies.set('a', 'bar', { path: '/' }); const entry = new_cookies['a']; assert.equal(entry?.value, 'bar'); }); @@ -141,8 +141,8 @@ test('last cookie set with the same name wins', () => { test('cookie names are case sensitive', () => { const { cookies, new_cookies } = cookies_setup(); // not that one should do this, but we follow the spec... - cookies.set('a', 'foo'); - cookies.set('A', 'bar'); + cookies.set('a', 'foo', { path: '/' }); + cookies.set('A', 'bar', { path: '/' }); const entrya = new_cookies['a']; const entryA = new_cookies['A']; assert.equal(entrya?.value, 'foo'); @@ -157,8 +157,8 @@ test('serialized cookie header should be url-encoded', () => { cookie: 'a=f%C3%BC; b=foo+bar' // a=fü } }); - cookies.set('c', 'fö'); // should use default encoding - cookies.set('d', 'fö', { encode: () => 'öf' }); // should respect `encode` + cookies.set('c', 'fö', { path: '/' }); // should use default encoding + cookies.set('d', 'fö', { path: '/', encode: () => 'öf' }); // should respect `encode` const header = get_cookie_header(new URL(href), 'e=f%C3%A4; f=foo+bar'); assert.equal(header, 'a=f%C3%BC; b=foo+bar; c=f%C3%B6; d=öf; e=f%C3%A4; f=foo+bar'); }); @@ -169,7 +169,7 @@ test('warns if cookie exceeds 4,129 bytes', () => { try { const { cookies } = cookies_setup(); - cookies.set('a', 'a'.repeat(4097)); + cookies.set('a', 'a'.repeat(4097), { path: '/' }); } catch (e) { const error = /** @type {Error} */ (e); @@ -184,9 +184,9 @@ test('get all cookies from header and set calls', () => { const { cookies } = cookies_setup(); expect(cookies.getAll()).toEqual([{ name: 'a', value: 'b' }]); - cookies.set('a', 'foo'); - cookies.set('a', 'bar'); - cookies.set('b', 'baz'); + cookies.set('a', 'foo', { path: '/' }); + cookies.set('a', 'bar', { path: '/' }); + cookies.set('b', 'baz', { path: '/' }); expect(cookies.getAll()).toEqual([ { name: 'a', value: 'bar' }, diff --git a/packages/kit/src/runtime/server/utils.js b/packages/kit/src/runtime/server/utils.js index 6e5a4ee1b6ad..8605076494c6 100644 --- a/packages/kit/src/runtime/server/utils.js +++ b/packages/kit/src/runtime/server/utils.js @@ -159,3 +159,17 @@ export function stringify_uses(node) { return `"uses":{${uses.join(',')}}`; } + +/** + * @param {string} message + * @param {number} offset + */ +export function warn_with_callsite(message, offset = 0) { + if (DEV) { + const stack = fix_stack_trace(new Error()).split('\n'); + const line = stack.at(3 + offset); + message += `\n${line}`; + } + + console.warn(message); +} From df8aa4977f159592135f2162e4fafb5b2c90a96a Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 8 Dec 2023 17:37:27 -0500 Subject: [PATCH 2/3] changeset --- .changeset/real-carrots-share.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/real-carrots-share.md diff --git a/.changeset/real-carrots-share.md b/.changeset/real-carrots-share.md new file mode 100644 index 000000000000..673cac4b602e --- /dev/null +++ b/.changeset/real-carrots-share.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': major +--- + +chore: deprecate cookies.set/delete without path option From 9f030d7c00b80f907ccc9d29521558fa3ecb7bd0 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 8 Dec 2023 19:20:02 -0500 Subject: [PATCH 3/3] Update .changeset/real-carrots-share.md --- .changeset/real-carrots-share.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/real-carrots-share.md b/.changeset/real-carrots-share.md index 673cac4b602e..c2c17f804b29 100644 --- a/.changeset/real-carrots-share.md +++ b/.changeset/real-carrots-share.md @@ -1,5 +1,5 @@ --- -'@sveltejs/kit': major +'@sveltejs/kit': patch --- chore: deprecate cookies.set/delete without path option