breaking: require path when setting/deleting/serializing cookies (#…

…11240)

* resolve empty string correctly

* require path when setting/deleting/serializing cookies

* update types and tests

* test for ""

* changeset

* fix

* fix

* lint

* fix

* fix

* fix

* update docs

* only resolve same-domain paths

* this is out of date

---------

Co-authored-by: Rich Harris <[email protected]>

.changeset/poor-parrots-own.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': major
+---
+
+breaking: require path option when setting/deleting/serializing cookies

documentation/docs/20-core-concepts/20-load.md

@@ -283,8 +283,6 @@ For example, if SvelteKit is serving my.domain.com:
 
 Other cookies will not be passed when `credentials: 'include'` is set, because SvelteKit does not know which domain which cookie belongs to (the browser does not pass this information along), so it's not safe to forward any of them. Use the [handleFetch hook](hooks#server-hooks-handlefetch) to work around it.
 
-> When setting cookies, be aware of the `path` property. By default, the `path` of a cookie is the current pathname. If you for example set a cookie at page `admin/user`, the cookie will only be available within the `admin` pages by default. In most cases you likely want to set `path` to `'/'` to make the cookie available throughout your app.
-
 ## Headers
 
 Both server and universal `load` functions have access to a `setHeaders` function that, when running on the server, can set headers for the response. (When running in the browser, `setHeaders` has no effect.) This is useful if you want the page to be cached, for example:

packages/create-svelte/templates/default/src/routes/sverdle/+page.server.ts

@@ -45,7 +45,7 @@ export const actions = {
 			game.guesses[i] += key;
 		}
 
-		cookies.set('sverdle', game.toString());
+		cookies.set('sverdle', game.toString(), { path: '' });
 	},
 
 	/**
@@ -62,10 +62,10 @@ export const actions = {
 			return fail(400, { badGuess: true });
 		}
 
-		cookies.set('sverdle', game.toString());
+		cookies.set('sverdle', game.toString(), { path: '' });
 	},
 
 	restart: async ({ cookies }) => {
-		cookies.delete('sverdle');
+		cookies.delete('sverdle', { path: '' });
 	}
 } satisfies Actions;

packages/kit/src/exports/public.d.ts

@@ -212,34 +212,42 @@ export interface Cookies {
 	 *
 	 * The `httpOnly` and `secure` options are `true` by default (except on http://localhost, where `secure` is `false`), and must be explicitly disabled if you want cookies to be readable by client-side JavaScript and/or transmitted over HTTP. The `sameSite` option defaults to `lax`.
 	 *
-	 * By default, the `path` of a cookie is the 'directory' of the current pathname. In most cases you should explicitly set `path: '/'` to make the cookie available throughout your app.
+	 * You must specify a `path` for the cookie. In most cases you should explicitly set `path: '/'` to make the cookie available throughout your app. You can use relative paths, or set `path: ''` to make the cookie only available on the current path and its children
 	 * @param name the name of the cookie
 	 * @param value the cookie value
 	 * @param opts the options, passed directly to `cookie.serialize`. See documentation [here](https://github.com/jshttp/cookie#cookieserializename-value-options)
 	 */
-	set(name: string, value: string, opts?: import('cookie').CookieSerializeOptions): void;
+	set(
+		name: string,
+		value: string,
+		opts: import('cookie').CookieSerializeOptions & { path: string }
+	): void;
 
 	/**
 	 * Deletes a cookie by setting its value to an empty string and setting the expiry date in the past.
 	 *
-	 * By default, the `path` of a cookie is the 'directory' of the current pathname. In most cases you should explicitly set `path: '/'` to make the cookie available throughout your app.
+	 * You must specify a `path` for the cookie. In most cases you should explicitly set `path: '/'` to make the cookie available throughout your app. You can use relative paths, or set `path: ''` to make the cookie only available on the current path and its children
 	 * @param name the name of the cookie
 	 * @param opts the options, passed directly to `cookie.serialize`. The `path` must match the path of the cookie you want to delete. See documentation [here](https://github.com/jshttp/cookie#cookieserializename-value-options)
 	 */
