Skip to content

refactor: use cookie-es for cookie utils #13512

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
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/cool-chefs-search.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

chore: migrate to cookie-es for cookie parsing and splitting
5 changes: 2 additions & 3 deletions packages/adapter-netlify/package.json
Original file line number Diff line number Diff line change
@@ -42,8 +42,8 @@
},
"dependencies": {
"@iarna/toml": "^2.2.5",
"esbuild": "^0.24.0",
"set-cookie-parser": "^2.6.0"
"cookie-es": "^2.0.0",
"esbuild": "^0.24.0"
},
"devDependencies": {
"@netlify/functions": "^3.0.0",
@@ -53,7 +53,6 @@
"@sveltejs/kit": "workspace:^",
"@sveltejs/vite-plugin-svelte": "^5.0.1",
"@types/node": "^18.19.48",
"@types/set-cookie-parser": "^2.4.7",
"rollup": "^4.14.2",
"typescript": "^5.3.3",
"vitest": "^3.0.1"
4 changes: 2 additions & 2 deletions packages/adapter-netlify/src/headers.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as set_cookie_parser from 'set-cookie-parser';
import { splitSetCookieString } from 'cookie-es';

/**
* Splits headers into two categories: single value and multi value
@@ -18,7 +18,7 @@ export function split_headers(headers) {
headers.forEach((value, key) => {
if (key === 'set-cookie') {
if (!m[key]) m[key] = [];
m[key].push(...set_cookie_parser.splitCookiesString(value));
m[key].push(...splitSetCookieString(value));
} else {
h[key] = value;
}
5 changes: 1 addition & 4 deletions packages/kit/package.json
Original file line number Diff line number Diff line change
@@ -18,24 +18,21 @@
"homepage": "https://svelte.dev",
"type": "module",
"dependencies": {
"@types/cookie": "^0.6.0",
"cookie": "^0.6.0",
"cookie-es": "^2.0.0",
"devalue": "^5.1.0",
"esm-env": "^1.2.2",
"import-meta-resolve": "^4.1.0",
"kleur": "^4.1.5",
"magic-string": "^0.30.5",
"mrmime": "^2.0.0",
"sade": "^1.8.1",
"set-cookie-parser": "^2.6.0",
"sirv": "^3.0.0"
},
"devDependencies": {
"@playwright/test": "^1.44.1",
"@sveltejs/vite-plugin-svelte": "^5.0.1",
"@types/connect": "^3.4.38",
"@types/node": "^18.19.48",
"@types/set-cookie-parser": "^2.4.7",
"dts-buddy": "^0.5.5",
"rollup": "^4.14.2",
"svelte": "^5.2.9",
4 changes: 2 additions & 2 deletions packages/kit/src/exports/node/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { createReadStream } from 'node:fs';
import { Readable } from 'node:stream';
import * as set_cookie_parser from 'set-cookie-parser';
import { splitSetCookieString } from 'cookie-es';
import { SvelteKitError } from '../../runtime/control.js';

/**
@@ -145,7 +145,7 @@ export async function setResponse(res, response) {
res.setHeader(
key,
key === 'set-cookie'
? set_cookie_parser.splitCookiesString(
? splitSetCookieString(
// This is absurd but necessary, TODO: investigate why
/** @type {string}*/ (response.headers.get(key))
)
13 changes: 8 additions & 5 deletions packages/kit/src/exports/public.d.ts
Original file line number Diff line number Diff line change
@@ -215,13 +215,13 @@ export interface Cookies {
* @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)
*/
get: (name: string, opts?: import('cookie').CookieParseOptions) => string | undefined;
get: (name: string, opts?: import('cookie-es').CookieParseOptions) => string | undefined;

/**
* Gets all cookies that were previously set with `cookies.set`, or from the request headers.
* @param opts the options, passed directly to `cookie.parse`. See documentation [here](https://github.com/jshttp/cookie#cookieparsestr-options)
*/
getAll: (opts?: import('cookie').CookieParseOptions) => Array<{ name: string; value: string }>;
getAll: (opts?: import('cookie-es').CookieParseOptions) => Array<{ name: string; value: string }>;

