apply set-cookie headers from page dependencies (#4588)

* apply set-cookie headers from page dependencies - fixes #1198

* hmmm

* deps

* accommodate commonjs laggards

* Update packages/adapter-static/package.json

Co-authored-by: Ben McCann <[email protected]>

* fix lockfile

Co-authored-by: Ben McCann <[email protected]>

Author: Rich-Harris

.changeset/eight-files-think.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+Apply set-cookie headers from page dependencies

packages/adapter-static/package.json

@@ -29,9 +29,11 @@
 	},
 	"devDependencies": {
 		"@sveltejs/kit": "workspace:*",
+		"cookie": "^0.5.0",
 		"devalue": "^2.0.1",
 		"playwright-chromium": "^1.21.0",
 		"port-authority": "^1.1.2",
+		"set-cookie-parser": "^2.4.8",
 		"sirv": "^2.0.0",
 		"svelte": "^3.44.2",
 		"typescript": "^4.6.2",

packages/kit/package.json

@@ -23,6 +23,7 @@
 		"@types/mime": "^2.0.3",
 		"@types/node": "^16.11.11",
 		"@types/sade": "^1.7.3",
+		"@types/set-cookie-parser": "^2.4.2",
 		"amphtml-validator": "^1.0.35",
 		"cookie": "^0.5.0",
 		"cross-env": "^7.0.3",
@@ -36,6 +37,7 @@
 		"port-authority": "^1.1.2",
 		"rollup": "^2.60.2",
 		"selfsigned": "^2.0.0",
+		"set-cookie-parser": "^2.4.8",
 		"sirv": "^2.0.0",
 		"svelte": "^3.44.2",
 		"svelte-check": "^2.5.0",

packages/kit/rollup.config.js

@@ -53,7 +53,8 @@ export default [
 		plugins: [
 			resolve({
 				extensions: ['.mjs', '.js', '.ts']
-			})
+			}),
+			commonjs()
 		]
 	},
 

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

@@ -0,0 +1,25 @@
+/**
+ * @param {string} hostname
+ * @param {string} [constraint]
+ */
+export function domain_matches(hostname, constraint) {
+	if (!constraint) return true;
+
+	const normalized = constraint[0] === '.' ? constraint.slice(1) : constraint;
+
+	if (hostname === normalized) return true;
+	return hostname.endsWith('.' + normalized);
+}
+
+/**
+ * @param {string} path
+ * @param {string} [constraint]
+ */
+export function path_matches(path, constraint) {
+	if (!constraint) return true;
+
+	const normalized = constraint.endsWith('/') ? constraint.slice(0, -1) : constraint;
+
+	if (path === normalized) return true;
+	return path.startsWith(normalized + '/');
+}

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

@@ -0,0 +1,42 @@
+import { test } from 'uvu';
+import * as assert from 'uvu/assert';
+import { domain_matches, path_matches } from './cookie.js';
+
+const domains = {
+	positive: [
+		['localhost'],
+		['example.com', 'example.com'],
+		['sub.example.com', 'example.com'],
+		['example.com', '.example.com'],
+		['sub.example.com', '.example.com']
+	]
+};
+
+const paths = {
+	positive: [['/'], ['/foo', '/'], ['/foo', '/foo'], ['/foo/', '/foo'], ['/foo', '/foo/']],
+
+	negative: [
+		['/', '/foo'],
+		['/food', '/foo']
+	]
+};
+
+domains.positive.forEach(([hostname, constraint]) => {
+	test(`${hostname} / ${constraint}`, () => {
+		assert.ok(domain_matches(hostname, constraint));
+	});
+});
+
+paths.positive.forEach(([path, constraint]) => {
+	test(`${path} / ${constraint}`, () => {
+		assert.ok(path_matches(path, constraint));
+	});
+});
+
+paths.negative.forEach(([path, constraint]) => {
+	test(`! ${path} / ${constraint}`, () => {
+		assert.ok(!path_matches(path, constraint));
+	});
+});
+
+test.run();

packages/kit/src/runtime/server/page/load_node.js

@@ -1,9 +1,12 @@
+import * as cookie from 'cookie';
+import * as set_cookie_parser from 'set-cookie-parser';
 import { normalize } from '../../load.js';
 import { respond } from '../index.js';
 import { is_root_relative, resolve } from '../../../utils/url.js';
 import { create_prerendering_url_proxy } from './utils.js';
 import { is_pojo, lowercase_keys, normalize_request_method } from '../utils.js';
 import { coalesce_to_error } from '../../../utils/error.js';
+import { domain_matches, path_matches } from './cookie.js';
 
 /**
  * @param {{
@@ -41,10 +44,10 @@ export async function load_node({
 	/** @type {Array<import('./types').Fetched>} */
 	const fetched = [];
 
