From 52e6b621788cba5406364757bfe736c9f91f1521 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 19 Jul 2021 10:25:28 -0400 Subject: [PATCH 1/3] fix server-side fetch URL replacement (#1952) --- .../kit/src/runtime/server/page/load_node.js | 187 ++++++++---------- .../kit/src/runtime/server/page/resolve.js | 13 +- .../src/runtime/server/page/resolve.spec.js | 8 + 3 files changed, 105 insertions(+), 103 deletions(-) diff --git a/packages/kit/src/runtime/server/page/load_node.js b/packages/kit/src/runtime/server/page/load_node.js index 5b97eeb6bc6c..ee70c1a10822 100644 --- a/packages/kit/src/runtime/server/page/load_node.js +++ b/packages/kit/src/runtime/server/page/load_node.js @@ -4,8 +4,6 @@ import { resolve } from './resolve.js'; const s = JSON.stringify; -const hasScheme = (/** @type {string} */ url) => /^[a-zA-Z]+:/.test(url); - /** * * @param {{ @@ -86,24 +84,91 @@ export async function load_node({ }; } - if (options.read && url.startsWith(options.paths.assets)) { - // when running `start`, or prerendering, `assets` should be - // config.kit.paths.assets, but we should still be able to fetch - // assets directly from `static` - url = url.replace(options.paths.assets, ''); - } - - if (url.startsWith('//')) { - throw new Error(`Cannot request protocol-relative URL (${url}) in server-side fetch`); - } + const [path, search] = url.split('?'); + const resolved = resolve(request.path, path); let response; - if (hasScheme(url)) { - // possibly external fetch + // handle fetch requests for static assets. e.g. prebaked data, etc. + // we need to support everything the browser's fetch supports + const filename = resolved.replace(options.paths.assets, '').slice(1); + const filename_html = `${filename}/index.html`; // path may also match path/index.html + const asset = options.manifest.assets.find( + (d) => d.file === filename || d.file === filename_html + ); + + if (asset) { + response = options.read + ? new Response(options.read(asset.file), { + headers: { + 'content-type': asset.type + } + }) + : await fetch( + // TODO we need to know what protocol to use + `http://${page.host}/${asset.file}`, + /** @type {RequestInit} */ (opts) + ); + } else if (resolved.startsWith(options.paths.base)) { + const relative = resolved.replace(options.paths.base, ''); + + const headers = /** @type {import('types/helper').Headers} */ ({ ...opts.headers }); + + // TODO: fix type https://github.com/node-fetch/node-fetch/issues/1113 + if (opts.credentials !== 'omit') { + uses_credentials = true; + + headers.cookie = request.headers.cookie; + + if (!headers.authorization) { + headers.authorization = request.headers.authorization; + } + } + + if (opts.body && typeof opts.body !== 'string') { + // per https://developer.mozilla.org/en-US/docs/Web/API/Request/Request, this can be a + // Blob, BufferSource, FormData, URLSearchParams, USVString, or ReadableStream object. + // non-string bodies are irksome to deal with, but luckily aren't particularly useful + // in this context anyway, so we take the easy route and ban them + throw new Error('Request body must be a string'); + } + + const rendered = await respond( + { + host: request.host, + method: opts.method || 'GET', + headers, + path: relative, + rawBody: /** @type {string} */ (opts.body), + query: new URLSearchParams(search) + }, + options, + { + fetched: url, + initiator: route + } + ); + + if (rendered) { + if (state.prerender) { + state.prerender.dependencies.set(relative, rendered); + } + + response = new Response(rendered.body, { + status: rendered.status, + headers: rendered.headers + }); + } + } else { + // external + if (resolved.startsWith('//')) { + throw new Error(`Cannot request protocol-relative URL (${url}) in server-side fetch`); + } + + // external fetch if (typeof request.host !== 'undefined') { - const { hostname: fetchHostname } = new URL(url); - const [serverHostname] = request.host.split(':'); + const { hostname: fetch_hostname } = new URL(url); + const [server_hostname] = request.host.split(':'); // allow cookie passthrough for "same-origin" // if SvelteKit is serving my.domain.com: @@ -113,7 +178,10 @@ export async function load_node({ // - sub.my.domain.com WILL receive cookies // ports do not affect the resolution // leading dot prevents mydomain.com matching domain.com - if (`.${fetchHostname}`.endsWith(`.${serverHostname}`) && opts.credentials !== 'omit') { + if ( + `.${fetch_hostname}`.endsWith(`.${server_hostname}`) && + opts.credentials !== 'omit' + ) { uses_credentials = true; opts.headers = { @@ -123,89 +191,8 @@ export async function load_node({ } } - const externalRequest = new Request(url, /** @type {RequestInit} */ (opts)); - response = await options.hooks.serverFetch.call(null, externalRequest); - } else { - const [path, search] = url.split('?'); - - // otherwise we're dealing with an internal fetch - const resolved = resolve(request.path, path); - - // handle fetch requests for static assets. e.g. prebaked data, etc. - // we need to support everything the browser's fetch supports - const filename = resolved.slice(1); - const filename_html = `${filename}/index.html`; // path may also match path/index.html - const asset = options.manifest.assets.find( - (d) => d.file === filename || d.file === filename_html - ); - - if (asset) { - // we don't have a running server while prerendering because jumping between - // processes would be inefficient so we have options.read instead - if (options.read) { - response = new Response(options.read(asset.file), { - headers: { - 'content-type': asset.type - } - }); - } else { - // TODO we need to know what protocol to use - response = await fetch( - `http://${page.host}/${asset.file}`, - /** @type {RequestInit} */ (opts) - ); - } - } - - if (!response) { - const headers = /** @type {import('types/helper').Headers} */ ({ ...opts.headers }); - - // TODO: fix type https://github.com/node-fetch/node-fetch/issues/1113 - if (opts.credentials !== 'omit') { - uses_credentials = true; - - headers.cookie = request.headers.cookie; - - if (!headers.authorization) { - headers.authorization = request.headers.authorization; - } - } - - if (opts.body && typeof opts.body !== 'string') { - // per https://developer.mozilla.org/en-US/docs/Web/API/Request/Request, this can be a - // Blob, BufferSource, FormData, URLSearchParams, USVString, or ReadableStream object. - // non-string bodies are irksome to deal with, but luckily aren't particularly useful - // in this context anyway, so we take the easy route and ban them - throw new Error('Request body must be a string'); - } - - const rendered = await respond( - { - host: request.host, - method: opts.method || 'GET', - headers, - path: resolved, - rawBody: /** @type {string} */ (opts.body), - query: new URLSearchParams(search) - }, - options, - { - fetched: url, - initiator: route - } - ); - - if (rendered) { - if (state.prerender) { - state.prerender.dependencies.set(resolved, rendered); - } - - response = new Response(rendered.body, { - status: rendered.status, - headers: rendered.headers - }); - } - } + const external_request = new Request(url, /** @type {RequestInit} */ (opts)); + response = await options.hooks.serverFetch.call(null, external_request); } if (response) { diff --git a/packages/kit/src/runtime/server/page/resolve.js b/packages/kit/src/runtime/server/page/resolve.js index a59785fbbb24..c67c23010646 100644 --- a/packages/kit/src/runtime/server/page/resolve.js +++ b/packages/kit/src/runtime/server/page/resolve.js @@ -1,10 +1,15 @@ +const absolute = /^([a-z]+:)?\/?\//; + /** * @param {string} base * @param {string} path */ export function resolve(base, path) { - const baseparts = path[0] === '/' ? [] : base.slice(1).split('/'); - const pathparts = path[0] === '/' ? path.slice(1).split('/') : path.split('/'); + const base_match = absolute.exec(base); + const path_match = absolute.exec(path); + + const baseparts = path_match ? [] : base.slice(base_match[0].length).split('/'); + const pathparts = path_match ? path.slice(path_match[0].length).split('/') : path.split('/'); baseparts.pop(); @@ -15,5 +20,7 @@ export function resolve(base, path) { else baseparts.push(part); } - return `/${baseparts.join('/')}`; + const prefix = (path_match && path_match[0]) || (base_match && base_match[0]) || ''; + + return `${prefix}${baseparts.join('/')}`; } diff --git a/packages/kit/src/runtime/server/page/resolve.spec.js b/packages/kit/src/runtime/server/page/resolve.spec.js index 611cfd88e2b0..20f5b91c9bed 100644 --- a/packages/kit/src/runtime/server/page/resolve.spec.js +++ b/packages/kit/src/runtime/server/page/resolve.spec.js @@ -38,4 +38,12 @@ test('resolves a root-relative path with .', () => { assert.equal(resolve('/a/b/c', '/x/./y/../z'), '/x/z'); }); +test('resolves a protocol-relative path', () => { + assert.equal(resolve('/a/b/c', '//example.com/foo'), '//example.com/foo'); +}); + +test('resolves an absolute path', () => { + assert.equal(resolve('/a/b/c', 'https://example.com/foo'), 'https://example.com/foo'); +}); + test.run(); From 05fefef9c08504ddc254606f21c47ad0944e8c2e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 19 Jul 2021 10:33:16 -0400 Subject: [PATCH 2/3] changeset --- .changeset/tall-pens-obey.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/tall-pens-obey.md diff --git a/.changeset/tall-pens-obey.md b/.changeset/tall-pens-obey.md new file mode 100644 index 000000000000..4f47f9533f74 --- /dev/null +++ b/.changeset/tall-pens-obey.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +Fix URL resolution for server-side fetch From 19cf6494da40d2973479570d15ead6e3e1800c18 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 19 Jul 2021 12:38:16 -0400 Subject: [PATCH 3/3] handle question marks in query string --- packages/kit/src/runtime/server/page/load_node.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/kit/src/runtime/server/page/load_node.js b/packages/kit/src/runtime/server/page/load_node.js index ee70c1a10822..38ea26bca38d 100644 --- a/packages/kit/src/runtime/server/page/load_node.js +++ b/packages/kit/src/runtime/server/page/load_node.js @@ -84,8 +84,7 @@ export async function load_node({ }; } - const [path, search] = url.split('?'); - const resolved = resolve(request.path, path); + const resolved = resolve(request.path, url.split('?')[0]); let response; @@ -133,6 +132,8 @@ export async function load_node({ throw new Error('Request body must be a string'); } + const search = url.includes('?') ? url.slice(url.indexOf('?') + 1) : ''; + const rendered = await respond( { host: request.host,