chore: deprecate pathless cookie, warn on path: '' (#11237)

This prints a warning if you call cookies.set(...) or cookies.delete(...) without specifying a path, as part of #9299.

Author: Rich-Harris

.changeset/real-carrots-share.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+chore: deprecate cookies.set/delete without path option

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
 			});

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,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' },

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);
+}