fix: cache mechanism for request with different headers (#8754)

* Fix cache mechanism for request with different headers.

* add headers to build_selector hash;

* update serialize_data to include req headers;

* remove data-hash if req data is an empty obj;

* fix failing tests and update harcoded hash;

* add new test;

* update hash function to support multiple params;

* fmt

* simplify

* remove runtime assertion, use for-of loop instead of forEach

* simplify

* simplify test

* revert hash changes

* Create silent-planes-smell.md

---------

Co-authored-by: Rich Harris <[email protected]>

Author: alejogschwind

.changeset/silent-planes-smell.md

@@ -0,0 +1,5 @@
+---
+"@sveltejs/kit": patch
+---
+
+fix: consider headers when constructing request hash

packages/kit/src/runtime/client/fetcher.js

@@ -127,8 +127,19 @@ function build_selector(resource, opts) {
 
 	let selector = `script[data-sveltekit-fetched][data-url=${url}]`;
 
-	if (opts?.body && (typeof opts.body === 'string' || ArrayBuffer.isView(opts.body))) {
-		selector += `[data-hash="${hash(opts.body)}"]`;
+	if (opts?.headers || opts?.body) {
+		/** @type {import('types').StrictBody[]} */
+		const values = [];
+
+		if (opts.headers) {
+			values.push([...new Headers(opts.headers)].join(','));
+		}
+
+		if (opts.body && (typeof opts.body === 'string' || ArrayBuffer.isView(opts.body))) {
+			values.push(opts.body);
+		}
+
+		selector += `[data-hash="${hash(...values)}"]`;
 	}
 
 	return selector;

packages/kit/src/runtime/hash.js

@@ -1,19 +1,21 @@
 /**
  * Hash using djb2
- * @param {import('types').StrictBody} value
+ * @param {import('types').StrictBody[]} values
  */
-export function hash(value) {
+export function hash(...values) {
 	let hash = 5381;
 
-	if (typeof value === 'string') {
-		let i = value.length;
-		while (i) hash = (hash * 33) ^ value.charCodeAt(--i);
-	} else if (ArrayBuffer.isView(value)) {
-		const buffer = new Uint8Array(value.buffer, value.byteOffset, value.byteLength);
-		let i = buffer.length;
-		while (i) hash = (hash * 33) ^ buffer[--i];
-	} else {
-		throw new TypeError('value must be a string or TypedArray');
+	for (const value of values) {
+		if (typeof value === 'string') {
+			let i = value.length;
+			while (i) hash = (hash * 33) ^ value.charCodeAt(--i);
+		} else if (ArrayBuffer.isView(value)) {
+			const buffer = new Uint8Array(value.buffer, value.byteOffset, value.byteLength);
+			let i = buffer.length;
+			while (i) hash = (hash * 33) ^ buffer[--i];
+		} else {
+			throw new TypeError('value must be a string or TypedArray');
+		}
 	}
 
 	return (hash >>> 0).toString(36);

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

@@ -194,6 +194,7 @@ export function create_universal_fetch(event, state, fetched, csr, resolve_opts)
 									? await stream_to_string(cloned_body)
 									: init?.body
 							),
+							request_headers: init?.headers,
 							response_body: body,
 							response: response
 						});

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

@@ -73,8 +73,19 @@ export function serialize_data(fetched, filter, prerendering = false) {
 		`data-url=${escape_html_attr(fetched.url)}`
 	];
 
-	if (fetched.request_body) {
-		attrs.push(`data-hash=${escape_html_attr(hash(fetched.request_body))}`);
+	if (fetched.request_headers || fetched.request_body) {
+		/** @type {import('types').StrictBody[]} */
+		const values = [];
+
+		if (fetched.request_headers) {
+			values.push([...new Headers(fetched.request_headers)].join(','));
+		}
+
+		if (fetched.request_body) {
+			values.push(fetched.request_body);
+		}
+
+		attrs.push(`data-hash="${hash(...values)}"`);
 	}
 
 	// Compute the time the response should be cached, taking into account max-age and age.

packages/kit/src/runtime/server/page/types.d.ts

@@ -5,6 +5,7 @@ export interface Fetched {
 	url: string;
 	method: string;
 	request_body?: string | ArrayBufferView | null;
+	request_headers?: HeadersInit | undefined;
 	response_body: string;
 	response: Response;
 }

packages/kit/test/apps/basics/src/routes/load/fetch-cache-control/+page.svelte

@@ -1 +1,4 @@
 <a href="/load/fetch-cache-control/load-data">load-data</a>
+
+<a href="/load/fetch-cache-control/headers-diff">headers-diff</a>
+

packages/kit/test/apps/basics/src/routes/load/fetch-cache-control/headers-diff/+page.js

@@ -0,0 +1,18 @@
+export async function load({ fetch, url }) {
+	const r1 = await fetch(url.pathname, {
+		headers: {
+			'x-foo': 'a'
+		}
+	});
+
+	const r2 = await fetch(url.pathname, {
+		headers: {
+			'x-foo': 'b'
+		}
+	});
+
+	return {
+		a: r1.json(),
+		b: r2.json()
+	};
+}

packages/kit/test/apps/basics/src/routes/load/fetch-cache-control/headers-diff/+page.svelte

@@ -0,0 +1,7 @@
+<script>
+	export let data;
+</script>
+
+<a href="/load/fetch-cache-control">fetch-cache-control</a>
+
+<h2>{data.a.foo} / {data.b.foo}</h2>

packages/kit/test/apps/basics/src/routes/load/fetch-cache-control/headers-diff/+server.js

@@ -0,0 +1,12 @@
+import { json } from '@sveltejs/kit';
+
+/** @type {import('./$types').RequestHandler} */
+export async function GET({ request, setHeaders }) {
+	setHeaders({
+		'cache-control': 'public, max-age=7'
+	});
+
+	return json({
+		foo: request.headers.get('x-foo')
+	});
+}

packages/kit/test/apps/basics/test/client.test.js

@@ -193,6 +193,25 @@ test.describe('Load', () => {
 		expect(did_request_data).toBe(false);
 	});
 
+	test('do not use cache if headers are different', async ({ page, clicknav }) => {
+		await page.goto('/load/fetch-cache-control/headers-diff');
+
+		// 1. We expect the right data
+		expect(await page.textContent('h2')).toBe('a / b');
+
+		// 2. Change to another route (client side)
+		await clicknav('[href="/load/fetch-cache-control"]');
+
+		// 3. Come back to the original page (client side)
+		const requests = [];
+		page.on('request', (request) => requests.push(request));
+		await clicknav('[href="/load/fetch-cache-control/headers-diff"]');
+
+		// 4. We expect the same data and no new request because it was cached.
+		expect(await page.textContent('h2')).toBe('a / b');
+		expect(requests).toEqual([]);
+	});
+
 	if (process.env.DEV) {
 		test('using window.fetch causes a warning', async ({ page, baseURL }) => {
 			await Promise.all([

packages/kit/test/apps/basics/test/test.js

@@ -238,10 +238,10 @@ test.describe('Load', () => {
 			const payload_b = '{"status":200,"statusText":"","headers":{},"body":"Y"}';
 			// by the time JS has run, hydration will have nuked these scripts
 			const script_contents_a = await page.innerHTML(
-				'script[data-sveltekit-fetched][data-url="/load/serialization-post.json"][data-hash="3t25"]'
+				'script[data-sveltekit-fetched][data-url="/load/serialization-post.json"][data-hash="1vn6nlx"]'
 			);
 			const script_contents_b = await page.innerHTML(
-				'script[data-sveltekit-fetched][data-url="/load/serialization-post.json"][data-hash="3t24"]'
+				'script[data-sveltekit-fetched][data-url="/load/serialization-post.json"][data-hash="1vn6nlw"]'
 			);
 
 			expect(script_contents_a).toBe(payload_a);