Skip to content

chore: upgrade to cookie 1.0.2 #13386

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 11 commits into
base: main
Choose a base branch
from
Open
5 changes: 5 additions & 0 deletions .changeset/chilly-berries-press.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

chore: upgrade to cookie 1.0.2
3 changes: 1 addition & 2 deletions packages/kit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@
"homepage": "https://svelte.dev",
"type": "module",
"dependencies": {
"@types/cookie": "^0.6.0",
"cookie": "^0.6.0",
"cookie": "^1.0.2",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you suggested on Discord allowing users to use multiple versions to prevent it from being a breaking change. I think that's a reasonable approach (would probably need to tweak the changeset message slightly in that case as well)

Suggested change
"cookie": "^1.0.2",
"cookie": "^0.6.0 || ^0.7.0 || ^1.0.2",

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% convinced this would actually prevent this from being a breaking change. If someone ran npm up in their project and it grabbed a new version of SvelteKit, it seems likely it would also grab the newest version of cookie in the range - which would mean 1.x. Specifying a range here seems better, but I'm not certain which situations it would actually improve.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be surprising, but at least would give folks a way to opt-out. I'm hoping there aren't many users left using uncommon cookie characters since we've been warning folks about those characters for quite awhile and InLang has updated their library to move away from using a character considered invalid in 1.x.

The bundling route would be cumbersome since we're not already bundling SvelteKit

"devalue": "^5.1.0",
"esm-env": "^1.2.2",
"import-meta-resolve": "^4.1.0",
Expand Down
10 changes: 5 additions & 5 deletions packages/kit/src/exports/public.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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').ParseOptions) => 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').ParseOptions) => 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.
Expand All @@ -236,7 +236,7 @@ export interface Cookies {
set: (
name: string,
value: string,
opts: import('cookie').CookieSerializeOptions & { path: string }
opts: import('cookie').SerializeOptions & { path: string }
) => void;

/**
Expand All @@ -246,7 +246,7 @@ 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').SerializeOptions & { path: string }) => void;

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

Expand Down
33 changes: 14 additions & 19 deletions packages/kit/src/runtime/server/cookie.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import { parse, serialize } from 'cookie';
import { add_data_suffix, normalize_path, resolve } from '../../utils/url.js';

// eslint-disable-next-line no-control-regex -- control characters are invalid in cookie names
const INVALID_COOKIE_CHARACTER_REGEX = /[\x00-\x1F\x7F()<>@,;:"/[\]?={} \t]/;

/**
* Tracks all cookies set during dev mode so we can emit warnings
* when we detect that there's likely cookie misusage due to wrong paths
Expand Down Expand Up @@ -32,14 +29,16 @@ function validate_options(options) {
*/
export function get_cookies(request, url, trailing_slash) {
const header = request.headers.get('cookie') ?? '';
const initial_cookies = parse(header, { decode: (value) => value });
const initial_cookies = /** @type {Record<string, string>} */ (
parse(header, { decode: (value) => value })
);

const normalized_url = normalize_path(url.pathname, trailing_slash);

/** @type {Record<string, import('./page/types.js').Cookie>} */
const new_cookies = {};

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

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

/**
* @param {import('cookie').CookieParseOptions} [opts]
* @param {import('cookie').ParseOptions} [opts]
*/
getAll(opts) {
const cookies = parse(header, { decode: opts?.decode });
Expand All @@ -105,7 +104,11 @@ export function get_cookies(request, url, trailing_slash) {
}
}

return Object.entries(cookies).map(([name, value]) => ({ name, value }));
return /** @type {Array<{ name: string; value: string }>} */ (
Object.entries(cookies)
.filter(([, value]) => value != null)
.map(([name, value]) => ({ name, value }))
);
},

/**
Expand All @@ -114,16 +117,6 @@ export function get_cookies(request, url, trailing_slash) {
* @param {import('./page/types.js').Cookie['options']} options
*/
set(name, value, options) {
// TODO: remove this check in 3.0
const illegal_characters = name.match(INVALID_COOKIE_CHARACTER_REGEX);
if (illegal_characters) {
console.warn(
`The cookie name "${name}" will be invalid in SvelteKit 3.0 as it contains ${illegal_characters.join(
' and '
)}. See RFC 2616 for more details https://datatracker.ietf.org/doc/html/rfc2616#section-2.2`
);
}

validate_options(options);
set_internal(name, value, { ...defaults, ...options });
},
Expand Down Expand Up @@ -178,7 +171,9 @@ export function get_cookies(request, url, trailing_slash) {

// explicit header has highest precedence
if (header) {
const parsed = parse(header, { decode: (value) => value });
const parsed = /** @type {Record<string, string>} */ (
parse(header, { decode: (value) => value })
);
for (const name in parsed) {
combined_cookies[name] = parsed[name];
}
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/src/runtime/server/fetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ export function create_fetch({ event, options, manifest, state, get_cookie_heade
set_internal(name, value, {
path,
encode: (value) => value,
.../** @type {import('cookie').CookieSerializeOptions} */ (options)
.../** @type {import('cookie').SerializeOptions} */ (options)
});
}
}
Expand Down
4 changes: 2 additions & 2 deletions 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 { SerializeOptions } from 'cookie';
import { SSRNode, CspDirectives, ServerDataNode } from 'types';

export interface Fetched {
Expand Down Expand Up @@ -32,5 +32,5 @@ export interface CspOpts {
export interface Cookie {
name: string;
value: string;
options: CookieSerializeOptions & { path: string };
options: SerializeOptions & { path: string };
}
10 changes: 5 additions & 5 deletions packages/kit/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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').ParseOptions) => 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').ParseOptions) => 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.
Expand All @@ -218,7 +218,7 @@ declare module '@sveltejs/kit' {
set: (
name: string,
value: string,
opts: import('cookie').CookieSerializeOptions & { path: string }
opts: import('cookie').SerializeOptions & { path: string }
) => void;

/**
Expand All @@ -228,7 +228,7 @@ 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').SerializeOptions & { path: string }) => void;

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

Expand Down
20 changes: 6 additions & 14 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading