Skip to content

Commit ad4dc73

Browse files
CopilotRich-Harris
andcommitted
Fix multiple cookies with same name but different paths/domains
- Add generate_cookie_key function to create unique keys from domain/path/name - Extend cookies.get() to accept optional domain and path options in opts parameter - Store cookies with unique keys to prevent overwrites - Maintain backward compatibility with existing API usage - Add comprehensive tests for new functionality - Address review feedback from PR #14056 Co-authored-by: Rich-Harris <[email protected]>
1 parent 338cd54 commit ad4dc73

File tree

5 files changed

+152
-25
lines changed

5 files changed

+152
-25
lines changed
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
'@sveltejs/kit': patch
3+
---
4+
5+
fix: support multiple cookies with the same name across different paths and domains
6+
7+
Fixes issue where setting multiple cookies with the same name but different paths would overwrite each other. Now cookies are stored with unique keys based on domain, path, and name, allowing proper handling of multiple cookies with identical names but different scopes.
8+
9+
The `cookies.get()` method has been extended to accept optional `domain` and `path` options to retrieve specific cookies:
10+
11+
```js
12+
// Set cookies with same name but different paths
13+
cookies.set('session', 'user1', { path: '/admin' });
14+
cookies.set('session', 'user2', { path: '/api' });
15+
16+
// Retrieve specific cookies
17+
cookies.get('session', { path: '/admin' }); // returns 'user1'
18+
cookies.get('session', { path: '/api' }); // returns 'user2'
19+
```
20+
21+
Backward compatibility is maintained - existing code that doesn't specify domain/path continues to work as before.

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,9 +212,12 @@ export interface Cookies {
212212
/**
213213
* Gets a cookie that was previously set with `cookies.set`, or from the request headers.
214214
* @param name the name of the cookie
215-
* @param opts the options, passed directly to `cookie.parse`. See documentation [here](https://github.com/jshttp/cookie#cookieparsestr-options)
215+
* @param opts the options, domain and path are used to find the cookie, and passed directly to `cookie.parse`. See documentation [here](https://github.com/jshttp/cookie#cookieparsestr-options)
216216
*/
217-
get: (name: string, opts?: import('cookie').CookieParseOptions) => string | undefined;
217+
get: (
218+
name: string,
219+
opts?: import('cookie').CookieParseOptions & { domain?: string; path?: string }
220+
) => string | undefined;
218221

219222
/**
220223
* Gets all cookies that were previously set with `cookies.set`, or from the request headers.

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

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,21 @@ function validate_options(options) {
2626
}
2727
}
2828

29+
/**
30+
* Generates a unique key for a cookie based on its domain, path, and name in
31+
* the format: `<domain>/<path>?<name>`.
32+
* If domain is undefined, it will be omitted.
33+
* For example: `/?name`, `example.com/foo?name`.
34+
*
35+
* @param {string | undefined} domain
36+
* @param {string} path
37+
* @param {string} name
38+
* @returns {string}
39+
*/
40+
function generate_cookie_key(domain, path, name) {
41+
return `${domain || ''}${path}?${name}`;
42+
}
43+
2944
/**
3045
* @param {Request} request
3146
* @param {URL} url
@@ -56,16 +71,33 @@ export function get_cookies(request, url) {
5671

5772
/**
5873
* @param {string} name
59-
* @param {import('cookie').CookieParseOptions} [opts]
74+
* @param {import('cookie').CookieParseOptions & {domain?: string, path?: string}} [opts]
6075
*/
6176
get(name, opts) {
62-
const c = new_cookies[name];
63-
if (
64-
c &&
65-
domain_matches(url.hostname, c.options.domain) &&
66-
path_matches(url.pathname, c.options.path)
67-
) {
68-
return c.value;
77+
// Try to get cookie using the unique key format if domain/path specified
78+
if (opts?.domain !== undefined || opts?.path !== undefined) {
79+
const cookie_key = generate_cookie_key(
80+
opts?.domain,
81+
opts?.path || url?.pathname || '/',
82+
name
83+
);
84+
const c = new_cookies[cookie_key];
85+
if (c) {
86+
// When specifically requesting a cookie by domain/path, we return it directly
87+
// if it exists in our storage, since the key already encodes the path/domain matching
88+
return c.value;
89+
}
90+
}
91+
92+
// Fallback: look for any cookie with this name that matches current domain/path
93+
// This maintains backward compatibility
94+
for (const key in new_cookies) {
95+
const c = new_cookies[key];
96+
if (c && c.name === name &&
97+
domain_matches(url.hostname, c.options.domain) &&
98+
path_matches(url.pathname, c.options.path)) {
99+
return c.value;
100+
}
69101
}
70102

71103
const req_cookies = parse(header, { decode: opts?.decode });
@@ -213,10 +245,12 @@ export function get_cookies(request, url) {
213245
path = resolve(normalized_url, path);
214246
}
215247

216-
new_cookies[name] = { name, value, options: { ...options, path } };
248+
// Generate unique key for cookie storage
249+
const cookie_key = generate_cookie_key(options.domain, path, name);
250+
new_cookies[cookie_key] = { name, value, options: { ...options, path } };
217251

218252
if (__SVELTEKIT_DEV__) {
219-
const serialized = serialize(name, value, new_cookies[name].options);
253+
const serialized = serialize(name, value, new_cookies[cookie_key].options);
220254
if (new TextEncoder().encode(serialized).byteLength > MAX_COOKIE_SIZE) {
221255
throw new Error(`Cookie "${name}" is too large, and will be discarded by the browser`);
222256
}

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

Lines changed: 77 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ test('a cookie should not be present after it is deleted', () => {
6969
test('default values when set is called', () => {
7070
const { cookies, new_cookies } = cookies_setup();
7171
cookies.set('a', 'b', { path: '/' });
72-
const opts = new_cookies['a']?.options;
72+
const opts = new_cookies['/?a']?.options;
7373
assert.equal(opts?.secure, true);
7474
assert.equal(opts?.httpOnly, true);
7575
assert.equal(opts?.path, '/');
@@ -79,7 +79,7 @@ test('default values when set is called', () => {
7979
test('default values when set is called on sub path', () => {
8080
const { cookies, new_cookies } = cookies_setup({ href: 'https://example.com/foo/bar' });
8181
cookies.set('a', 'b', { path: '' });
82-
const opts = new_cookies['a']?.options;
82+
const opts = new_cookies['/foo/bar?a']?.options;
8383
assert.equal(opts?.secure, true);
8484
assert.equal(opts?.httpOnly, true);
8585
assert.equal(opts?.path, '/foo/bar');
@@ -89,14 +89,14 @@ test('default values when set is called on sub path', () => {
8989
test('default values when on localhost', () => {
9090
const { cookies, new_cookies } = cookies_setup({ href: 'http://localhost:1234' });
9191
cookies.set('a', 'b', { path: '/' });
92-
const opts = new_cookies['a']?.options;
92+
const opts = new_cookies['/?a']?.options;
9393
assert.equal(opts?.secure, false);
9494
});
9595

9696
test('overridden defaults when set is called', () => {
9797
const { cookies, new_cookies } = cookies_setup();
9898
cookies.set('a', 'b', { secure: false, httpOnly: false, sameSite: 'strict', path: '/a/b/c' });
99-
const opts = new_cookies['a']?.options;
99+
const opts = new_cookies['/a/b/c?a']?.options;
100100
assert.equal(opts?.secure, false);
101101
assert.equal(opts?.httpOnly, false);
102102
assert.equal(opts?.path, '/a/b/c');
@@ -106,7 +106,7 @@ test('overridden defaults when set is called', () => {
106106
test('default values when delete is called', () => {
107107
const { cookies, new_cookies } = cookies_setup();
108108
cookies.delete('a', { path: '/' });
109-
const opts = new_cookies['a']?.options;
109+
const opts = new_cookies['/?a']?.options;
110110
assert.equal(opts?.secure, true);
111111
assert.equal(opts?.httpOnly, true);
112112
assert.equal(opts?.path, '/');
@@ -117,7 +117,7 @@ test('default values when delete is called', () => {
117117
test('overridden defaults when delete is called', () => {
118118
const { cookies, new_cookies } = cookies_setup();
119119
cookies.delete('a', { secure: false, httpOnly: false, sameSite: 'strict', path: '/a/b/c' });
120-
const opts = new_cookies['a']?.options;
120+
const opts = new_cookies['/a/b/c?a']?.options;
121121
assert.equal(opts?.secure, false);
122122
assert.equal(opts?.httpOnly, false);
123123
assert.equal(opts?.path, '/a/b/c');
@@ -128,15 +128,15 @@ test('overridden defaults when delete is called', () => {
128128
test('cannot override maxAge on delete', () => {
129129
const { cookies, new_cookies } = cookies_setup();
130130
cookies.delete('a', { path: '/', maxAge: 1234 });
131-
const opts = new_cookies['a']?.options;
131+
const opts = new_cookies['/?a']?.options;
132132
assert.equal(opts?.maxAge, 0);
133133
});
134134

135135
test('last cookie set with the same name wins', () => {
136136
const { cookies, new_cookies } = cookies_setup();
137137
cookies.set('a', 'foo', { path: '/' });
138138
cookies.set('a', 'bar', { path: '/' });
139-
const entry = new_cookies['a'];
139+
const entry = new_cookies['/?a'];
140140
assert.equal(entry?.value, 'bar');
141141
});
142142

@@ -145,8 +145,8 @@ test('cookie names are case sensitive', () => {
145145
// not that one should do this, but we follow the spec...
146146
cookies.set('a', 'foo', { path: '/' });
147147
cookies.set('A', 'bar', { path: '/' });
148-
const entrya = new_cookies['a'];
149-
const entryA = new_cookies['A'];
148+
const entrya = new_cookies['/?a'];
149+
const entryA = new_cookies['/?A'];
150150
assert.equal(entrya?.value, 'foo');
151151
assert.equal(entryA?.value, 'bar');
152152
});
@@ -211,5 +211,71 @@ test("set_internal isn't affected by defaults", () => {
211211
set_internal('test', 'foo', options);
212212

213213
expect(cookies.get('test')).toEqual('foo');
214-
expect(new_cookies['test']?.options).toEqual(options);
214+
expect(new_cookies['/a/b/c?test']?.options).toEqual(options);
215+
});
216+
217+
test('reproduce issue #13947: multiple cookies with same name but different paths', () => {
218+
const { cookies } = cookies_setup();
219+
220+
// Set two cookies with the same name but different paths
221+
cookies.set('key', 'value1', { path: '/foo' });
222+
cookies.set('key', 'value2', { path: '/bar' });
223+
224+
// Both cookies should be accessible by specifying their path
225+
expect(cookies.get('key', { path: '/foo' })).toEqual('value1');
226+
expect(cookies.get('key', { path: '/bar' })).toEqual('value2');
227+
});
228+
229+
test('cookies with same name but different domains', () => {
230+
const { cookies } = cookies_setup();
231+
232+
// Set two cookies with the same name but different domains
233+
cookies.set('key', 'value1', { path: '/', domain: 'example.com' });
234+
cookies.set('key', 'value2', { path: '/', domain: 'sub.example.com' });
235+
236+
// Both cookies should be accessible by specifying their domain
237+
expect(cookies.get('key', { domain: 'example.com', path: '/' })).toEqual('value1');
238+
expect(cookies.get('key', { domain: 'sub.example.com', path: '/' })).toEqual('value2');
239+
});
240+
241+
test('cookies with same name but different domain and path combinations', () => {
242+
const { cookies } = cookies_setup();
243+
244+
// Set cookies with same name but different domain/path combinations
245+
cookies.set('key', 'value1', { path: '/foo', domain: 'example.com' });
246+
cookies.set('key', 'value2', { path: '/bar', domain: 'example.com' });
247+
cookies.set('key', 'value3', { path: '/foo', domain: 'sub.example.com' });
248+
249+
// All cookies should be accessible by specifying their domain and path
250+
expect(cookies.get('key', { domain: 'example.com', path: '/foo' })).toEqual('value1');
251+
expect(cookies.get('key', { domain: 'example.com', path: '/bar' })).toEqual('value2');
252+
expect(cookies.get('key', { domain: 'sub.example.com', path: '/foo' })).toEqual('value3');
253+
});
254+
255+
test('backward compatibility: get without domain/path options still works', () => {
256+
const { cookies } = cookies_setup();
257+
258+
// Set a cookie the old way
259+
cookies.set('old-style', 'value', { path: '/' });
260+
261+
// Should be retrievable without specifying path
262+
expect(cookies.get('old-style')).toEqual('value');
263+
});
264+
265+
test('get with only path specified (no domain)', () => {
266+
const { cookies } = cookies_setup();
267+
268+
cookies.set('key', 'value', { path: '/test' });
269+
270+
// Should be retrievable by path only
271+
expect(cookies.get('key', { path: '/test' })).toEqual('value');
272+
});
273+
274+
test('get with only domain specified (no path)', () => {
275+
const { cookies } = cookies_setup();
276+
277+
cookies.set('key', 'value', { path: '/', domain: 'example.com' });
278+
279+
// Should be retrievable by domain only (path defaults to current URL path)
280+
expect(cookies.get('key', { domain: 'example.com' })).toEqual('value');
215281
});

packages/kit/types/index.d.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,9 +189,12 @@ declare module '@sveltejs/kit' {
189189
/**
190190
* Gets a cookie that was previously set with `cookies.set`, or from the request headers.
191191
* @param name the name of the cookie
192-
* @param opts the options, passed directly to `cookie.parse`. See documentation [here](https://github.com/jshttp/cookie#cookieparsestr-options)
192+
* @param opts the options, domain and path are used to find the cookie, and passed directly to `cookie.parse`. See documentation [here](https://github.com/jshttp/cookie#cookieparsestr-options)
193193
*/
194-
get: (name: string, opts?: import('cookie').CookieParseOptions) => string | undefined;
194+
get: (
195+
name: string,
196+
opts?: import('cookie').CookieParseOptions & { domain?: string; path?: string }
197+
) => string | undefined;
195198

196199
/**
197200
* Gets all cookies that were previously set with `cookies.set`, or from the request headers.

0 commit comments

Comments
 (0)