fix: support multiple cookies with the same name across different paths and domains

.changeset/fix-multiple-cookies-same-name.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: support multiple cookies with the same name across different paths and domains

.changeset/seven-cougars-train.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: `cookies.get(...)` returns `undefined` for a just-deleted cookie

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

@@ -27,6 +27,21 @@ function validate_options(options) {
 	}
 }
 
+/**
+ * Generates a unique key for a cookie based on its domain, path, and name in
+ * the format: `<domain>/<path>?<name>`.
+ * If domain is undefined, it will be omitted.
+ * For example: `/?name`, `example.com/foo?name`.
+ *
+ * @param {string | undefined} domain
+ * @param {string} path
+ * @param {string} name
+ * @returns {string}
+ */
+function generate_cookie_key(domain, path, name) {
+	return `${domain || ''}${path}?${encodeURIComponent(name)}`;
+}
+
 /**
  * @param {Request} request
  * @param {URL} url
@@ -38,8 +53,8 @@ export function get_cookies(request, url) {
 	/** @type {string | undefined} */
 	let normalized_url;
 
-	/** @type {Record<string, import('./page/types.js').Cookie>} */
-	const new_cookies = {};
+	/** @type {Map<string, import('./page/types.js').Cookie>} */
+	const new_cookies = new Map();
 
 	/** @type {import('cookie').CookieSerializeOptions} */
 	const defaults = {
@@ -60,13 +75,19 @@ export function get_cookies(request, url) {
 		 * @param {import('cookie').CookieParseOptions} [opts]
 		 */
 		get(name, opts) {
-			const c = new_cookies[name];
-			if (
-				c &&
-				domain_matches(url.hostname, c.options.domain) &&
-				path_matches(url.pathname, c.options.path)
-			) {
-				return c.value;
+			// Look for the most specific matching cookie from new_cookies
+			const best_match = Array.from(new_cookies.values())
+				.filter((c) => {
+					return (
+						c.name === name &&
+						domain_matches(url.hostname, c.options.domain) &&
+						path_matches(url.pathname, c.options.path)
+					);
+				})
+				.sort((a, b) => b.options.path.length - a.options.path.length)[0];
+
+			if (best_match) {
+				return best_match.options.maxAge === 0 ? undefined : best_match.value;
 			}
 
 			const req_cookies = parse(header, { decode: opts?.decode });
@@ -98,15 +119,28 @@ export function get_cookies(request, url) {
 		getAll(opts) {
 			const cookies = parse(header, { decode: opts?.decode });
 
-			for (const c of Object.values(new_cookies)) {
+			// Group cookies by name and find the most specific one for each name
+			const lookup = new Map();
+
+			for (const c of new_cookies.values()) {
 				if (
 					domain_matches(url.hostname, c.options.domain) &&
 					path_matches(url.pathname, c.options.path)
 				) {
-					cookies[c.name] = c.value;
+					const existing = lookup.get(c.name);
+
+					// If no existing cookie or this one has a more specific (longer) path, use this one
+					if (!existing || c.options.path.length > existing.options.path.length) {
+						lookup.set(c.name, c);
+					}
 				}
 			}
 
+			// Add the most specific cookies to the result
+			for (const c of lookup.values()) {
+				cookies[c.name] = c.value;
+			}
+
 			return Object.entries(cookies).map(([name, value]) => ({ name, value }));
 		},
 
@@ -172,8 +206,7 @@ export function get_cookies(request, url) {
 		};
 
 		// cookies previous set during this event with cookies.set have higher precedence
-		for (const key in new_cookies) {
-			const cookie = new_cookies[key];
+		for (const cookie of new_cookies.values()) {
 			if (!domain_matches(destination.hostname, cookie.options.domain)) continue;
 			if (!path_matches(destination.pathname, cookie.options.path)) continue;
 
@@ -214,10 +247,13 @@ export function get_cookies(request, url) {
 			path = resolve(normalized_url, path);
 		}
 
-		new_cookies[name] = { name, value, options: { ...options, path } };
+		// Generate unique key for cookie storage
+		const cookie_key = generate_cookie_key(options.domain, path, name);
+		const cookie = { name, value, options: { ...options, path } };
+		new_cookies.set(cookie_key, cookie);
 
 		if (__SVELTEKIT_DEV__) {
-			const serialized = serialize(name, value, new_cookies[name].options);
+			const serialized = serialize(name, value, cookie.options);
 			if (text_encoder.encode(serialized).byteLength > MAX_COOKIE_SIZE) {
 				throw new Error(`Cookie "${name}" is too large, and will be discarded by the browser`);
 			}
@@ -271,7 +307,7 @@ export function path_matches(path, constraint) {
 
 /**
  * @param {Headers} headers
- * @param {import('./page/types.js').Cookie[]} cookies
+ * @param {MapIterator<import('./page/types.js').Cookie>} cookies
  */
 export function add_cookies_to_headers(headers, cookies) {
 	for (const new_cookie of cookies) {

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

@@ -63,13 +63,13 @@ test('a cookie should not be present after it is deleted', () => {
 	cookies.set('a', 'b', { path: '/' });
 	expect(cookies.get('a')).toEqual('b');
 	cookies.delete('a', { path: '/' });
-	assert.isNotOk(cookies.get('a'));
+	assert.isUndefined(cookies.get('a'));
 });
 
 test('default values when set is called', () => {
 	const { cookies, new_cookies } = cookies_setup();
 	cookies.set('a', 'b', { path: '/' });
-	const opts = new_cookies['a']?.options;
+	const opts = new_cookies.get('/?a')?.options;
 	assert.equal(opts?.secure, true);
 	assert.equal(opts?.httpOnly, true);
 	assert.equal(opts?.path, '/');
@@ -79,7 +79,7 @@ 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', { path: '' });
-	const opts = new_cookies['a']?.options;
+	const opts = new_cookies.get('/foo/bar?a')?.options;
 	assert.equal(opts?.secure, true);
 	assert.equal(opts?.httpOnly, true);
 	assert.equal(opts?.path, '/foo/bar');
@@ -89,14 +89,14 @@ 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', { path: '/' });
-	const opts = new_cookies['a']?.options;
+	const opts = new_cookies.get('/?a')?.options;
 	assert.equal(opts?.secure, false);
 });
 
 test('overridden defaults when set is called', () => {
 	const { cookies, new_cookies } = cookies_setup();
 	cookies.set('a', 'b', { secure: false, httpOnly: false, sameSite: 'strict', path: '/a/b/c' });
-	const opts = new_cookies['a']?.options;
+	const opts = new_cookies.get('/a/b/c?a')?.options;
 	assert.equal(opts?.secure, false);
 	assert.equal(opts?.httpOnly, false);
 	assert.equal(opts?.path, '/a/b/c');
@@ -106,7 +106,7 @@ test('overridden defaults when set is called', () => {
 test('default values when delete is called', () => {
 	const { cookies, new_cookies } = cookies_setup();
 	cookies.delete('a', { path: '/' });
-	const opts = new_cookies['a']?.options;
+	const opts = new_cookies.get('/?a')?.options;
 	assert.equal(opts?.secure, true);
 	assert.equal(opts?.httpOnly, true);
 	assert.equal(opts?.path, '/');
@@ -117,7 +117,7 @@ test('default values when delete is called', () => {
 test('overridden defaults when delete is called', () => {
 	const { cookies, new_cookies } = cookies_setup();
 	cookies.delete('a', { secure: false, httpOnly: false, sameSite: 'strict', path: '/a/b/c' });
-	const opts = new_cookies['a']?.options;
+	const opts = new_cookies.get('/a/b/c?a')?.options;
 	assert.equal(opts?.secure, false);
 	assert.equal(opts?.httpOnly, false);
 	assert.equal(opts?.path, '/a/b/c');
@@ -128,15 +128,15 @@ test('overridden defaults when delete is called', () => {
 test('cannot override maxAge on delete', () => {
 	const { cookies, new_cookies } = cookies_setup();
 	cookies.delete('a', { path: '/', maxAge: 1234 });
-	const opts = new_cookies['a']?.options;
+	const opts = new_cookies.get('/?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', { path: '/' });
 	cookies.set('a', 'bar', { path: '/' });
-	const entry = new_cookies['a'];
+	const entry = new_cookies.get('/?a');
 	assert.equal(entry?.value, 'bar');
 });
 
@@ -145,8 +145,8 @@ test('cookie names are case sensitive', () => {
 	// not that one should do this, but we follow the spec...
 	cookies.set('a', 'foo', { path: '/' });
 	cookies.set('A', 'bar', { path: '/' });
-	const entrya = new_cookies['a'];
-	const entryA = new_cookies['A'];
+	const entrya = new_cookies.get('/?a');
+	const entryA = new_cookies.get('/?A');
 	assert.equal(entrya?.value, 'foo');
 	assert.equal(entryA?.value, 'bar');
 });
@@ -211,5 +211,76 @@ test("set_internal isn't affected by defaults", () => {
 	set_internal('test', 'foo', options);
 
 	expect(cookies.get('test')).toEqual('foo');
-	expect(new_cookies['test']?.options).toEqual(options);
+	expect(new_cookies.get('/a/b/c?test')?.options).toEqual(options);
+});
+
+test('reproduce issue #13947: multiple cookies with same name but different paths', () => {
+	// Test on root path to see if most specific cookie wins
+	const { cookies: root_cookies } = cookies_setup({ href: 'https://example.com/' });
+	root_cookies.set('key', 'value_root', { path: '/' });
+	root_cookies.set('key', 'value_foo', { path: '/foo' });
+
+	// When on root path, should get the root cookie
+	expect(root_cookies.get('key')).toEqual('value_root');
+
+	// Test on /foo path to see if more specific cookie wins
+	const { cookies: foo_cookies } = cookies_setup({ href: 'https://example.com/foo' });
+	foo_cookies.set('key', 'value_root', { path: '/' });
+	foo_cookies.set('key', 'value_foo', { path: '/foo' });
+
+	// When on /foo path, should get the more specific /foo cookie
+	expect(foo_cookies.get('key')).toEqual('value_foo');
+});
+
+test('cookies with same name but different domains work correctly', () => {
+	// Test setting cookies with different domains (unique key storage should work)
+	const { cookies, new_cookies } = cookies_setup({ href: 'https://example.com/' });
+
+	cookies.set('key', 'value1', { path: '/', domain: 'example.com' });
+	cookies.set('key', 'value2', { path: '/', domain: 'sub.example.com' });
+
+	// Both cookies should be stored with unique keys
+	expect(new_cookies.get('example.com/?key')).toBeDefined();
+	expect(new_cookies.get('sub.example.com/?key')).toBeDefined();
+	expect(new_cookies.get('example.com/?key')?.value).toEqual('value1');
+	expect(new_cookies.get('sub.example.com/?key')?.value).toEqual('value2');
+});
+
+test('cookie path specificity: more specific paths win', () => {
+	const { cookies } = cookies_setup({ href: 'https://example.com/x/y/z' });
+
+	// Set cookies with increasing path specificity
+	cookies.set('n', '1', { path: '/' });
+	cookies.set('n', '2', { path: '/x' });
+	cookies.set('n', '3', { path: '/x/y' });
+	cookies.set('n', '4', { path: '/x/y/z' });
+
+	// Most specific path should win
+	expect(cookies.get('n')).toEqual('4');
+});
+
+test('backward compatibility: get without domain/path options still works', () => {
+	const { cookies } = cookies_setup();
+
+	// Set a cookie the old way
+	cookies.set('old-style', 'value', { path: '/' });
+
+	// Should be retrievable without specifying path
+	expect(cookies.get('old-style')).toEqual('value');
+});
+
+test('getAll should return most specific cookie when multiple cookies have same name', () => {
+	// This test demonstrates the bug: getAll doesn't respect path specificity like get() does
+	const { cookies } = cookies_setup({ href: 'https://example.com/foo/bar' });
+
+	// Set cookies with the same name but different path specificity
+	// Setting most specific first, then less specific ones to expose the bug
+	cookies.set('duplicate', 'foobar_value', { path: '/foo/bar' }); // Most specific
+	cookies.set('duplicate', 'root_value', { path: '/' }); // Least specific
+	cookies.set('duplicate', 'foo_value', { path: '/foo' }); // Middle specificity
+
+	const all = cookies.getAll();
+	const duplicate = all.find((c) => c.name === 'duplicate');
+
+	expect(duplicate?.value).toEqual('foobar_value');
 });

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

@@ -432,7 +432,7 @@ export async function internal_respond(request, options, manifest, state) {
 													response.headers.set(key, /** @type {string} */ (value));
 												}
 
-												add_cookies_to_headers(response.headers, Object.values(new_cookies));
+												add_cookies_to_headers(response.headers, new_cookies.values());
 
 												if (state.prerendering && event.route.id !== null) {
 													response.headers.set('x-sveltekit-routeid', encodeURI(event.route.id));
@@ -507,7 +507,7 @@ export async function internal_respond(request, options, manifest, state) {
 				: route?.page && is_action_json_request(event)
 					? action_json_redirect(e)
 					: redirect_response(e.status, e.location);
-			add_cookies_to_headers(response.headers, Object.values(new_cookies));
+			add_cookies_to_headers(response.headers, new_cookies.values());
 			return response;
 		}
 		return await handle_fatal_error(event, event_state, options, e);