/**
* Sets a cookie. This will add a `set-cookie` header to the response, but also make the cookie available via `cookies.get` or `cookies.getAll` during the current request.
@@ -236,7 +236,7 @@ export interface Cookies {
set: (
name: string,
value: string,
opts: import('cookie').CookieSerializeOptions & { path: string }
opts: import('cookie-es').CookieSerializeOptions & { path: string }
) => void;

/**
@@ -246,7 +246,10 @@ export interface Cookies {
* @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 & { path: string }) => void;
delete: (
name: string,
opts: import('cookie-es').CookieSerializeOptions & { path: string }
) => void;

/**
* Serialize a cookie name-value pair into a `Set-Cookie` header string, but don't apply it to the response.
@@ -262,7 +265,7 @@ export interface Cookies {
serialize: (
name: string,
value: string,
opts: import('cookie').CookieSerializeOptions & { path: string }
opts: import('cookie-es').CookieSerializeOptions & { path: string }
) => string;
}

7 changes: 0 additions & 7 deletions packages/kit/src/exports/vite/index.js
Original file line number Diff line number Diff line change
@@ -334,13 +334,6 @@ async function kit({ svelte_config }) {
__SVELTEKIT_EMBEDDED__: kit.embedded ? 'true' : 'false',
__SVELTEKIT_CLIENT_ROUTING__: kit.router.resolution === 'client' ? 'true' : 'false'
};

// These Kit dependencies are packaged as CommonJS, which means they must always be externalized.
// Without this, the tests will still pass but `pnpm dev` will fail in projects that link `@sveltejs/kit`.
/** @type {NonNullable<import('vite').UserConfig['ssr']>} */ (new_config.ssr).external = [
'cookie',
'set-cookie-parser'
];
}

warn_overridden_config(config, new_config);
8 changes: 4 additions & 4 deletions packages/kit/src/runtime/server/cookie.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { parse, serialize } from 'cookie';
import { parse, serialize } from 'cookie-es';
import { normalize_path, resolve } from '../../utils/url.js';
import { add_data_suffix } from '../pathname.js';

