Skip to content

Commit ff4247e

Browse files
Nic-PolumeyvNic PolumeyvRich-Harris
authored
fix: prerender and crawl pages whose content-type header carries a charset parameter (#16567)
The prerenderer compares `content-type` with `===`, so `text/html; charset=utf-8` is not treated as HTML: the page is written without a `.html` extension and its links are never crawled. `is_content_type` already normalizes the header for this reason, from #7195 (strip parameters) and GHSA-gv7g-x59x-wf8f (case-insensitive compare); the two prerender checks predate both. Reproduces #13612, closed as unreproducible, using the charset workaround from #3184. The root `+server.js` error added in #15994 is gated on the same value, so it can fire for an app that has none. Co-authored-by: Nic Polumeyv <nicolas.polum@gmail.com> Co-authored-by: Rich Harris <richard.a.harris@gmail.com>
1 parent 131509a commit ff4247e

8 files changed

Lines changed: 43 additions & 10 deletions

File tree

.changeset/wild-donkeys-listen.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@sveltejs/kit': patch
3+
---
4+
5+
fix: prerender and crawl pages whose `content-type` header carries a `charset` parameter

packages/kit/src/core/postbuild/prerender.js

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { createReadableStream } from '@sveltejs/kit/node';
1717
import generate_fallback from './fallback.js';
1818
import { stringify_remote_arg } from '../../runtime/shared.js';
1919
import { log_response } from '../../exports/vite/utils.js';
20+
import { matches_content_type } from '../../utils/http.js';
2021

2122
export default forked(import.meta.url, prerender);
2223

@@ -387,7 +388,11 @@ async function prerender({ hash, out, manifest_path, metadata, verbose, env, vit
387388
const headers = Object.fromEntries(response.headers);
388389

389390
// if it's a 200 HTML response, crawl it. Skip error responses, as we don't save those
390-
if (response.ok && config.prerender.crawl && headers['content-type'] === 'text/html') {
391+
if (
392+
response.ok &&
393+
config.prerender.crawl &&
394+
matches_content_type(headers['content-type'], 'text/html')
395+
) {
391396
const { ids, hrefs, invalid } = crawl(body.toString(), decoded);
392397

393398
for (const href of invalid) {
@@ -444,7 +449,7 @@ async function prerender({ hash, out, manifest_path, metadata, verbose, env, vit
444449
const headers = Object.fromEntries(response.headers);
445450

446451
const type = headers['content-type'];
447-
const is_html = response_type === REDIRECT || type === 'text/html';
452+
const is_html = response_type === REDIRECT || matches_content_type(type, 'text/html');
448453

449454
if (!is_html && response.status === 200 && decoded.slice(config.paths.base.length + 1) === '') {
450455
throw new Error(

packages/kit/src/utils/http.js

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,13 @@ export function negotiate(accept, types) {
5757
}
5858

5959
/**
60-
* Returns `true` if the request contains a `content-type` header with the given type
61-
* @param {Request} request
60+
* Returns `true` if a `content-type` header value is one of the given types, ignoring
61+
* parameters such as `charset` and comparing case-insensitively
62+
* @param {string | null | undefined} header
6263
* @param {...string} types
6364
*/
64-
function is_content_type(request, ...types) {
65-
const type = request.headers.get('content-type')?.split(';', 1)[0].trim() ?? '';
65+
export function matches_content_type(header, ...types) {
66+
const type = header?.split(';', 1)[0].trim() ?? '';
6667
return types.includes(type.toLowerCase());
6768
}
6869

@@ -72,8 +73,8 @@ function is_content_type(request, ...types) {
7273
export function is_form_content_type(request) {
7374
// These content types must be protected against CSRF
7475
// https://developer.mozilla.org/en-US/docs/Web/API/HTMLFormElement/enctype
75-
return is_content_type(
76-
request,
76+
return matches_content_type(
77+
request.headers.get('content-type'),
7778
'application/x-www-form-urlencoded',
7879
'multipart/form-data',
7980
'text/plain',

packages/kit/src/utils/http.spec.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { assert, test } from 'vitest';
2-
import { negotiate } from './http.js';
2+
import { matches_content_type, negotiate } from './http.js';
33

44
test('handle valid accept header value', () => {
55
const accept = 'text/html';
@@ -18,6 +18,12 @@ test('handle invalid accept header value', () => {
1818
assert.equal(negotiate(accept, ['text/html']), 'text/html');
1919
});
2020

21+
test('matches content types regardless of parameters and casing', () => {
22+
assert.isTrue(matches_content_type('text/html; charset=utf-8', 'text/html'));
23+
assert.isTrue(matches_content_type('TEXT/HTML ; charset=UTF-8', 'text/html'));
24+
assert.isFalse(matches_content_type('text/html', 'text/plain'));
25+
});
26+
2127
test('ignores an accept segment with no slash without catastrophic backtracking', () => {
2228
assert.equal(negotiate('a'.repeat(200_000), ['text/html']), undefined);
2329
}, 100);

packages/kit/test/prerendering/basics/src/hooks.server.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,16 @@ export const handle = async ({ event, resolve }) => {
1212
.replace('__PRERENDERING__', String(building))
1313
});
1414
}
15-
return await resolve(event, {
15+
16+
const response = await resolve(event, {
1617
filterSerializedResponseHeaders: (name) => name === 'content-type'
1718
});
19+
20+
if (event.url.pathname.startsWith('/content-type-charset')) {
21+
response.headers.set('content-type', 'text/html; charset=utf-8');
22+
}
23+
24+
return response;
1825
};
1926

2027
// this code is here to make sure that we kill the process
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
<a href="/content-type-charset/dynamic">Please crawl this</a>
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<h1>
2+
This page will only be discovered if pages whose content-type has a charset parameter are crawled
3+
</h1>

packages/kit/test/prerendering/basics/test/tests.spec.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,11 @@ test('crawls links that start with config.paths.origin', () => {
281281
expect(content).toBeTruthy();
282282
});
283283

284+
test('crawls pages whose content-type has a charset parameter', () => {
285+
expect(read('content-type-charset.html')).toBeTruthy();
286+
expect(read('content-type-charset/dynamic.html')).toBeTruthy();
287+
});
288+
284289
test('identifies missing ids', () => {
285290
const missing_ids_file = fileURLToPath(new URL('../missing_ids/index.jsonl', import.meta.url));
286291
const missing_ids_content = fs.readFileSync(missing_ids_file, 'utf-8');

0 commit comments

Comments
 (0)