Skip to content

breaking: require path when setting/deleting/serializing cookies #11240

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

Merged
merged 14 commits into from
Dec 10, 2023
5 changes: 5 additions & 0 deletions .changeset/poor-parrots-own.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': major
---

breaking: require path option when setting/deleting/serializing cookies
2 changes: 0 additions & 2 deletions documentation/docs/20-core-concepts/20-load.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export const actions = {
game.guesses[i] += key;
}

cookies.set('sverdle', game.toString());
cookies.set('sverdle', game.toString(), { path: '' });
},

/**
Expand All @@ -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;
20 changes: 14 additions & 6 deletions packages/kit/src/exports/public.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
71 changes: 39 additions & 32 deletions packages/kit/src/runtime/server/cookie.js
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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 = {};
Expand Down Expand Up @@ -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 });
}
};

Expand Down Expand Up @@ -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);
Expand All @@ -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);
}
}
}
Expand Down
42 changes: 21 additions & 21 deletions packages/kit/src/runtime/server/cookie.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
});
Expand All @@ -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);
Expand All @@ -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');
Expand All @@ -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');
});
Expand All @@ -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);

Expand All @@ -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' },
Expand All @@ -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);

Expand Down
13 changes: 7 additions & 6 deletions packages/kit/src/runtime/server/fetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}
*/
Expand Down Expand Up @@ -131,12 +131,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)
});
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/kit/src/runtime/server/page/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,5 @@ export interface CspOpts {
export interface Cookie {
name: string;
value: string;
options: CookieSerializeOptions;
options: CookieSerializeOptions & { path: string };
}
1 change: 1 addition & 0 deletions packages/kit/src/utils/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions packages/kit/src/utils/url.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
Loading