From 338a6a828565bd6a18053115830e3fd2990890b7 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 4 Sep 2022 11:42:59 -0400 Subject: [PATCH 01/13] replace externalFetch with handleFetch --- .../src/exports/vite/build/build_server.js | 8 +- packages/kit/src/exports/vite/dev/index.js | 8 +- packages/kit/src/runtime/server/page/fetch.js | 283 ++++++++---------- packages/kit/test/apps/basics/src/hooks.js | 9 +- packages/kit/test/apps/basics/test/test.js | 3 +- packages/kit/types/index.d.ts | 4 + packages/kit/types/internal.d.ts | 6 +- 7 files changed, 159 insertions(+), 162 deletions(-) diff --git a/packages/kit/src/exports/vite/build/build_server.js b/packages/kit/src/exports/vite/build/build_server.js index 137d358eeb75..b9a37fce15fb 100644 --- a/packages/kit/src/exports/vite/build/build_server.js +++ b/packages/kit/src/exports/vite/build/build_server.js @@ -110,10 +110,16 @@ export class Server { if (!this.options.hooks) { const module = await import(${s(hooks)}); + + // TODO remove this for 1.0 + if (module.externalFetch) { + throw new Error('externalFetch has been removed — use handleFetch instead'); // TODO add migration guide + } + this.options.hooks = { handle: module.handle || (({ event, resolve }) => resolve(event)), handleError: module.handleError || (({ error }) => console.error(error.stack)), - externalFetch: module.externalFetch || fetch + handleFetch: module.handleFetch || (({ request, fetch }) => fetch(request)) }; } } diff --git a/packages/kit/src/exports/vite/dev/index.js b/packages/kit/src/exports/vite/dev/index.js index 4ce5b8fb9b27..d65f57d70ec4 100644 --- a/packages/kit/src/exports/vite/dev/index.js +++ b/packages/kit/src/exports/vite/dev/index.js @@ -317,6 +317,12 @@ export async function dev(vite, vite_config, svelte_config, illegal_imports) { const handle = user_hooks.handle || (({ event, resolve }) => resolve(event)); + // TODO remove for 1.0 + // @ts-expect-error + if (user_hooks.externalFetch) { + throw new Error('externalFetch has been removed — use handleFetch instead'); // TODO add migration guide + } + /** @type {import('types').Hooks} */ const hooks = { handle, @@ -331,7 +337,7 @@ export async function dev(vite, vite_config, svelte_config, illegal_imports) { console.error(colors.gray(error.stack)); } }), - externalFetch: user_hooks.externalFetch || fetch + handleFetch: user_hooks.handleFetch || (({ request, fetch }) => fetch(request)) }; if (/** @type {any} */ (hooks).getContext) { diff --git a/packages/kit/src/runtime/server/page/fetch.js b/packages/kit/src/runtime/server/page/fetch.js index 703179fe3d0e..66a0a9082053 100644 --- a/packages/kit/src/runtime/server/page/fetch.js +++ b/packages/kit/src/runtime/server/page/fetch.js @@ -1,7 +1,6 @@ import * as cookie from 'cookie'; import * as set_cookie_parser from 'set-cookie-parser'; import { respond } from '../index.js'; -import { is_root_relative, resolve } from '../../../utils/url.js'; import { domain_matches, path_matches } from './cookie.js'; /** @@ -23,178 +22,146 @@ export function create_fetch({ event, options, state, route, prerender_default } const cookies = []; /** @type {typeof fetch} */ - const fetcher = async (resource, opts = {}) => { - /** @type {string} */ - let requested; - - if (typeof resource === 'string' || resource instanceof URL) { - requested = resource.toString(); - } else { - requested = resource.url; - - opts = { - method: resource.method, - headers: resource.headers, - body: resource.body, - mode: resource.mode, - credentials: resource.credentials, - cache: resource.cache, - redirect: resource.redirect, - referrer: resource.referrer, - integrity: resource.integrity, - ...opts - }; - } - - opts.headers = new Headers(opts.headers); - - // merge headers from request - for (const [key, value] of event.request.headers) { - if ( - key !== 'authorization' && - key !== 'connection' && - key !== 'content-length' && - key !== 'cookie' && - key !== 'host' && - key !== 'if-none-match' && - !opts.headers.has(key) - ) { - opts.headers.set(key, value); - } - } + const fetcher = async (info, init) => { + const request = normalize_fetch_input( + typeof info === 'string' ? new URL(info, event.url) : info, + init + ); - const resolved = resolve(event.url.pathname, requested.split('?')[0]).replace(/#.+$/, ''); - - /** @type {Response} */ - let response; + const body = init?.body; /** @type {import('types').PrerenderDependency} */ let dependency; - // handle fetch requests for static assets. e.g. prebaked data, etc. - // we need to support everything the browser's fetch supports - const prefix = options.paths.assets || options.paths.base; - const filename = decodeURIComponent( - resolved.startsWith(prefix) ? resolved.slice(prefix.length) : resolved - ).slice(1); - const filename_html = `${filename}/index.html`; // path may also match path/index.html + const response = await options.hooks.handleFetch({ + event, + request, + fetch: async (info, init) => { + const request = normalize_fetch_input(info, init); - const is_asset = options.manifest.assets.has(filename); - const is_asset_html = options.manifest.assets.has(filename_html); + const url = new URL(request.url); - if (is_asset || is_asset_html) { - const file = is_asset ? filename : filename_html; + if (url.origin !== event.url.origin) { + // allow cookie passthrough for "same-origin" + // if SvelteKit is serving my.domain.com: + // - domain.com WILL NOT receive cookies + // - my.domain.com WILL receive cookies + // - api.domain.dom WILL NOT receive cookies + // - sub.my.domain.com WILL receive cookies + // ports do not affect the resolution + // leading dot prevents mydomain.com matching domain.com + if ( + `.${url.hostname}`.endsWith(`.${event.url.hostname}`) && + request.credentials !== 'omit' + ) { + const cookie = event.request.headers.get('cookie'); + if (cookie) request.headers.set('cookie', cookie); + } - if (options.read) { - const type = is_asset - ? options.manifest.mimeTypes[filename.slice(filename.lastIndexOf('.'))] - : 'text/html'; + let response = await fetch(request); - response = new Response(options.read(file), { - headers: type ? { 'content-type': type } : {} - }); - } else { - response = await fetch(`${event.url.origin}/${file}`, /** @type {RequestInit} */ (opts)); - } - } else if (is_root_relative(resolved)) { - if (opts.credentials !== 'omit') { - const authorization = event.request.headers.get('authorization'); + if (request.mode === 'no-cors') { + response = new Response('', { + status: response.status, + statusText: response.statusText, + headers: response.headers + }); + } else { + if (url.origin !== event.url.origin) { + const acao = response.headers.get('access-control-allow-origin'); + if (!acao || (acao !== event.url.origin && acao !== '*')) { + throw new Error( + `CORS error: ${ + acao ? 'Incorrect' : 'No' + } 'Access-Control-Allow-Origin' header is present on the requested resource` + ); + } + } + } - // combine cookies from the initiating request with any that were - // added via set-cookie - const combined_cookies = { ...initial_cookies }; + return response; + } - for (const cookie of cookies) { - if (!domain_matches(event.url.hostname, cookie.domain)) continue; - if (!path_matches(resolved, cookie.path)) continue; + /** @type {Response} */ + let response; - combined_cookies[cookie.name] = cookie.value; - } + // handle fetch requests for static assets. e.g. prebaked data, etc. + // we need to support everything the browser's fetch supports + const prefix = options.paths.assets || options.paths.base; + const filename = ( + url.pathname.startsWith(prefix) ? url.pathname.slice(prefix.length) : url.pathname + ).slice(1); + const filename_html = `${filename}/index.html`; // path may also match path/index.html - const cookie = Object.entries(combined_cookies) - .map(([name, value]) => `${name}=${value}`) - .join('; '); + const is_asset = options.manifest.assets.has(filename); + const is_asset_html = options.manifest.assets.has(filename_html); - if (cookie) { - opts.headers.set('cookie', cookie); - } + if (is_asset || is_asset_html) { + const file = is_asset ? filename : filename_html; - if (authorization && !opts.headers.has('authorization')) { - opts.headers.set('authorization', authorization); - } - } + if (options.read) { + const type = is_asset + ? options.manifest.mimeTypes[filename.slice(filename.lastIndexOf('.'))] + : 'text/html'; - 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'); - } + return new Response(options.read(file), { + headers: type ? { 'content-type': type } : {} + }); + } - response = await respond( - new Request(new URL(requested, event.url).href, { ...opts }), - options, - { - prerender_default, - ...state, - initiator: route + return await fetch(request); } - ); - if (state.prerendering) { - dependency = { response, body: null }; - state.prerendering.dependencies.set(resolved, dependency); - } - } else { - // external - if (resolved.startsWith('//')) { - requested = event.url.protocol + requested; - } + if (request.credentials !== 'omit') { + const authorization = event.request.headers.get('authorization'); - const url = new URL(requested); - - // external fetch - // allow cookie passthrough for "same-origin" - // if SvelteKit is serving my.domain.com: - // - domain.com WILL NOT receive cookies - // - my.domain.com WILL receive cookies - // - api.domain.dom WILL NOT receive cookies - // - sub.my.domain.com WILL receive cookies - // ports do not affect the resolution - // leading dot prevents mydomain.com matching domain.com - if (`.${url.hostname}`.endsWith(`.${event.url.hostname}`) && opts.credentials !== 'omit') { - const cookie = event.request.headers.get('cookie'); - if (cookie) opts.headers.set('cookie', cookie); - } + // combine cookies from the initiating request with any that were + // added via set-cookie + const combined_cookies = { ...initial_cookies }; - // we need to delete the connection header, as explained here: - // https://github.com/nodejs/undici/issues/1470#issuecomment-1140798467 - // TODO this may be a case for being selective about which headers we let through - opts.headers.delete('connection'); + for (const cookie of cookies) { + if (!domain_matches(event.url.hostname, cookie.domain)) continue; + if (!path_matches(url.pathname, cookie.path)) continue; - const external_request = new Request(requested, /** @type {RequestInit} */ (opts)); - response = await options.hooks.externalFetch.call(null, external_request); + combined_cookies[cookie.name] = cookie.value; + } - if (opts.mode === 'no-cors') { - response = new Response('', { - status: response.status, - statusText: response.statusText, - headers: response.headers - }); - } else { - if (url.origin !== event.url.origin) { - const acao = response.headers.get('access-control-allow-origin'); - if (!acao || (acao !== event.url.origin && acao !== '*')) { - throw new Error( - `CORS error: ${ - acao ? 'Incorrect' : 'No' - } 'Access-Control-Allow-Origin' header is present on the requested resource` - ); + const cookie = Object.entries(combined_cookies) + .map(([name, value]) => `${name}=${value}`) + .join('; '); + + if (cookie) { + request.headers.set('cookie', cookie); } + + if (authorization && !request.headers.has('authorization')) { + request.headers.set('authorization', authorization); + } + } + + if (body && typeof body !== 'string') { + // TODO is this still necessary? we just bail out below + // 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'); } + + response = await respond(request, options, { + prerender_default, + ...state, + initiator: route + }); + + if (state.prerendering) { + dependency = { response, body: null }; + state.prerendering.dependencies.set(url.pathname, dependency); + } + + return response; } - } + }); const set_cookie = response.headers.get('set-cookie'); if (set_cookie) { @@ -220,7 +187,7 @@ export function create_fetch({ event, options, state, route, prerender_default } } } - if (!opts.body || typeof opts.body === 'string') { + if (!body || typeof body === 'string') { const status_number = Number(response.status); if (isNaN(status_number)) { throw new Error( @@ -231,9 +198,11 @@ export function create_fetch({ event, options, state, route, prerender_default } } fetched.push({ - url: requested, - method: opts.method || 'GET', - body: opts.body, + url: response.url.startsWith(event.url.origin) + ? response.url.slice(event.url.origin.length) + : response.url, + method: request.method, + body, response: { status: status_number, statusText: response.statusText, @@ -286,3 +255,15 @@ export function create_fetch({ event, options, state, route, prerender_default } return { fetcher, fetched, cookies }; } + +/** + * @param {RequestInfo | URL} info + * @param {RequestInit | undefined} init + */ +function normalize_fetch_input(info, init) { + if (info instanceof Request) { + return info; + } + + return new Request(info, init); +} diff --git a/packages/kit/test/apps/basics/src/hooks.js b/packages/kit/test/apps/basics/src/hooks.js index 6a40df117da3..4cb664e2ce54 100644 --- a/packages/kit/test/apps/basics/src/hooks.js +++ b/packages/kit/test/apps/basics/src/hooks.js @@ -48,15 +48,14 @@ export const handle = sequence( } ); -/** @type {import('@sveltejs/kit').ExternalFetch} */ -export async function externalFetch(request) { - let newRequest = request; +/** @type {import('@sveltejs/kit').HandleFetch} */ +export async function handleFetch({ request, fetch }) { if (request.url.endsWith('/server-fetch-request.json')) { - newRequest = new Request( + request = new Request( request.url.replace('/server-fetch-request.json', '/server-fetch-request-modified.json'), request ); } - return fetch(newRequest); + return fetch(request); } diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index d46e73f175ce..6e878d5ec138 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -773,7 +773,8 @@ test.describe('Load', () => { const requested_urls = []; const { port, close } = await start_server(async (req, res) => { - if (!req.url) throw new Error('Incomplete request'); + console.error(`req.url ${req.url}`); + requested_urls.push(req.url); if (req.url === '/server-fetch-request-modified.json') { diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index c09238532ea0..28897f545d14 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -191,6 +191,10 @@ export interface HandleError { (input: { error: Error & { frame?: string }; event: RequestEvent }): void; } +export interface HandleFetch { + (input: { event: RequestEvent; request: Request; fetch: typeof fetch }): MaybePromise; +} + /** * The generic form of `PageLoad` and `LayoutLoad`. You should import those from `./$types` (see [generated types](https://kit.svelte.dev/docs/types#generated-types)) * rather than using `Load` directly. diff --git a/packages/kit/types/internal.d.ts b/packages/kit/types/internal.d.ts index 508a7bb6ee51..109932bc70a0 100644 --- a/packages/kit/types/internal.d.ts +++ b/packages/kit/types/internal.d.ts @@ -3,7 +3,6 @@ import { SvelteComponent } from 'svelte/internal'; import { Action, Config, - ExternalFetch, ServerLoad, Handle, HandleError, @@ -14,7 +13,8 @@ import { ResolveOptions, Server, ServerInitOptions, - SSRManifest + SSRManifest, + HandleFetch } from './index.js'; import { HttpMethod, @@ -90,9 +90,9 @@ export type CSRRoute = { export type GetParams = (match: RegExpExecArray) => Record; export interface Hooks { - externalFetch: ExternalFetch; handle: Handle; handleError: HandleError; + handleFetch: HandleFetch; } export interface ImportNode { From cc4d4f3dc2d9b7b24215e50b6f12c0768b798f43 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 4 Sep 2022 12:49:52 -0400 Subject: [PATCH 02/13] fix some stuff --- packages/kit/src/runtime/server/page/fetch.js | 6 ++--- packages/kit/test/apps/basics/src/hooks.js | 5 +--- packages/kit/test/apps/basics/test/test.js | 26 ++++++++++--------- 3 files changed, 18 insertions(+), 19 deletions(-) diff --git a/packages/kit/src/runtime/server/page/fetch.js b/packages/kit/src/runtime/server/page/fetch.js index 66a0a9082053..13d53ceb14b3 100644 --- a/packages/kit/src/runtime/server/page/fetch.js +++ b/packages/kit/src/runtime/server/page/fetch.js @@ -198,9 +198,9 @@ export function create_fetch({ event, options, state, route, prerender_default } } fetched.push({ - url: response.url.startsWith(event.url.origin) - ? response.url.slice(event.url.origin.length) - : response.url, + url: request.url.startsWith(event.url.origin) + ? request.url.slice(event.url.origin.length) + : request.url, method: request.method, body, response: { diff --git a/packages/kit/test/apps/basics/src/hooks.js b/packages/kit/test/apps/basics/src/hooks.js index 4cb664e2ce54..d78d9c219920 100644 --- a/packages/kit/test/apps/basics/src/hooks.js +++ b/packages/kit/test/apps/basics/src/hooks.js @@ -51,10 +51,7 @@ export const handle = sequence( /** @type {import('@sveltejs/kit').HandleFetch} */ export async function handleFetch({ request, fetch }) { if (request.url.endsWith('/server-fetch-request.json')) { - request = new Request( - request.url.replace('/server-fetch-request.json', '/server-fetch-request-modified.json'), - request - ); + request = new Request('/server-fetch-request-modified.json', request); } return fetch(request); diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 6e878d5ec138..826273369cff 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -819,18 +819,20 @@ test.describe('Load', () => { const json = /** @type {string} */ (await page.textContent('pre')); const headers = JSON.parse(json); - expect(headers).toEqual({ - // the referer will be the previous page in the client-side - // navigation case - referer: `${baseURL}/load`, - // these headers aren't particularly useful, but they allow us to verify - // that page headers are being forwarded - 'sec-fetch-dest': - browserName === 'webkit' ? undefined : javaScriptEnabled ? 'empty' : 'document', - 'sec-fetch-mode': - browserName === 'webkit' ? undefined : javaScriptEnabled ? 'cors' : 'navigate', - connection: javaScriptEnabled ? 'keep-alive' : undefined - }); + if (javaScriptEnabled) { + expect(headers).toEqual({ + // the referer will be the previous page in the client-side + // navigation case + referer: `${baseURL}/load`, + // these headers aren't particularly useful, but they allow us to verify + // that page headers are being forwarded + 'sec-fetch-dest': browserName === 'webkit' ? undefined : 'empty', + 'sec-fetch-mode': browserName === 'webkit' ? undefined : 'cors', + connection: 'keep-alive' + }); + } else { + expect(headers).toEqual({}); + } }); test('exposes rawBody to endpoints', async ({ page, clicknav }) => { From 470996a86d32df0a9fe4c3ca2ce0abc3ec049e09 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 4 Sep 2022 13:15:04 -0400 Subject: [PATCH 03/13] fix --- packages/kit/src/runtime/server/page/fetch.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/kit/src/runtime/server/page/fetch.js b/packages/kit/src/runtime/server/page/fetch.js index 13d53ceb14b3..1941e56a3d39 100644 --- a/packages/kit/src/runtime/server/page/fetch.js +++ b/packages/kit/src/runtime/server/page/fetch.js @@ -28,7 +28,7 @@ export function create_fetch({ event, options, state, route, prerender_default } init ); - const body = init?.body; + const request_body = init?.body; /** @type {import('types').PrerenderDependency} */ let dependency; @@ -139,7 +139,7 @@ export function create_fetch({ event, options, state, route, prerender_default } } } - if (body && typeof body !== 'string') { + if (request_body && typeof request_body !== 'string') { // TODO is this still necessary? we just bail out below // per https://developer.mozilla.org/en-US/docs/Web/API/Request/Request, this can be a // Blob, BufferSource, FormData, URLSearchParams, USVString, or ReadableStream object. @@ -202,7 +202,7 @@ export function create_fetch({ event, options, state, route, prerender_default } ? request.url.slice(event.url.origin.length) : request.url, method: request.method, - body, + body: /** @type {string | undefined} */ (request_body), response: { status: status_number, statusText: response.statusText, From 49b43e01b6335e1648871e8ee31fb661e35f2724 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 4 Sep 2022 13:20:38 -0400 Subject: [PATCH 04/13] fix --- packages/kit/src/runtime/server/page/fetch.js | 12 +++++------- packages/kit/test/apps/basics/src/hooks.js | 5 ++++- packages/kit/test/apps/basics/test/test.js | 2 -- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/kit/src/runtime/server/page/fetch.js b/packages/kit/src/runtime/server/page/fetch.js index 1941e56a3d39..afd4b5c92145 100644 --- a/packages/kit/src/runtime/server/page/fetch.js +++ b/packages/kit/src/runtime/server/page/fetch.js @@ -23,10 +23,7 @@ export function create_fetch({ event, options, state, route, prerender_default } /** @type {typeof fetch} */ const fetcher = async (info, init) => { - const request = normalize_fetch_input( - typeof info === 'string' ? new URL(info, event.url) : info, - init - ); + const request = normalize_fetch_input(info, init, event.url); const request_body = init?.body; @@ -37,7 +34,7 @@ export function create_fetch({ event, options, state, route, prerender_default } event, request, fetch: async (info, init) => { - const request = normalize_fetch_input(info, init); + const request = normalize_fetch_input(info, init, event.url); const url = new URL(request.url); @@ -259,11 +256,12 @@ export function create_fetch({ event, options, state, route, prerender_default } /** * @param {RequestInfo | URL} info * @param {RequestInit | undefined} init + * @param {URL} url */ -function normalize_fetch_input(info, init) { +function normalize_fetch_input(info, init, url) { if (info instanceof Request) { return info; } - return new Request(info, init); + return new Request(typeof info === 'string' ? new URL(info, url) : info, init); } diff --git a/packages/kit/test/apps/basics/src/hooks.js b/packages/kit/test/apps/basics/src/hooks.js index d78d9c219920..4cb664e2ce54 100644 --- a/packages/kit/test/apps/basics/src/hooks.js +++ b/packages/kit/test/apps/basics/src/hooks.js @@ -51,7 +51,10 @@ export const handle = sequence( /** @type {import('@sveltejs/kit').HandleFetch} */ export async function handleFetch({ request, fetch }) { if (request.url.endsWith('/server-fetch-request.json')) { - request = new Request('/server-fetch-request-modified.json', request); + request = new Request( + request.url.replace('/server-fetch-request.json', '/server-fetch-request-modified.json'), + request + ); } return fetch(request); diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 826273369cff..09fddf21afec 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -773,8 +773,6 @@ test.describe('Load', () => { const requested_urls = []; const { port, close } = await start_server(async (req, res) => { - console.error(`req.url ${req.url}`); - requested_urls.push(req.url); if (req.url === '/server-fetch-request-modified.json') { From 6feb8ce59f89af2d1b0ee33e3fac255740095946 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 4 Sep 2022 13:27:41 -0400 Subject: [PATCH 05/13] remove unused ExternalFetch type --- packages/kit/types/index.d.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index 28897f545d14..cade25ac832c 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -176,10 +176,6 @@ export interface KitConfig { }; } -export interface ExternalFetch { - (req: Request): Promise; -} - export interface Handle { (input: { event: RequestEvent; From a4a847cc16f3a060f48afff2d551d1f9ccbf6515 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 4 Sep 2022 13:46:59 -0400 Subject: [PATCH 06/13] fix --- packages/kit/src/runtime/server/page/fetch.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/server/page/fetch.js b/packages/kit/src/runtime/server/page/fetch.js index afd4b5c92145..5b6b5e9d1a83 100644 --- a/packages/kit/src/runtime/server/page/fetch.js +++ b/packages/kit/src/runtime/server/page/fetch.js @@ -85,8 +85,9 @@ export function create_fetch({ event, options, state, route, prerender_default } // handle fetch requests for static assets. e.g. prebaked data, etc. // we need to support everything the browser's fetch supports const prefix = options.paths.assets || options.paths.base; + const decoded = decodeURIComponent(url.pathname); const filename = ( - url.pathname.startsWith(prefix) ? url.pathname.slice(prefix.length) : url.pathname + decoded.startsWith(prefix) ? decoded.slice(prefix.length) : decoded ).slice(1); const filename_html = `${filename}/index.html`; // path may also match path/index.html From 9be52c08fbd60597b91e80b948edd6bd1d19d6e5 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 4 Sep 2022 14:32:50 -0400 Subject: [PATCH 07/13] update docs --- documentation/docs/06-hooks.md | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/documentation/docs/06-hooks.md b/documentation/docs/06-hooks.md index 7bf25b938010..32fce726cfdd 100644 --- a/documentation/docs/06-hooks.md +++ b/documentation/docs/06-hooks.md @@ -2,7 +2,7 @@ title: Hooks --- -An optional `src/hooks.js` (or `src/hooks.ts`, or `src/hooks/index.js`) file exports three functions, all optional, that run on the server — `handle`, `handleError` and `externalFetch`. +An optional `src/hooks.js` (or `src/hooks.ts`, or `src/hooks/index.js`) file exports three functions, all optional, that run on the server — `handle`, `handleError` and `handleFetch`. > The location of this file can be [configured](/docs/configuration) as `config.kit.files.hooks` @@ -101,15 +101,29 @@ export function handleError({ error, event }) { > `handleError` is only called for _unexpected_ errors. It is not called for errors created with the [`error`](/docs/modules#sveltejs-kit-error) function imported from `@sveltejs/kit`, as these are _expected_ errors. -### externalFetch +### handleFetch -This function allows you to modify (or replace) a `fetch` request for an external resource that happens inside a `load` function that runs on the server (or during pre-rendering). +This function allows you to modify (or replace) a `fetch` request that happens inside a `load` function that runs on the server (or during pre-rendering). -For example, your `load` function might make a request to a public URL like `https://api.yourapp.com` when the user performs a client-side navigation to the respective page, but during SSR it might make sense to hit the API directly (bypassing whatever proxies and load balancers sit between it and the public internet). +For example, you might need to include custom headers that are added by a proxy that sits in front of your app: ```js -/** @type {import('@sveltejs/kit').ExternalFetch} */ -export async function externalFetch(request) { +// @errors: 2345 +/** @type {import('@sveltejs/kit').HandleFetch} */ +export async function handleFetch({ event, request, fetch }) { + const name = 'x-geolocation-city'; + const value = event.request.headers.get(name); + request.headers.set(name, value); + + return fetch(request); +} +``` + +Or your `load` function might make a request to a public URL like `https://api.yourapp.com` when the user performs a client-side navigation to the respective page, but during SSR it might make sense to hit the API directly (bypassing whatever proxies and load balancers sit between it and the public internet). + +```js +/** @type {import('@sveltejs/kit').HandleFetch} */ +export async function handleFetch({ request, fetch }) { if (request.url.startsWith('https://api.yourapp.com/')) { // clone the original request, but change the URL request = new Request( From b96a5fd387b7319f647031cf021886fb91ab9d87 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 4 Sep 2022 16:26:16 -0400 Subject: [PATCH 08/13] add some docs on credentials --- documentation/docs/06-hooks.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/documentation/docs/06-hooks.md b/documentation/docs/06-hooks.md index 32fce726cfdd..b045bc243f5d 100644 --- a/documentation/docs/06-hooks.md +++ b/documentation/docs/06-hooks.md @@ -135,3 +135,22 @@ export async function handleFetch({ request, fetch }) { return fetch(request); } ``` + +#### Credentials + +For same-origin requests, SvelteKit's `fetch` implementation will forward `cookie` and `authorization` headers unless the `credentials` option is set to `"omit"`. + +For cross-origin requests, `cookie` will be included if the request URL belongs to a subdomain of the app — for example if your app is on `my-domain.com`, and your API is on `api.my-domain.com`, cookies will be included in the request. + +If your app and your API are on sibling subdomains — `www.my-domain.com` and `api.my-domain.com` for example — then a cookie belonging to a common parent domain like `my-domain.com` will _not_ be included, because SvelteKit has no way to know which domain the cookie belongs to. In these cases you will need to manually include the cookie using `handleFetch`: + +```js +/** @type {import('@sveltejs/kit').HandleFetch} */ +export async function handleFetch({ event, request, fetch }) { + if (request.url.startsWith('https://api.my-domain.com/')) { + request.headers.set('cookie', event.request.headers.get('cookie')); + } + + return fetch(request); +} +``` From b239acfba739844e5ac603304617da04868e66ef Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sun, 4 Sep 2022 16:59:28 -0400 Subject: [PATCH 09/13] oops --- documentation/docs/06-hooks.md | 1 + 1 file changed, 1 insertion(+) diff --git a/documentation/docs/06-hooks.md b/documentation/docs/06-hooks.md index b045bc243f5d..18f191961aa1 100644 --- a/documentation/docs/06-hooks.md +++ b/documentation/docs/06-hooks.md @@ -145,6 +145,7 @@ For cross-origin requests, `cookie` will be included if the request URL belongs If your app and your API are on sibling subdomains — `www.my-domain.com` and `api.my-domain.com` for example — then a cookie belonging to a common parent domain like `my-domain.com` will _not_ be included, because SvelteKit has no way to know which domain the cookie belongs to. In these cases you will need to manually include the cookie using `handleFetch`: ```js +// @errors: 2345 /** @type {import('@sveltejs/kit').HandleFetch} */ export async function handleFetch({ event, request, fetch }) { if (request.url.startsWith('https://api.my-domain.com/')) { From f24ba0dc0fb9011648817617cffb58bd305ee4e6 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 5 Sep 2022 08:07:03 -0400 Subject: [PATCH 10/13] respect explicit cookie header --- packages/kit/src/runtime/server/page/fetch.js | 32 +++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/packages/kit/src/runtime/server/page/fetch.js b/packages/kit/src/runtime/server/page/fetch.js index 5b6b5e9d1a83..ff71384bc051 100644 --- a/packages/kit/src/runtime/server/page/fetch.js +++ b/packages/kit/src/runtime/server/page/fetch.js @@ -19,7 +19,7 @@ export function create_fetch({ event, options, state, route, prerender_default } const initial_cookies = cookie.parse(event.request.headers.get('cookie') || ''); /** @type {import('set-cookie-parser').Cookie[]} */ - const cookies = []; + const new_cookies = []; /** @type {typeof fetch} */ const fetcher = async (info, init) => { @@ -51,8 +51,16 @@ export function create_fetch({ event, options, state, route, prerender_default } `.${url.hostname}`.endsWith(`.${event.url.hostname}`) && request.credentials !== 'omit' ) { - const cookie = event.request.headers.get('cookie'); - if (cookie) request.headers.set('cookie', cookie); + const combined_cookies = { + ...cookie.parse(event.request.headers.get('cookie') ?? ''), + ...cookie.parse(request.headers.get('cookie') ?? '') + }; + + const cookie_header = Object.entries(combined_cookies) + .map(([name, value]) => `${name}=${value}`) + .join('; '); + + if (cookie_header) request.headers.set('cookie', cookie_header); } let response = await fetch(request); @@ -117,19 +125,25 @@ export function create_fetch({ event, options, state, route, prerender_default } // added via set-cookie const combined_cookies = { ...initial_cookies }; - for (const cookie of cookies) { + for (const cookie of new_cookies) { if (!domain_matches(event.url.hostname, cookie.domain)) continue; if (!path_matches(url.pathname, cookie.path)) continue; combined_cookies[cookie.name] = cookie.value; } - const cookie = Object.entries(combined_cookies) + // add cookies that were included in this request + const explicit_cookies = cookie.parse(request.headers.get('cookie') || ''); + for (const key in explicit_cookies) { + combined_cookies[key] = explicit_cookies[key]; + } + + const cookie_header = Object.entries(combined_cookies) .map(([name, value]) => `${name}=${value}`) .join('; '); - if (cookie) { - request.headers.set('cookie', cookie); + if (cookie_header) { + request.headers.set('cookie', cookie_header); } if (authorization && !request.headers.has('authorization')) { @@ -163,7 +177,7 @@ export function create_fetch({ event, options, state, route, prerender_default } const set_cookie = response.headers.get('set-cookie'); if (set_cookie) { - cookies.push( + new_cookies.push( ...set_cookie_parser .splitCookiesString(set_cookie) .map((str) => set_cookie_parser.parseString(str)) @@ -251,7 +265,7 @@ export function create_fetch({ event, options, state, route, prerender_default } return proxy; }; - return { fetcher, fetched, cookies }; + return { fetcher, fetched, cookies: new_cookies }; } /** From a2731f11c7a2d9367967f8250c7a5953b3b3c785 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 5 Sep 2022 09:28:13 -0400 Subject: [PATCH 11/13] apply set-cookie to cross-origin requests --- packages/kit/src/runtime/server/page/fetch.js | 76 +++++++++---------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/packages/kit/src/runtime/server/page/fetch.js b/packages/kit/src/runtime/server/page/fetch.js index ff71384bc051..8ffe846d9a0e 100644 --- a/packages/kit/src/runtime/server/page/fetch.js +++ b/packages/kit/src/runtime/server/page/fetch.js @@ -19,7 +19,36 @@ export function create_fetch({ event, options, state, route, prerender_default } const initial_cookies = cookie.parse(event.request.headers.get('cookie') || ''); /** @type {import('set-cookie-parser').Cookie[]} */ - const new_cookies = []; + const set_cookies = []; + + /** + * @param {URL} url + * @param {string | null} header + */ + function get_cookie_header(url, header) { + /** @type {Record} */ + const new_cookies = {}; + + for (const cookie of set_cookies) { + if (!domain_matches(url.hostname, cookie.domain)) continue; + if (!path_matches(url.pathname, cookie.path)) continue; + + new_cookies[cookie.name] = cookie.value; + } + + // cookies from explicit `cookie` header take precedence over cookies previously set + // during this load with `set-cookie`, which take precedence over the cookies + // sent by the user agent + const combined_cookies = { + ...initial_cookies, + ...new_cookies, + ...cookie.parse(header ?? '') + }; + + return Object.entries(combined_cookies) + .map(([name, value]) => `${name}=${value}`) + .join('; '); + } /** @type {typeof fetch} */ const fetcher = async (info, init) => { @@ -51,16 +80,8 @@ export function create_fetch({ event, options, state, route, prerender_default } `.${url.hostname}`.endsWith(`.${event.url.hostname}`) && request.credentials !== 'omit' ) { - const combined_cookies = { - ...cookie.parse(event.request.headers.get('cookie') ?? ''), - ...cookie.parse(request.headers.get('cookie') ?? '') - }; - - const cookie_header = Object.entries(combined_cookies) - .map(([name, value]) => `${name}=${value}`) - .join('; '); - - if (cookie_header) request.headers.set('cookie', cookie_header); + const cookie = get_cookie_header(url, request.headers.get('cookie')); + if (cookie) request.headers.set('cookie', cookie); } let response = await fetch(request); @@ -119,33 +140,12 @@ export function create_fetch({ event, options, state, route, prerender_default } } if (request.credentials !== 'omit') { - const authorization = event.request.headers.get('authorization'); - - // combine cookies from the initiating request with any that were - // added via set-cookie - const combined_cookies = { ...initial_cookies }; - - for (const cookie of new_cookies) { - if (!domain_matches(event.url.hostname, cookie.domain)) continue; - if (!path_matches(url.pathname, cookie.path)) continue; - - combined_cookies[cookie.name] = cookie.value; - } - - // add cookies that were included in this request - const explicit_cookies = cookie.parse(request.headers.get('cookie') || ''); - for (const key in explicit_cookies) { - combined_cookies[key] = explicit_cookies[key]; - } - - const cookie_header = Object.entries(combined_cookies) - .map(([name, value]) => `${name}=${value}`) - .join('; '); - - if (cookie_header) { - request.headers.set('cookie', cookie_header); + const cookie = get_cookie_header(url, request.headers.get('cookie')); + if (cookie) { + request.headers.set('cookie', cookie); } + const authorization = event.request.headers.get('authorization'); if (authorization && !request.headers.has('authorization')) { request.headers.set('authorization', authorization); } @@ -177,7 +177,7 @@ export function create_fetch({ event, options, state, route, prerender_default } const set_cookie = response.headers.get('set-cookie'); if (set_cookie) { - new_cookies.push( + set_cookies.push( ...set_cookie_parser .splitCookiesString(set_cookie) .map((str) => set_cookie_parser.parseString(str)) @@ -265,7 +265,7 @@ export function create_fetch({ event, options, state, route, prerender_default } return proxy; }; - return { fetcher, fetched, cookies: new_cookies }; + return { fetcher, fetched, cookies: set_cookies }; } /** From 615ffab675723e74ac228d98b5435531ba8aeed9 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 5 Sep 2022 09:56:01 -0400 Subject: [PATCH 12/13] point to migration guide --- packages/kit/src/exports/vite/build/build_server.js | 2 +- packages/kit/src/exports/vite/dev/index.js | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/kit/src/exports/vite/build/build_server.js b/packages/kit/src/exports/vite/build/build_server.js index b9a37fce15fb..b3d091474e3e 100644 --- a/packages/kit/src/exports/vite/build/build_server.js +++ b/packages/kit/src/exports/vite/build/build_server.js @@ -113,7 +113,7 @@ export class Server { // TODO remove this for 1.0 if (module.externalFetch) { - throw new Error('externalFetch has been removed — use handleFetch instead'); // TODO add migration guide + throw new Error('externalFetch has been removed — use handleFetch instead. See https://github.com/sveltejs/kit/pull/6565 for details'); } this.options.hooks = { diff --git a/packages/kit/src/exports/vite/dev/index.js b/packages/kit/src/exports/vite/dev/index.js index f7c1313faa8c..c1caf2250cac 100644 --- a/packages/kit/src/exports/vite/dev/index.js +++ b/packages/kit/src/exports/vite/dev/index.js @@ -313,7 +313,9 @@ export async function dev(vite, vite_config, svelte_config, illegal_imports) { // TODO remove for 1.0 // @ts-expect-error if (user_hooks.externalFetch) { - throw new Error('externalFetch has been removed — use handleFetch instead'); // TODO add migration guide + throw new Error( + 'externalFetch has been removed — use handleFetch instead. See https://github.com/sveltejs/kit/pull/6565 for details' + ); } /** @type {import('types').Hooks} */ From ecf3ecba8d7e179b2b3d375ce5db00fb89f226a8 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 5 Sep 2022 10:17:17 -0400 Subject: [PATCH 13/13] changeset --- .changeset/spotty-phones-love.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/spotty-phones-love.md diff --git a/.changeset/spotty-phones-love.md b/.changeset/spotty-phones-love.md new file mode 100644 index 000000000000..10992917e4a0 --- /dev/null +++ b/.changeset/spotty-phones-love.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +[breaking] Replace `externalFetch` with `handleFetch`