Skip to content

fix: support multiple cookies with the same name across different paths #14056

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/shiny-spoons-stick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: support multiple cookies with the same name across different paths
7 changes: 5 additions & 2 deletions packages/kit/src/exports/public.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,12 @@ export interface Cookies {
/**
* Gets a cookie that was previously set with `cookies.set`, or from the request headers.
* @param name the name of the cookie
* @param opts the options, passed directly to `cookie.parse`. See documentation [here](https://github.com/jshttp/cookie#cookieparsestr-options)
* @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)
*/
get: (name: string, opts?: import('cookie').CookieParseOptions) => string | undefined;
get: (
name: string,
opts?: import('cookie').CookieParseOptions & { domain?: string; path?: string }
) => string | undefined;

/**
* Gets all cookies that were previously set with `cookies.set`, or from the request headers.
Expand Down
31 changes: 27 additions & 4 deletions packages/kit/src/runtime/server/cookie.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,20 @@ function validate_options(options) {
}
}

/**
* Generates a unique key for a cookie based on its domain, path, and name in
* cookies[<domain>/<path>?<name>].
* If the domain or path is undefined, it will be omitted.
* For example, cookies[/?name], cookies['example.com/foo?name'].
*
* @param {string | undefined} domain
* @param {string} path
* @param {string} name
*/
function generate_cookie_key(domain, path, name) {
return `${domain || ''}${path}?${name}`;
}

/**
* @param {Request} request
* @param {URL} url
Expand Down Expand Up @@ -56,10 +70,11 @@ export function get_cookies(request, url) {

/**
* @param {string} name
* @param {import('cookie').CookieParseOptions} [opts]
* @param {import('cookie').CookieParseOptions & {domain?: string, path?: string}} [opts]
*/
get(name, opts) {
const c = new_cookies[name];
const cookie_key = generate_cookie_key(opts?.domain, opts?.path || url?.pathname, name);
const c = new_cookies[cookie_key];
if (
c &&
domain_matches(url.hostname, c.options.domain) &&
Expand Down Expand Up @@ -213,10 +228,18 @@ export function get_cookies(request, url) {
path = resolve(normalized_url, path);
}

new_cookies[name] = { name, value, options: { ...options, path } };
new_cookies[generate_cookie_key(options.domain, path, name)] = {
name,
value,
options: { ...options, path }
};

if (__SVELTEKIT_DEV__) {
const serialized = serialize(name, value, new_cookies[name].options);
const serialized = serialize(
name,
value,
new_cookies[generate_cookie_key(options.domain, path, name)].options
);
if (new TextEncoder().encode(serialized).byteLength > MAX_COOKIE_SIZE) {
throw new Error(`Cookie "${name}" is too large, and will be discarded by the browser`);
}
Expand Down
40 changes: 29 additions & 11 deletions packages/kit/src/runtime/server/cookie.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ test('a cookie should not be present after it is deleted', () => {
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['/?a']?.options;
assert.equal(opts?.secure, true);
assert.equal(opts?.httpOnly, true);
assert.equal(opts?.path, '/');
Expand All @@ -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['/foo/bar?a']?.options;
assert.equal(opts?.secure, true);
assert.equal(opts?.httpOnly, true);
assert.equal(opts?.path, '/foo/bar');
Expand All @@ -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['/?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['/a/b/c?a']?.options;
assert.equal(opts?.secure, false);
assert.equal(opts?.httpOnly, false);
assert.equal(opts?.path, '/a/b/c');
Expand All @@ -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['/?a']?.options;
assert.equal(opts?.secure, true);
assert.equal(opts?.httpOnly, true);
assert.equal(opts?.path, '/');
Expand All @@ -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['/a/b/c?a']?.options;
assert.equal(opts?.secure, false);
assert.equal(opts?.httpOnly, false);
assert.equal(opts?.path, '/a/b/c');
Expand All @@ -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['/?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['/?a'];
assert.equal(entry?.value, 'bar');
});

Expand All @@ -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['/?a'];
const entryA = new_cookies['/?A'];
assert.equal(entrya?.value, 'foo');
assert.equal(entryA?.value, 'bar');
});
Expand Down Expand Up @@ -211,5 +211,23 @@ 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['/a/b/c?test']?.options).toEqual(options);
});

test('set same name in different path', () => {
const { cookies, new_cookies } = cookies_setup();

cookies.set('a', '1', { path: '/foo' });
cookies.set('a', '2', { path: '/bar' });
expect(new_cookies['/bar?a'].name).toEqual('a');
expect(new_cookies['/bar?a'].value).toEqual('2');
expect(new_cookies['/foo?a'].name).toEqual('a');
expect(new_cookies['/foo?a'].value).toEqual('1');
});

test('set cookie from sub domain with path', () => {
const { cookies } = cookies_setup({ href: 'https://sub.example.com/a/b/c' });

cookies.set('foo', 'value', { path: '/a/b/c', domain: 'example.com' });
expect(cookies.get('foo', { domain: 'example.com', path: '/a/b/c' })).toEqual('value');
});
7 changes: 5 additions & 2 deletions packages/kit/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,12 @@ declare module '@sveltejs/kit' {
/**
* Gets a cookie that was previously set with `cookies.set`, or from the request headers.
* @param name the name of the cookie
* @param opts the options, passed directly to `cookie.parse`. See documentation [here](https://github.com/jshttp/cookie#cookieparsestr-options)
* @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)
*/
get: (name: string, opts?: import('cookie').CookieParseOptions) => string | undefined;
get: (
name: string,
opts?: import('cookie').CookieParseOptions & { domain?: string; path?: string }
) => string | undefined;

/**
* Gets all cookies that were previously set with `cookies.set`, or from the request headers.
Expand Down
Loading