-	delete(name: string, opts?: import('cookie').CookieSerializeOptions): void;
+	delete(name: string, opts: import('cookie').CookieSerializeOptions & { path: string }): void;
 
 	/**
 	 * Serialize a cookie name-value pair into a `Set-Cookie` header string, but don't apply it to the response.
 	 *
 	 * The `httpOnly` and `secure` options are `true` by default (except on http://localhost, where `secure` is `false`), and must be explicitly disabled if you want cookies to be readable by client-side JavaScript and/or transmitted over HTTP. The `sameSite` option defaults to `lax`.
 	 *
-	 * By default, the `path` of a cookie is the current pathname. In most cases you should explicitly set `path: '/'` to make the cookie available throughout your app.
+	 * You must specify a `path` for the cookie. In most cases you should explicitly set `path: '/'` to make the cookie available throughout your app. You can use relative paths, or set `path: ''` to make the cookie only available on the current path and its children
 	 *
 	 * @param name the name of the cookie
 	 * @param value the cookie value
 	 * @param opts the options, passed directly to `cookie.serialize`. See documentation [here](https://github.com/jshttp/cookie#cookieserializename-value-options)
 	 */
-	serialize(name: string, value: string, opts?: import('cookie').CookieSerializeOptions): string;
+	serialize(
+		name: string,
+		value: string,
+		opts: import('cookie').CookieSerializeOptions & { path: string }
+	): string;
 }
 
 export interface KitConfig {

packages/kit/src/runtime/server/cookie.js

@@ -1,5 +1,5 @@
 import { parse, serialize } from 'cookie';
-import { normalize_path } from '../../utils/url.js';
+import { normalize_path, resolve } from '../../utils/url.js';
 
 /**
  * Tracks all cookies set during dev mode so we can emit warnings
@@ -14,6 +14,14 @@ const cookie_paths = {};
  */
 const MAX_COOKIE_SIZE = 4129;
 
+// TODO 3.0 remove this check
+/** @param {import('./page/types.js').Cookie['options']} options */
+function validate_options(options) {
+	if (options?.path === undefined) {
+		throw new Error('You must specify a `path` when setting, deleting or serializing cookies');
+	}
+}
+
 /**
  * @param {Request} request
  * @param {URL} url
@@ -24,8 +32,6 @@ export function get_cookies(request, url, trailing_slash) {
 	const initial_cookies = parse(header, { decode: (value) => value });
 
 	const normalized_url = normalize_path(url.pathname, trailing_slash);
-	// Emulate browser-behavior: if the cookie is set at '/foo/bar', its path is '/foo'
-	const default_path = normalized_url.split('/').slice(0, -1).join('/') || '/';
 
 	/** @type {Record<string, import('./page/types.js').Cookie>} */
 	const new_cookies = {};
@@ -104,33 +110,37 @@ export function get_cookies(request, url, trailing_slash) {
 		/**
 		 * @param {string} name
 		 * @param {string} value
-		 * @param {import('cookie').CookieSerializeOptions} opts
+		 * @param {import('./page/types.js').Cookie['options']} options
 		 */
-		set(name, value, opts = {}) {
-			set_internal(name, value, { ...defaults, ...opts });
+		set(name, value, options) {
+			validate_options(options);
+			set_internal(name, value, { ...defaults, ...options });
 		},
 
 		/**
 		 * @param {string} name
-		 * @param {import('cookie').CookieSerializeOptions} opts
+		 *  @param {import('./page/types.js').Cookie['options']} options
 		 */
-		delete(name, opts = {}) {
-			cookies.set(name, '', {
-				...opts,
-				maxAge: 0
-			});
+		delete(name, options) {
+			validate_options(options);
+			cookies.set(name, '', { ...options, maxAge: 0 });
 		},
 
 		/**
 		 * @param {string} name
 		 * @param {string} value
-		 * @param {import('cookie').CookieSerializeOptions} opts
+		 *  @param {import('./page/types.js').Cookie['options']} options
 		 */
-		serialize(name, value, opts) {
-			return serialize(name, value, {
-				...defaults,
-				...opts
-			});
+		serialize(name, value, options) {
+			validate_options(options);
+
+			let path = options.path;
+
+			if (!options.domain || options.domain === url.hostname) {
+				path = resolve(normalized_url, path);
+			}
+
+			return serialize(name, value, { ...defaults, ...options, path });
 		}
 	};
 
@@ -171,19 +181,16 @@ export function get_cookies(request, url, trailing_slash) {
 	/**
 	 * @param {string} name
 	 * @param {string} value
-	 * @param {import('cookie').CookieSerializeOptions} opts
+	 * @param {import('./page/types.js').Cookie['options']} options
 	 */
-	function set_internal(name, value, opts) {
-		const path = opts.path ?? default_path;
-
-		new_cookies[name] = {
-			name,
-			value,
-			options: {
-				...opts,
-				path
-			}
-		};
+	function set_internal(name, value, options) {
+		let path = options.path;
+
+		if (!options.domain || options.domain === url.hostname) {
+			path = resolve(normalized_url, path);
+		}
+
+		new_cookies[name] = { name, value, options: { ...options, path } };
 
 		if (__SVELTEKIT_DEV__) {
 			const serialized = serialize(name, value, new_cookies[name].options);
@@ -194,9 +201,9 @@ export function get_cookies(request, url, trailing_slash) {
 			cookie_paths[name] ??= new Set();
 
 			if (!value) {
-				cookie_paths[name].delete(path);
+				cookie_paths[name].delete(options.path);
 			} else {
-				cookie_paths[name].add(path);
+				cookie_paths[name].add(options.path);
 			}
 		}
 	}

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);
@@ -76,17 +76,17 @@ test('default values when set is called', () => {
 
 test('default values when set is called on sub path', () => {
 	const { cookies, new_cookies } = cookies_setup({ href: 'https://example.com/foo/bar' });
-	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);
-	assert.equal(opts?.path, '/foo');
+	assert.equal(opts?.path, '/foo/bar');
 	assert.equal(opts?.sameSite, 'lax');
 });
 
 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,24 +125,24 @@ 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');
 });
 
 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' },
@@ -199,12 +199,12 @@ test("set_internal isn't affected by defaults", () => {
 		href: 'https://example.com/a/b/c'
 	});
 
-	const options = /** @type {const} */ ({
+	const options = {
 		secure: false,
 		httpOnly: false,
-		sameSite: 'none',
+		sameSite: /** @type {const} */ ('none'),
 		path: '/a/b/c'
-	});
+	};
 
 	set_internal('test', 'foo', options);
 

packages/kit/src/runtime/server/fetch.js

@@ -9,7 +9,7 @@ import * as paths from '__sveltekit/paths';
  *   manifest: import('@sveltejs/kit').SSRManifest;
  *   state: import('types').SSRState;
  *   get_cookie_header: (url: URL, header: string | null) => string;
- *   set_internal: (name: string, value: string, opts: import('cookie').CookieSerializeOptions) => void;
+ *   set_internal: (name: string, value: string, opts: import('./page/types.js').Cookie['options']) => void;
  * }} opts
  * @returns {typeof fetch}
  */
@@ -134,12 +134,13 @@ export function create_fetch({ event, options, manifest, state, get_cookie_heade
 					for (const str of set_cookie_parser.splitCookiesString(set_cookie)) {
 						const { name, value, ...options } = set_cookie_parser.parseString(str);
 
+						const path = options.path ?? (url.pathname.split('/').slice(0, -1).join('/') || '/');
+
 						// options.sameSite is string, something more specific is required - type cast is safe
-						set_internal(
-							name,
-							value,
-							/** @type {import('cookie').CookieSerializeOptions} */ (options)
-						);
+						set_internal(name, value, {
+							path,
+							.../** @type {import('cookie').CookieSerializeOptions} */ (options)
+						});
 					}
 				}
 

packages/kit/src/runtime/server/page/types.d.ts

@@ -31,5 +31,5 @@ export interface CspOpts {
 export interface Cookie {
 	name: string;
 	value: string;
-	options: CookieSerializeOptions;
+	options: CookieSerializeOptions & { path: string };
 }

packages/kit/src/utils/url.js

@@ -15,6 +15,7 @@ const absolute = /^([a-z]+:)?\/?\//;
 export function resolve(base, path) {
 	if (SCHEME.test(path)) return path;
 	if (path[0] === '#') return base + path;
+	if (path === '') return base;
 
 	const base_match = absolute.exec(base);
 	const path_match = absolute.exec(path);

packages/kit/src/utils/url.spec.js

@@ -57,6 +57,10 @@ describe('resolve', (test) => {
 	test('resolves data: urls', () => {
 		assert.equal(resolve('/a/b/c', 'data:text/plain,hello'), 'data:text/plain,hello');
 	});
+
+	test('resolves empty string', () => {
+		assert.equal(resolve('/a/b/c', ''), '/a/b/c');
+	});
 });
 
 describe('normalize_path', (test) => {