Skip to content
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

Add filterSerializedResponseHeaders function #6569

Merged
merged 8 commits into from
Sep 5, 2022
Merged
Show file tree
Hide file tree
Changes from 3 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-cycles-reply.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

[breaking] exclude headers from serialized responses by default, add filterSerializedResponseHeaders resolve option
Rich-Harris marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 1 addition & 1 deletion documentation/docs/05-load.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ export async function load({ depends }) {
- it can be used to make credentialed requests on the server, as it inherits the `cookie` and `authorization` headers for the page request
- it can make relative requests on the server (ordinarily, `fetch` requires a URL with an origin when used in a server context)
- internal requests (e.g. for `+server.js` routes) go direct to the handler function when running on the server, without the overhead of an HTTP call
- during server-side rendering, the response will be captured and inlined into the rendered HTML
- during server-side rendering, the response will be captured and inlined into the rendered HTML. Note that headers will _not_ be serialized, unless explicitly included via [`filterSerializedResponseHeaders`](/docs/hooks#handle)
Copy link
Member

Choose a reason for hiding this comment

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

Are we not automatically whitelisting any headers anymore? I remember in some version of this proposal, we would automatically include content-type. That's gone?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was whitelisted because some tests expected it, but that seems like a bad reason to keep it. Things like response.json() work the same way with or without content-type, so the only time you'd actually need it is if you're explicitly reading that header.

I did wonder about filtering the headers during SSR so that you'd get a 'btw you need to include this header' error

Copy link
Member Author

Choose a reason for hiding this comment

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

(though now that I think about it i have no idea if it's possible to monkey-patch response.headers like that)

Copy link
Member Author

Choose a reason for hiding this comment

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

the monkey has been patched

- during hydration, the response will be read from the HTML, guaranteeing consistency and preventing an additional network request

> Cookies will only be passed through if the target host is the same as the SvelteKit application or a more specific subdomain of it.
Expand Down
4 changes: 3 additions & 1 deletion documentation/docs/06-hooks.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,15 @@ You can add call multiple `handle` functions with [the `sequence` helper functio
`resolve` also supports a second, optional parameter that gives you more control over how the response will be rendered. That parameter is an object that can have the following fields:

- `transformPageChunk(opts: { html: string, done: boolean }): MaybePromise<string | undefined>` — applies custom transforms to HTML. If `done` is true, it's the final chunk. Chunks are not guaranteed to be well-formed HTML (they could include an element's opening tag but not its closing tag, for example) but they will always be split at sensible boundaries such as `%sveltekit.head%` or layout/page components.
- `filterSerializedResponseHeaders(name: string, value: string): boolean` — determines which headers should be included in serialized responses when a `load` function loads a resource with `fetch`. By default, none will be included
Rich-Harris marked this conversation as resolved.
Show resolved Hide resolved

```js
/// file: src/hooks.js
/** @type {import('@sveltejs/kit').Handle} */
export async function handle({ event, resolve }) {
const response = await resolve(event, {
transformPageChunk: ({ html }) => html.replace('old', 'new')
transformPageChunk: ({ html }) => html.replace('old', 'new'),
filterSerializedResponseHeaders: (name) => name.startsWith('x-')
});

return response;
Expand Down
8 changes: 6 additions & 2 deletions packages/kit/src/runtime/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import { DATA_SUFFIX } from '../../constants.js';
/** @param {{ html: string }} opts */
const default_transform = ({ html }) => html;

const default_filter = () => false;

/** @type {import('types').Respond} */
export async function respond(request, options, state) {
let url = new URL(request.url);
Expand Down Expand Up @@ -201,7 +203,8 @@ export async function respond(request, options, state) {

/** @type {import('types').RequiredResolveOptions} */
let resolve_opts = {
transformPageChunk: default_transform
transformPageChunk: default_transform,
filterSerializedResponseHeaders: default_filter
};

/**
Expand All @@ -226,7 +229,8 @@ export async function respond(request, options, state) {
}

resolve_opts = {
transformPageChunk: opts.transformPageChunk || default_transform
transformPageChunk: opts.transformPageChunk || default_transform,
filterSerializedResponseHeaders: opts.filterSerializedResponseHeaders || default_filter
};
}

Expand Down
10 changes: 3 additions & 7 deletions packages/kit/src/runtime/server/page/fetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,13 +214,9 @@ export function create_fetch({ event, options, state, route, prerender_default }
? request.url.slice(event.url.origin.length)
: request.url,
method: request.method,
body: /** @type {string | undefined} */ (request_body),
response: {
status: status_number,
statusText: response.statusText,
headers,
body
}
request_body: /** @type {string | undefined} */ (request_body),
response_body: body,
response: response
});
}

Expand Down
6 changes: 5 additions & 1 deletion packages/kit/src/runtime/server/page/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,11 @@ export async function render_response({
}

if (page_config.ssr && page_config.csr) {
body += `\n\t${fetched.map((item) => serialize_data(item, !!state.prerendering)).join('\n\t')}`;
body += `\n\t${fetched
.map((item) =>
serialize_data(item, resolve_opts.filterSerializedResponseHeaders, !!state.prerendering)
)
.join('\n\t')}`;
}

if (options.service_worker) {
Expand Down
33 changes: 24 additions & 9 deletions packages/kit/src/runtime/server/page/serialize_data.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,32 +35,47 @@ const pattern = new RegExp(`[${Object.keys(replacements).join('')}]`, 'g');
* and that the resulting string isn't further modified.
*
* @param {import('./types.js').Fetched} fetched
* @param {(name: string, value: string) => boolean} filter
* @param {boolean} [prerendering]
* @returns {string} The raw HTML of a script element carrying the JSON payload.
* @example const html = serialize_data('/data.json', null, { foo: 'bar' });
*/
export function serialize_data(fetched, prerendering = false) {
const safe_payload = JSON.stringify(fetched.response).replace(
pattern,
(match) => replacements[match]
);
export function serialize_data(fetched, filter, prerendering = false) {
/** @type {Record<string, string>} */
const headers = {};

for (const [key, value] of fetched.response.headers) {
const lower = key.toLowerCase();
if (filter(lower, value)) {
headers[lower] = value;
}
}

const payload = {
status: fetched.response.status,
statusText: fetched.response.statusText,
headers,
body: fetched.response_body
};

const safe_payload = JSON.stringify(payload).replace(pattern, (match) => replacements[match]);

const attrs = [
'type="application/json"',
'data-sveltekit-fetched',
`data-url=${escape_html_attr(fetched.url)}`
];

if (fetched.body) {
attrs.push(`data-hash=${escape_html_attr(hash(fetched.body))}`);
if (fetched.request_body) {
attrs.push(`data-hash=${escape_html_attr(hash(fetched.request_body))}`);
}

if (!prerendering && fetched.method === 'GET') {
const cache_control = /** @type {string} */ (fetched.response.headers['cache-control']);
const cache_control = fetched.response.headers.get('cache-control');
if (cache_control) {
const match = /s-maxage=(\d+)/g.exec(cache_control) ?? /max-age=(\d+)/g.exec(cache_control);
if (match) {
const age = /** @type {string} */ (fetched.response.headers['age']) ?? '0';
const age = fetched.response.headers.get('age') ?? '0';

const ttl = +match[1] - +age;
attrs.push(`data-ttl="${ttl}"`);
Expand Down
74 changes: 38 additions & 36 deletions packages/kit/src/runtime/server/page/serialize_data.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,59 +3,61 @@ import * as assert from 'uvu/assert';
import { serialize_data } from './serialize_data.js';

test('escapes slashes', () => {
const response_body = '</script><script>alert("xss")';

assert.equal(
serialize_data({
url: 'foo',
method: 'GET',
body: null,
response: {
status: 200,
statusText: 'OK',
headers: {},
body: '</script><script>alert("xss")'
}
}),
serialize_data(
{
url: 'foo',
method: 'GET',
request_body: null,
response_body,
response: new Response(response_body)
},
() => false
),
'<script type="application/json" data-sveltekit-fetched data-url="foo">' +
'{"status":200,"statusText":"OK","headers":{},"body":"\\u003C/script>\\u003Cscript>alert(\\"xss\\")"}' +
'{"status":200,"statusText":"","headers":{},"body":"\\u003C/script>\\u003Cscript>alert(\\"xss\\")"}' +
'</script>'
);
});

test('escapes exclamation marks', () => {
const response_body = '<!--</script>...-->alert("xss")';

assert.equal(
serialize_data({
url: 'foo',
method: 'GET',
body: null,
response: {
status: 200,
statusText: 'OK',
headers: {},
body: '<!--</script>...-->alert("xss")'
}
}),
serialize_data(
{
url: 'foo',
method: 'GET',
request_body: null,
response_body,
response: new Response(response_body)
},
() => false
),
'<script type="application/json" data-sveltekit-fetched data-url="foo">' +
'{"status":200,"statusText":"OK","headers":{},"body":"\\u003C!--\\u003C/script>...-->alert(\\"xss\\")"}' +
'{"status":200,"statusText":"","headers":{},"body":"\\u003C!--\\u003C/script>...-->alert(\\"xss\\")"}' +
'</script>'
);
});

test('escapes the attribute values', () => {
const raw = 'an "attr" & a \ud800';
const escaped = 'an &quot;attr&quot; &amp; a &#55296;';
const response_body = '';
assert.equal(
serialize_data({
url: raw,
method: 'GET',
body: null,
response: {
status: 200,
statusText: 'OK',
headers: {},
body: ''
}
}),
`<script type="application/json" data-sveltekit-fetched data-url="${escaped}">{"status":200,"statusText":"OK","headers":{},"body":\"\"}</script>`
serialize_data(
{
url: raw,
method: 'GET',
request_body: null,
response_body,
response: new Response(response_body)
},
() => false
),
`<script type="application/json" data-sveltekit-fetched data-url="${escaped}">{"status":200,"statusText":"","headers":{},"body":\"\"}</script>`
);
});

Expand Down
12 changes: 4 additions & 8 deletions packages/kit/src/runtime/server/page/types.d.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
import { ResponseHeaders, SSRNode, CspDirectives } from 'types';
import { SSRNode, CspDirectives } from 'types';
import { HttpError } from '../../control.js';

export interface Fetched {
url: string;
method: string;
body?: string | null;
response: {
status: number;
statusText: string;
headers: ResponseHeaders;
body: string;
};
request_body?: string | null;
response_body: string;
response: Response;
}

export interface FetchState {
Expand Down
10 changes: 3 additions & 7 deletions packages/kit/test/apps/basics/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -682,8 +682,7 @@ test.describe('Load', () => {
// by the time JS has run, hydration will have nuked these scripts
const script_contents = await page.innerHTML('script[data-sveltekit-fetched]');

const payload =
'{"status":200,"statusText":"","headers":{"content-type":"application/json"},"body":"{\\"answer\\":42}"}';
const payload = '{"status":200,"statusText":"","headers":{},"body":"{\\"answer\\":42}"}';

expect(script_contents).toBe(payload);
}
Expand All @@ -701,11 +700,8 @@ test.describe('Load', () => {
expect(await page.textContent('h1')).toBe('a: X');
expect(await page.textContent('h2')).toBe('b: Y');

const payload_a =
'{"status":200,"statusText":"","headers":{"content-type":"text/plain;charset=UTF-8"},"body":"X"}';

const payload_b =
'{"status":200,"statusText":"","headers":{"content-type":"text/plain;charset=UTF-8"},"body":"Y"}';
const payload_a = '{"status":200,"statusText":"","headers":{},"body":"X"}';
const payload_b = '{"status":200,"statusText":"","headers":{},"body":"Y"}';

if (!javaScriptEnabled) {
// by the time JS has run, hydration will have nuked these scripts
Expand Down
1 change: 1 addition & 0 deletions packages/kit/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ export interface RequestHandler<

export interface ResolveOptions {
transformPageChunk?: (input: { html: string; done: boolean }) => MaybePromise<string | undefined>;
filterSerializedResponseHeaders?: (name: string, value: string) => boolean;
}

export class Server {
Expand Down