@@ -40,7 +40,7 @@ export function get_cookies(request, url, trailing_slash) {
/** @type {Record<string, import('./page/types.js').Cookie>} */
const new_cookies = {};

/** @type {import('cookie').CookieSerializeOptions} */
/** @type {import('cookie-es').CookieSerializeOptions} */
const defaults = {
httpOnly: true,
sameSite: 'lax',
@@ -56,7 +56,7 @@ export function get_cookies(request, url, trailing_slash) {

/**
* @param {string} name
* @param {import('cookie').CookieParseOptions} [opts]
* @param {import('cookie-es').CookieParseOptions} [opts]
*/
get(name, opts) {
const c = new_cookies[name];
@@ -92,7 +92,7 @@ export function get_cookies(request, url, trailing_slash) {
},

/**
* @param {import('cookie').CookieParseOptions} [opts]
* @param {import('cookie-es').CookieParseOptions} [opts]
*/
getAll(opts) {
const cookies = parse(header, { decode: opts?.decode });
10 changes: 4 additions & 6 deletions packages/kit/src/runtime/server/fetch.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as set_cookie_parser from 'set-cookie-parser';
import { splitSetCookieString, parseSetCookie } from 'cookie-es';
import { respond } from './respond.js';
import * as paths from '__sveltekit/paths';
import { read_implementation } from '__sveltekit/server';
@@ -154,18 +154,16 @@ export function create_fetch({ event, options, manifest, state, get_cookie_heade

const set_cookie = response.headers.get('set-cookie');
if (set_cookie) {
for (const str of set_cookie_parser.splitCookiesString(set_cookie)) {
const { name, value, ...options } = set_cookie_parser.parseString(str, {
decodeValues: false
});
for (const str of splitSetCookieString(set_cookie)) {
const { name, value, ...options } = parseSetCookie(str, { decode: false });

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, {
path,
encode: (value) => value,
.../** @type {import('cookie').CookieSerializeOptions} */ (options)
.../** @type {import('cookie-es').CookieSerializeOptions} */ (options)
});
}
}
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
@@ -1,4 +1,4 @@
import { CookieSerializeOptions } from 'cookie';
import { CookieSerializeOptions } from 'cookie-es';
import { SSRNode, CspDirectives, ServerDataNode } from 'types';

export interface Fetched {
13 changes: 8 additions & 5 deletions packages/kit/types/index.d.ts
Original file line number Diff line number Diff line change
@@ -197,13 +197,13 @@ declare module '@sveltejs/kit' {
* @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)
*/
get: (name: string, opts?: import('cookie').CookieParseOptions) => string | undefined;
get: (name: string, opts?: import('cookie-es').CookieParseOptions) => string | undefined;

/**
* Gets all cookies that were previously set with `cookies.set`, or from the request headers.
* @param opts the options, passed directly to `cookie.parse`. See documentation [here](https://github.com/jshttp/cookie#cookieparsestr-options)
*/
getAll: (opts?: import('cookie').CookieParseOptions) => Array<{ name: string; value: string }>;
getAll: (opts?: import('cookie-es').CookieParseOptions) => Array<{ name: string; value: string }>;

/**
* Sets a cookie. This will add a `set-cookie` header to the response, but also make the cookie available via `cookies.get` or `cookies.getAll` during the current request.
@@ -218,7 +218,7 @@ declare module '@sveltejs/kit' {
set: (
name: string,
value: string,
opts: import('cookie').CookieSerializeOptions & { path: string }
opts: import('cookie-es').CookieSerializeOptions & { path: string }
) => void;

/**
@@ -228,7 +228,10 @@ declare module '@sveltejs/kit' {
* @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 & { path: string }) => void;
delete: (
name: string,
opts: import('cookie-es').CookieSerializeOptions & { path: string }
) => void;

/**
* Serialize a cookie name-value pair into a `Set-Cookie` header string, but don't apply it to the response.
@@ -244,7 +247,7 @@ declare module '@sveltejs/kit' {
serialize: (
name: string,
value: string,
opts: import('cookie').CookieSerializeOptions & { path: string }
opts: import('cookie-es').CookieSerializeOptions & { path: string }
) => string;
}

52 changes: 11 additions & 41 deletions pnpm-lock.yaml

Unchanged files with check annotations Beta

expect(page).toHaveURL(offline_url);
});
test('data-sveltekit-preload-data error does not block user navigation', async ({

Check warning on line 961 in packages/kit/test/apps/basics/test/client.test.js

GitHub Actions / test-kit (18, ubuntu-latest, chromium)

flaky test: data-sveltekit-preload-data error does not block user navigation

retries: 2
page,
context,
browserName
test.describe('Filesystem updates', () => {
if (process.env.DEV) {
test('New route is immediately available in dev', async ({ page }) => {

Check warning on line 14 in packages/kit/test/apps/writes/test/test.js

GitHub Actions / test-kit (18, ubuntu-latest, chromium)

flaky test: New route is immediately available in dev

retries: 2
await page.goto('/new-route');
// hash the filename so that it won't conflict with
test.describe.configure({ mode: 'parallel' });
test.describe('a11y', () => {
test('resets focus', async ({ page, clicknav, browserName }) => {

Check warning on line 12 in packages/kit/test/apps/basics/test/cross-platform/client.test.js

GitHub Actions / test-kit-cross-browser (18, windows-2019, chromium, dev)

flaky test: resets focus

retries: 2
const tab = browserName === 'webkit' ? 'Alt+Tab' : 'Tab';
await page.goto('/accessibility/a');
expect(await page.evaluate(() => document.documentElement.getAttribute('tabindex'))).toBe(null);
});
test('applies autofocus after a navigation', async ({ page, clicknav }) => {

Check warning on line 36 in packages/kit/test/apps/basics/test/cross-platform/client.test.js

GitHub Actions / test-kit-cross-browser (18, windows-2019, chromium, dev)

flaky test: applies autofocus after a navigation

retries: 2
await page.goto('/accessibility/autofocus/a');
await clicknav('[href="/accessibility/autofocus/b"]');
}
});
test('Scroll position is correct after going back from a shallow route', async ({

Check warning on line 504 in packages/kit/test/apps/basics/test/cross-platform/client.test.js

GitHub Actions / test-kit-server-side-route-resolution (dev)

flaky test: Scroll position is correct after going back from a shallow route

retries: 2
page,
scroll_to
}) => {
await expect(page.locator('h1')).toHaveText('target: 1');
});
test('responds to <form method="GET"> submission without reload', async ({ page }) => {

Check warning on line 852 in packages/kit/test/apps/basics/test/cross-platform/client.test.js

GitHub Actions / test-kit-server-side-route-resolution (build)

flaky test: responds to <form method="GET"> submission without reload

retries: 2
await page.goto('/routing/form-get');
expect(await page.textContent('h1')).toBe('...');
expect(await page.textContent('h3')).toBe('bar');
});
test('responds to <form target="_blank"> submission with new tab', async ({ page }) => {

Check warning on line 873 in packages/kit/test/apps/basics/test/cross-platform/client.test.js

GitHub Actions / test-kit-cross-browser (18, macOS-latest, webkit, dev)

flaky test: responds to <form target="_blank"> submission with new tab

retries: 2

Check warning on line 873 in packages/kit/test/apps/basics/test/cross-platform/client.test.js

GitHub Actions / test-kit-cross-browser (18, ubuntu-latest, firefox, build)

flaky test: responds to <form target="_blank"> submission with new tab

retries: 2
await page.goto('/routing/form-target-blank');
let tabs = page.context().pages();
expect(tabs.length > 1);
});
test('responds to <button formtarget="_blank" submission with new tab', async ({ page }) => {

Check warning on line 887 in packages/kit/test/apps/basics/test/cross-platform/client.test.js

GitHub Actions / test-kit-cross-browser (18, ubuntu-latest, firefox, build)

flaky test: responds to <button formtarget="_blank" submission with new tab

retries: 2
await page.goto('/routing/form-target-blank');
let tabs = page.context().pages();