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 all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/silent-cycles-reply.md
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

[breaking] exclude headers from serialized responses by default, add `filterSerializedResponseHeaders` `resolve` option
2 changes: 2 additions & 0 deletions .gitignore
Expand Up @@ -4,6 +4,7 @@ dist
test-results/
package-lock.json
yarn.lock
vite.config.js.timestamp-*
/packages/create-svelte/template/CHANGELOG.md
/packages/package/test/**/package
/documentation/types.js
Expand All @@ -16,3 +17,4 @@ yarn.lock
.turbo
.vercel
.test-tmp

2 changes: 1 addition & 1 deletion documentation/docs/05-load.md
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
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.

```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
18 changes: 12 additions & 6 deletions packages/kit/src/core/prerender/prerender.js
Expand Up @@ -237,10 +237,11 @@ export async function prerender() {
const encoded_dependency_path = new URL(dependency_path, 'http://localhost').pathname;
const decoded_dependency_path = decodeURI(encoded_dependency_path);

const prerender = result.response.headers.get('x-sveltekit-prerender');
const headers = Object.fromEntries(result.response.headers);

const prerender = headers['x-sveltekit-prerender'];
if (prerender) {
const route_id = /** @type {string} */ (result.response.headers.get('x-sveltekit-routeid'));
const route_id = headers['x-sveltekit-routeid'];
const existing_value = prerender_map.get(route_id);
if (existing_value !== 'auto') {
prerender_map.set(route_id, prerender === 'true' ? true : 'auto');
Expand All @@ -259,7 +260,10 @@ export async function prerender() {
);
}

if (config.prerender.crawl && response.headers.get('content-type') === 'text/html') {
// avoid triggering `filterSerializeResponseHeaders` guard
const headers = Object.fromEntries(response.headers);

if (config.prerender.crawl && headers['content-type'] === 'text/html') {
for (const href of crawl(body.toString())) {
if (href.startsWith('data:') || href.startsWith('#')) continue;

Expand Down Expand Up @@ -288,7 +292,9 @@ export async function prerender() {
*/
function save(category, response, body, decoded, encoded, referrer, referenceType) {
const response_type = Math.floor(response.status / 100);
const type = /** @type {string} */ (response.headers.get('content-type'));
const headers = Object.fromEntries(response.headers);

const type = headers['content-type'];
const is_html = response_type === REDIRECT || type === 'text/html';

const file = output_filename(decoded, is_html);
Expand All @@ -297,15 +303,15 @@ export async function prerender() {
if (written.has(file)) return;

if (response_type === REDIRECT) {
const location = response.headers.get('location');
const location = headers['location'];

if (location) {
const resolved = resolve(encoded, location);
if (is_root_relative(resolved)) {
enqueue(decoded, decodeURI(resolved), resolved);
}

if (!response.headers.get('x-sveltekit-normalize')) {
if (!headers['x-sveltekit-normalize']) {
mkdirp(dirname(dest));

log.warn(`${response.status} ${decoded} -> ${location}`);
Expand Down
8 changes: 6 additions & 2 deletions packages/kit/src/runtime/server/index.js
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
40 changes: 22 additions & 18 deletions packages/kit/src/runtime/server/page/fetch.js
Expand Up @@ -10,9 +10,10 @@ import { domain_matches, path_matches } from './cookie.js';
* state: import('types').SSRState;
* route: import('types').SSRRoute | import('types').SSRErrorPage;
* prerender_default?: import('types').PrerenderOption;
* resolve_opts: import('types').RequiredResolveOptions;
* }} opts
*/
export function create_fetch({ event, options, state, route, prerender_default }) {
export function create_fetch({ event, options, state, route, prerender_default, resolve_opts }) {
/** @type {import('./types').Fetched[]} */
const fetched = [];

Expand Down Expand Up @@ -189,16 +190,6 @@ export function create_fetch({ event, options, state, route, prerender_default }
async function text() {
const body = await response.text();

// TODO just pass `response.headers`, for processing inside `serialize_data`
/** @type {import('types').ResponseHeaders} */
const headers = {};
for (const [key, value] of response.headers) {
// TODO skip others besides set-cookie and etag?
if (key !== 'set-cookie' && key !== 'etag') {
headers[key] = value;
}
}

if (!body || typeof body === 'string') {
const status_number = Number(response.status);
if (isNaN(status_number)) {
Expand All @@ -214,14 +205,27 @@ 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
});

// ensure that excluded headers can't be read
const get = response.headers.get;
response.headers.get = (key) => {
const lower = key.toLowerCase();
const value = get.call(response.headers, lower);
if (value && !lower.startsWith('x-sveltekit-')) {
const included = resolve_opts.filterSerializedResponseHeaders(lower, value);
if (!included) {
throw new Error(
`Failed to get response header "${lower}" — it must be included by the \`filterSerializedResponseHeaders\` option: https://kit.svelte.dev/docs/hooks#handle`
);
}
}

return value;
};
}

if (dependency) {
Expand Down
3 changes: 2 additions & 1 deletion packages/kit/src/runtime/server/page/index.js
Expand Up @@ -131,7 +131,8 @@ export async function render_page(event, route, page, options, state, resolve_op
options,
state,
route,
prerender_default: should_prerender
prerender_default: should_prerender,
resolve_opts
});

if (get_option(nodes, 'ssr') === false) {
Expand Down
7 changes: 6 additions & 1 deletion packages/kit/src/runtime/server/page/render.js
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 Expand Up @@ -321,6 +325,7 @@ export async function render_response({
})) || '';

const headers = new Headers({
'x-sveltekit-page': 'true',
'content-type': 'text/html',
etag: `"${hash(html)}"`
});
Expand Down
3 changes: 2 additions & 1 deletion packages/kit/src/runtime/server/page/respond_with_error.js
Expand Up @@ -25,7 +25,8 @@ export async function respond_with_error({ event, options, state, status, error,
event,
options,
state,
route: GENERIC_ERROR
route: GENERIC_ERROR,
resolve_opts
});

try {
Expand Down
49 changes: 32 additions & 17 deletions packages/kit/src/runtime/server/page/serialize_data.js
Expand Up @@ -35,36 +35,51 @@ 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 = {};

let cache_control = null;
let age = null;

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

if (key === 'cache-control') cache_control = value;
if (key === 'age') age = 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']);
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 ttl = +match[1] - +age;
attrs.push(`data-ttl="${ttl}"`);
}
if (!prerendering && fetched.method === 'GET' && cache_control) {
const match = /s-maxage=(\d+)/g.exec(cache_control) ?? /max-age=(\d+)/g.exec(cache_control);
if (match) {
const ttl = +match[1] - +(age ?? '0');
attrs.push(`data-ttl="${ttl}"`);
}
}

Expand Down