Skip to content

fix: cache mechanism for request with different headers #8754

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
Show file tree
Hide file tree
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/silent-planes-smell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@sveltejs/kit": patch
---

fix: consider headers when constructing request hash
15 changes: 13 additions & 2 deletions packages/kit/src/runtime/client/fetcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
24 changes: 13 additions & 11 deletions packages/kit/src/runtime/hash.js
Original file line number Diff line number Diff line change
@@ -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);
Expand Down
1 change: 1 addition & 0 deletions packages/kit/src/runtime/server/page/load_data.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
});
Expand Down
15 changes: 13 additions & 2 deletions packages/kit/src/runtime/server/page/serialize_data.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions packages/kit/src/runtime/server/page/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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>

Original file line number Diff line number Diff line change
@@ -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()
};
}
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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')
});
}
19 changes: 19 additions & 0 deletions packages/kit/test/apps/basics/test/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand Down
4 changes: 2 additions & 2 deletions packages/kit/test/apps/basics/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down