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` diff --git a/documentation/docs/06-hooks.md b/documentation/docs/06-hooks.md index 7bf25b938010..18f191961aa1 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( @@ -121,3 +135,23 @@ export async function externalFetch(request) { 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 +// @errors: 2345 +/** @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); +} +``` diff --git a/packages/kit/src/exports/vite/build/build_server.js b/packages/kit/src/exports/vite/build/build_server.js index 137d358eeb75..b3d091474e3e 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. See https://github.com/sveltejs/kit/pull/6565 for details'); + } + 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 aa1bc6bd836e..c1caf2250cac 100644 --- a/packages/kit/src/exports/vite/dev/index.js +++ b/packages/kit/src/exports/vite/dev/index.js @@ -310,6 +310,14 @@ 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. See https://github.com/sveltejs/kit/pull/6565 for details' + ); + } + /** @type {import('types').Hooks} */ const hooks = { handle, @@ -324,7 +332,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..8ffe846d9a0e 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'; /** @@ -20,185 +19,165 @@ 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 set_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 - }; - } + /** + * @param {URL} url + * @param {string | null} header + */ + function get_cookie_header(url, header) { + /** @type {Record} */ + const new_cookies = {}; - 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); - } + 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; } - const resolved = resolve(event.url.pathname, requested.split('?')[0]).replace(/#.+$/, ''); + // 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) => { + const request = normalize_fetch_input(info, init, event.url); - /** @type {Response} */ - let response; + const request_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, event.url); - 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 = get_cookie_header(url, 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 decoded = decodeURIComponent(url.pathname); + const filename = ( + decoded.startsWith(prefix) ? decoded.slice(prefix.length) : decoded + ).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; - } - 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); - } + if (request.credentials !== 'omit') { + const cookie = get_cookie_header(url, request.headers.get('cookie')); + if (cookie) { + request.headers.set('cookie', cookie); + } - // 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'); + const authorization = event.request.headers.get('authorization'); + if (authorization && !request.headers.has('authorization')) { + request.headers.set('authorization', authorization); + } + } - const external_request = new Request(requested, /** @type {RequestInit} */ (opts)); - response = await options.hooks.externalFetch.call(null, external_request); + 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. + // 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'); + } - if (opts.mode === 'no-cors') { - response = new Response('', { - status: response.status, - statusText: response.statusText, - headers: response.headers + response = await respond(request, options, { + prerender_default, + ...state, + initiator: route }); - } 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` - ); - } + + 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) { - cookies.push( + set_cookies.push( ...set_cookie_parser .splitCookiesString(set_cookie) .map((str) => set_cookie_parser.parseString(str)) @@ -220,7 +199,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 +210,11 @@ export function create_fetch({ event, options, state, route, prerender_default } } fetched.push({ - url: requested, - method: opts.method || 'GET', - body: opts.body, + url: request.url.startsWith(event.url.origin) + ? 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, @@ -284,5 +265,18 @@ export function create_fetch({ event, options, state, route, prerender_default } return proxy; }; - return { fetcher, fetched, cookies }; + return { fetcher, fetched, cookies: set_cookies }; +} + +/** + * @param {RequestInfo | URL} info + * @param {RequestInit | undefined} init + * @param {URL} url + */ +function normalize_fetch_input(info, init, url) { + if (info instanceof Request) { + return info; + } + + 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 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 9244aeb7acc1..373423798a10 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -790,7 +790,6 @@ test.describe('Load', () => { const requested_urls = []; const { port, close } = await start_server(async (req, res) => { - if (!req.url) throw new Error('Incomplete request'); requested_urls.push(req.url); if (req.url === '/server-fetch-request-modified.json') { @@ -835,18 +834,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 }) => { diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index c09238532ea0..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; @@ -191,6 +187,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 {