-	/**
-	 * @type {string[]}
-	 */
-	let set_cookie_headers = [];
+	const cookies = cookie.parse(event.request.headers.get('cookie') || '');
+
+	/** @type {import('set-cookie-parser').Cookie[]} */
+	const new_cookies = [];
 
 	/** @type {import('types').LoadOutput} */
 	let loaded;
@@ -60,7 +63,9 @@ export async function load_node({
 		: {};
 
 	if (shadow.cookies) {
-		set_cookie_headers.push(...shadow.cookies);
+		shadow.cookies.forEach((header) => {
+			new_cookies.push(set_cookie_parser.parseString(header));
+		});
 	}
 
 	if (shadow.error) {
@@ -166,9 +171,23 @@ export async function load_node({
 					if (opts.credentials !== 'omit') {
 						uses_credentials = true;
 
-						const cookie = event.request.headers.get('cookie');
 						const authorization = event.request.headers.get('authorization');
 
+						// combine cookies from the initiating request with any that were
+						// added via set-cookie
+						const combined_cookies = { ...cookies };
+
+						for (const cookie of new_cookies) {
+							if (!domain_matches(event.url.hostname, cookie.domain)) continue;
+							if (!path_matches(resolved, cookie.path)) continue;
+
+							combined_cookies[cookie.name] = cookie.value;
+						}
+
+						const cookie = Object.entries(combined_cookies)
+							.map(([name, value]) => `${name}=${value}`)
+							.join('; ');
+
 						if (cookie) {
 							opts.headers.set('cookie', cookie);
 						}
@@ -231,6 +250,15 @@ export async function load_node({
 					response = await options.hooks.externalFetch.call(null, external_request);
 				}
 
+				const set_cookie = response.headers.get('set-cookie');
+				if (set_cookie) {
+					new_cookies.push(
+						...set_cookie_parser
+							.splitCookiesString(set_cookie)
+							.map((str) => set_cookie_parser.parseString(str))
+					);
+				}
+
 				const proxy = new Proxy(response, {
 					get(response, key, _receiver) {
 						async function text() {
@@ -239,9 +267,8 @@ export async function load_node({
 							/** @type {import('types').ResponseHeaders} */
 							const headers = {};
 							for (const [key, value] of response.headers) {
-								if (key === 'set-cookie') {
-									set_cookie_headers = set_cookie_headers.concat(value);
-								} else if (key !== 'etag') {
+								// TODO skip others besides set-cookie and etag?
+								if (key !== 'set-cookie' && key !== 'etag') {
 									headers[key] = value;
 								}
 							}
@@ -362,7 +389,11 @@ export async function load_node({
 		loaded: normalize(loaded),
 		stuff: loaded.stuff || stuff,
 		fetched,
-		set_cookie_headers,
+		set_cookie_headers: new_cookies.map((new_cookie) => {
+			const { name, value, ...options } = new_cookie;
+			// @ts-expect-error
+			return cookie.serialize(name, value, options);
+		}),
 		uses_credentials
 	};
 }

packages/kit/test/apps/basics/src/routes/load/set-cookie-fetch/a.json.js

@@ -0,0 +1,11 @@
+/** @type {import('./a.json').RequestHandler} */
+export function get({ url }) {
+	const answer = url.searchParams.get('answer') || '42';
+
+	return {
+		headers: {
+			'set-cookie': `answer=${answer}; HttpOnly; Path=/load/set-cookie-fetch`
+		},
+		body: {}
+	};
+}

packages/kit/test/apps/basics/src/routes/load/set-cookie-fetch/b.json.js

@@ -0,0 +1,16 @@
+/** @type {import('./b.json').RequestHandler} */
+export function get({ request }) {
+	const cookie = request.headers.get('cookie');
+
+	const match = /answer=([^;]+)/.exec(cookie);
+	const answer = +match?.[1];
+
+	return {
+		body: {
+			answer
+		},
+		headers: {
+			'set-cookie': `doubled=${answer * 2}; HttpOnly; Path=/load/set-cookie-fetch`
+		}
+	};
+}

packages/kit/test/apps/basics/src/routes/load/set-cookie-fetch/index.svelte

@@ -0,0 +1,18 @@
+<script context="module">
+	export async function load({ fetch, url }) {
+		await fetch(`/load/set-cookie-fetch/a.json${url.search}`);
+		const res = await fetch('/load/set-cookie-fetch/b.json');
+		const { answer } = await res.json(); // need to read the response so it gets serialized
+
+		return {
+			props: { answer }
+		};
+	}
+</script>
+
+<script>
+	/** @type {number} */
+	export let answer;
+</script>
+
+<h1>the answer is {answer}</h1>