From 8f41797111b9ea7f49024a1a226d395f306972be Mon Sep 17 00:00:00 2001 From: Tee Ming Date: Thu, 23 Apr 2026 03:08:20 +0800 Subject: [PATCH 1/4] prefer default error page when failing to decode the uri pathname --- packages/kit/src/runtime/server/respond.js | 138 ++++++++++++--------- 1 file changed, 79 insertions(+), 59 deletions(-) diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index f2ad42e17730..a89de93e1a1d 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -233,6 +233,7 @@ export async function internal_respond(request, options, manifest, state) { }); } + /** @type {string | null} */ let resolved_path = url.pathname; if (!remote_id) { @@ -257,77 +258,79 @@ export async function internal_respond(request, options, manifest, state) { try { resolved_path = decode_pathname(resolved_path); } catch { - return text('Malformed URI', { status: 400 }); + resolved_path = null; } - // try to serve the rerouted prerendered resource if it exists - if ( - // the resolved path has been decoded so it should be compared to the decoded url pathname - resolved_path !== decode_pathname(url.pathname) && - !state.prerendering?.fallback && - has_prerendered_path(manifest, resolved_path) - ) { - const url = new URL(request.url); - url.pathname = is_data_request - ? add_data_suffix(resolved_path) - : is_route_resolution_request - ? add_resolution_suffix(resolved_path) - : resolved_path; + /** @type {import('types').SSRRoute | null} */ + let route = null; - try { - // `fetch` automatically decodes the body, so we need to delete the related headers to not break the response - // Also see https://github.com/sveltejs/kit/issues/12197 for more info (we should fix this more generally at some point) - const response = await fetch(url, request); - const headers = new Headers(response.headers); - if (headers.has('content-encoding')) { - headers.delete('content-encoding'); - headers.delete('content-length'); - } + if (resolved_path) { + // try to serve the rerouted prerendered resource if it exists + if ( + // the resolved path has been decoded so it should be compared to the decoded url pathname + resolved_path !== decode_pathname(url.pathname) && + !state.prerendering?.fallback && + has_prerendered_path(manifest, resolved_path) + ) { + const url = new URL(request.url); + url.pathname = is_data_request + ? add_data_suffix(resolved_path) + : is_route_resolution_request + ? add_resolution_suffix(resolved_path) + : resolved_path; - return new Response(response.body, { - headers, - status: response.status, - statusText: response.statusText - }); - } catch (error) { - return await handle_fatal_error(event, event_state, options, error); - } - } + try { + // `fetch` automatically decodes the body, so we need to delete the related headers to not break the response + // Also see https://github.com/sveltejs/kit/issues/12197 for more info (we should fix this more generally at some point) + const response = await fetch(url, request); + const headers = new Headers(response.headers); + if (headers.has('content-encoding')) { + headers.delete('content-encoding'); + headers.delete('content-length'); + } - /** @type {import('types').SSRRoute | null} */ - let route = null; + return new Response(response.body, { + headers, + status: response.status, + statusText: response.statusText + }); + } catch (error) { + return await handle_fatal_error(event, event_state, options, error); + } + } - if (base && !state.prerendering?.fallback) { - if (!resolved_path.startsWith(base)) { - return text('Not found', { status: 404 }); + if (base && !state.prerendering?.fallback) { + if (!resolved_path.startsWith(base)) { + return text('Not found', { status: 404 }); + } + resolved_path = resolved_path.slice(base.length) || '/'; } - resolved_path = resolved_path.slice(base.length) || '/'; - } - if (is_route_resolution_request) { - return resolve_route(resolved_path, new URL(request.url), manifest); - } + if (is_route_resolution_request) { + return resolve_route(resolved_path, new URL(request.url), manifest); + } - if (resolved_path === `/${app_dir}/env.js`) { - return get_public_env(request); - } + if (resolved_path === `/${app_dir}/env.js`) { + return get_public_env(request); + } - if (!remote_id && resolved_path.startsWith(`/${app_dir}`)) { - // Ensure that 404'd static assets are not cached - some adapters might apply caching by default - const headers = new Headers(); - headers.set('cache-control', 'public, max-age=0, must-revalidate'); - return text('Not found', { status: 404, headers }); - } + if (!remote_id && resolved_path.startsWith(`/${app_dir}`)) { + // Ensure that 404'd static assets are not cached - some adapters might apply caching by default + const headers = new Headers(); + headers.set('cache-control', 'public, max-age=0, must-revalidate'); + return text('Not found', { status: 404, headers }); + } - if (!state.prerendering?.fallback) { - // TODO this could theoretically break — should probably be inside a try-catch - const matchers = await manifest._.matchers(); - const result = find_route(resolved_path, manifest._.routes, matchers); + if (!state.prerendering?.fallback) { + // TODO this could theoretically break — should probably be inside a try-catch + const matchers = await manifest._.matchers(); + const result = find_route(resolved_path, manifest._.routes, matchers); - if (result) { - route = result.route; - event.route = { id: route.id }; - event.params = result.params; + if (result) { + route = result.route; + event.route = { id: route.id }; + event.params = result.params; + } } } @@ -556,6 +559,23 @@ export async function internal_respond(request, options, manifest, state) { }; } + if (resolved_path === null) { + return await respond_with_error({ + event, + event_state, + options, + manifest, + state, + status: 400, + error: new SvelteKitError( + 400, + 'Malformed URI', + `Failed to decode URI: ${event.url.pathname}` + ), + resolve_opts + }); + } + if (options.hash_routing || state.prerendering?.fallback) { return await render_response({ event, From 3bdd4ad68c0ca1eb011dd158286a4ee150b4665b Mon Sep 17 00:00:00 2001 From: Tee Ming Date: Thu, 23 Apr 2026 03:08:52 +0800 Subject: [PATCH 2/4] changeset --- .changeset/red-bugs-bake.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/red-bugs-bake.md diff --git a/.changeset/red-bugs-bake.md b/.changeset/red-bugs-bake.md new file mode 100644 index 000000000000..8e7085c7c9ee --- /dev/null +++ b/.changeset/red-bugs-bake.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +fix: prefer default error page when failing to decode the URL pathname From 3fe9f4a7fe4b9da3c5c683a9f0ef41f4d1d83417 Mon Sep 17 00:00:00 2001 From: Tee Ming Date: Fri, 22 May 2026 01:20:19 +0800 Subject: [PATCH 3/4] try this --- packages/kit/src/runtime/server/respond.js | 381 ++++++++++----------- 1 file changed, 188 insertions(+), 193 deletions(-) diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index 4c89723a18f7..8b63dc707d37 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -248,98 +248,99 @@ export async function internal_respond(request, options, manifest, state) { } } + /** @type {import('types').RequiredResolveOptions} */ + let resolve_opts = { + transformPageChunk: default_transform, + filterSerializedResponseHeaders: default_filter, + preload: default_preload + }; + + /** @type {import('types').TrailingSlash} */ + let trailing_slash = 'never'; + + /** @type {PageNodes | undefined} */ + let page_nodes; + try { resolved_path = decode_pathname(resolved_path); } catch { resolved_path = null; + return await handle(); } /** @type {import('types').SSRRoute | null} */ let route = null; - if (resolved_path) { - // try to serve the rerouted prerendered resource if it exists - if ( - // the resolved path has been decoded so it should be compared to the decoded url pathname - resolved_path !== decode_pathname(url.pathname) && - !state.prerendering?.fallback && - has_prerendered_path(manifest, resolved_path) - ) { - const url = new URL(request.url); - url.pathname = is_data_request - ? add_data_suffix(resolved_path) - : is_route_resolution_request - ? add_resolution_suffix(resolved_path) - : resolved_path; + // try to serve the rerouted prerendered resource if it exists + if ( + // the resolved path has been decoded so it should be compared to the decoded url pathname + resolved_path !== decode_pathname(url.pathname) && + !state.prerendering?.fallback && + has_prerendered_path(manifest, resolved_path) + ) { + const url = new URL(request.url); + url.pathname = is_data_request + ? add_data_suffix(resolved_path) + : is_route_resolution_request + ? add_resolution_suffix(resolved_path) + : resolved_path; - try { - // `fetch` automatically decodes the body, so we need to delete the related headers to not break the response - // Also see https://github.com/sveltejs/kit/issues/12197 for more info (we should fix this more generally at some point) - const response = await fetch(url, request); - const headers = new Headers(response.headers); - if (headers.has('content-encoding')) { - headers.delete('content-encoding'); - headers.delete('content-length'); - } - - return new Response(response.body, { - headers, - status: response.status, - statusText: response.statusText - }); - } catch (error) { - return await handle_fatal_error(event, event_state, options, error); - } - } - - if (base && !state.prerendering?.fallback) { - if (!resolved_path.startsWith(base)) { - return text('Not found', { status: 404 }); + try { + // `fetch` automatically decodes the body, so we need to delete the related headers to not break the response + // Also see https://github.com/sveltejs/kit/issues/12197 for more info (we should fix this more generally at some point) + const response = await fetch(url, request); + const headers = new Headers(response.headers); + if (headers.has('content-encoding')) { + headers.delete('content-encoding'); + headers.delete('content-length'); } - resolved_path = resolved_path.slice(base.length) || '/'; - } - if (is_route_resolution_request) { - return resolve_route(resolved_path, new URL(request.url), manifest); + return new Response(response.body, { + headers, + status: response.status, + statusText: response.statusText + }); + } catch (error) { + return await handle_fatal_error(event, event_state, options, error); } + } - if (resolved_path === `/${app_dir}/env.js`) { - return get_public_env(request); + if (base && !state.prerendering?.fallback) { + if (!resolved_path.startsWith(base)) { + return text('Not found', { status: 404 }); } + resolved_path = resolved_path.slice(base.length) || '/'; + } - if (!remote_id && resolved_path.startsWith(`/${app_dir}`)) { - // Ensure that 404'd static assets are not cached - some adapters might apply caching by default - const headers = new Headers(); - headers.set('cache-control', 'public, max-age=0, must-revalidate'); - return text('Not found', { status: 404, headers }); - } + if (is_route_resolution_request) { + return resolve_route(resolved_path, new URL(request.url), manifest); + } - if (!state.prerendering?.fallback) { - // TODO this could theoretically break — should probably be inside a try-catch - const matchers = await manifest._.matchers(); - const result = find_route(resolved_path, manifest._.routes, matchers); + if (resolved_path === `/${app_dir}/env.js`) { + return get_public_env(request); + } - if (result) { - route = result.route; - event.route = { id: route.id }; - event.params = result.params; - } - } + if (!remote_id && resolved_path.startsWith(`/${app_dir}`)) { + // Ensure that 404'd static assets are not cached - some adapters might apply caching by default + const headers = new Headers(); + headers.set('cache-control', 'public, max-age=0, must-revalidate'); + return text('Not found', { status: 404, headers }); } - /** @type {import('types').RequiredResolveOptions} */ - let resolve_opts = { - transformPageChunk: default_transform, - filterSerializedResponseHeaders: default_filter, - preload: default_preload - }; + if (!state.prerendering?.fallback) { + // TODO this could theoretically break — should probably be inside a try-catch + const matchers = await manifest._.matchers(); + const result = find_route(resolved_path, manifest._.routes, matchers); - /** @type {import('types').TrailingSlash} */ - let trailing_slash = 'never'; + if (result) { + route = result.route; + event.route = { id: route.id }; + event.params = result.params; + } + } try { - /** @type {PageNodes | undefined} */ - const page_nodes = route?.page + page_nodes = route?.page ? new PageNodes(await load_page_nodes(route.page, manifest)) : undefined; @@ -404,130 +405,6 @@ export async function internal_respond(request, options, manifest, state) { } } - async function handle() { - set_trailing_slash(trailing_slash); - - if ( - state.prerendering && - !state.prerendering.fallback && - !state.prerendering.inside_reroute - ) { - disable_search(url); - } - - const response = await record_span({ - name: 'sveltekit.handle.root', - attributes: { - 'http.route': event.route.id || 'unknown', - 'http.method': event.request.method, - 'http.url': event.url.href, - 'sveltekit.is_data_request': is_data_request, - 'sveltekit.is_sub_request': event.isSubRequest - }, - fn: async (root_span) => { - const traced_event = { - ...event, - tracing: { - enabled: __SVELTEKIT_SERVER_TRACING_ENABLED__, - root: root_span, - current: root_span - } - }; - - return await with_request_store({ event: traced_event, state: event_state }, () => - options.hooks.handle({ - event: traced_event, - resolve: (event, opts) => { - return record_span({ - name: 'sveltekit.resolve', - attributes: { - 'http.route': event.route.id || 'unknown' - }, - fn: (resolve_span) => { - // counter-intuitively, we need to clear the event, so that it's not - // e.g. accessible when loading modules needed to handle the request - return with_request_store(null, () => - resolve(merge_tracing(event, resolve_span), page_nodes, opts).then( - (response) => { - // add headers/cookies here, rather than inside `resolve`, so that we - // can do it once for all responses instead of once per `return` - for (const key in headers) { - const value = headers[key]; - response.headers.set(key, /** @type {string} */ (value)); - } - - add_cookies_to_headers(response.headers, new_cookies.values()); - - if (state.prerendering && event.route.id !== null) { - response.headers.set('x-sveltekit-routeid', encodeURI(event.route.id)); - } - - resolve_span.setAttributes({ - 'http.response.status_code': response.status, - 'http.response.body.size': - response.headers.get('content-length') || 'unknown' - }); - - return response; - } - ) - ); - } - }); - } - }) - ); - } - }); - - // respond with 304 if etag matches - if (response.status === 200 && response.headers.has('etag')) { - let if_none_match_value = request.headers.get('if-none-match'); - - // ignore W/ prefix https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-None-Match#directives - if (if_none_match_value?.startsWith('W/"')) { - if_none_match_value = if_none_match_value.substring(2); - } - - const etag = /** @type {string} */ (response.headers.get('etag')); - - if (if_none_match_value === etag) { - const headers = new Headers({ etag }); - - // https://datatracker.ietf.org/doc/html/rfc7232#section-4.1 + set-cookie - for (const key of [ - 'cache-control', - 'content-location', - 'date', - 'expires', - 'vary', - 'set-cookie' - ]) { - const value = response.headers.get(key); - if (value) headers.set(key, value); - } - - return new Response(undefined, { - status: 304, - headers - }); - } - } - - // Edge case: If user does `return Response(30x)` in handle hook while processing a data request, - // we need to transform the redirect response to a corresponding JSON response. - if (is_data_request && response.status >= 300 && response.status <= 308) { - const location = response.headers.get('location'); - if (location) { - return redirect_json_response( - new Redirect(/** @type {any} */ (response.status), location) - ); - } - } - - return response; - } - return await handle(); } catch (e) { if (e instanceof Redirect) { @@ -547,6 +424,124 @@ export async function internal_respond(request, options, manifest, state) { return await handle_fatal_error(event, event_state, options, e); } + async function handle() { + set_trailing_slash(trailing_slash); + + if (state.prerendering && !state.prerendering.fallback && !state.prerendering.inside_reroute) { + disable_search(url); + } + + const response = await record_span({ + name: 'sveltekit.handle.root', + attributes: { + 'http.route': event.route.id || 'unknown', + 'http.method': event.request.method, + 'http.url': event.url.href, + 'sveltekit.is_data_request': is_data_request, + 'sveltekit.is_sub_request': event.isSubRequest + }, + fn: async (root_span) => { + const traced_event = { + ...event, + tracing: { + enabled: __SVELTEKIT_SERVER_TRACING_ENABLED__, + root: root_span, + current: root_span + } + }; + + return await with_request_store({ event: traced_event, state: event_state }, () => + options.hooks.handle({ + event: traced_event, + resolve: (event, opts) => { + return record_span({ + name: 'sveltekit.resolve', + attributes: { + 'http.route': event.route.id || 'unknown' + }, + fn: (resolve_span) => { + // counter-intuitively, we need to clear the event, so that it's not + // e.g. accessible when loading modules needed to handle the request + return with_request_store(null, () => + resolve(merge_tracing(event, resolve_span), page_nodes, opts).then( + (response) => { + // add headers/cookies here, rather than inside `resolve`, so that we + // can do it once for all responses instead of once per `return` + for (const key in headers) { + const value = headers[key]; + response.headers.set(key, /** @type {string} */ (value)); + } + + add_cookies_to_headers(response.headers, new_cookies.values()); + + if (state.prerendering && event.route.id !== null) { + response.headers.set('x-sveltekit-routeid', encodeURI(event.route.id)); + } + + resolve_span.setAttributes({ + 'http.response.status_code': response.status, + 'http.response.body.size': + response.headers.get('content-length') || 'unknown' + }); + + return response; + } + ) + ); + } + }); + } + }) + ); + } + }); + + // respond with 304 if etag matches + if (response.status === 200 && response.headers.has('etag')) { + let if_none_match_value = request.headers.get('if-none-match'); + + // ignore W/ prefix https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-None-Match#directives + if (if_none_match_value?.startsWith('W/"')) { + if_none_match_value = if_none_match_value.substring(2); + } + + const etag = /** @type {string} */ (response.headers.get('etag')); + + if (if_none_match_value === etag) { + const headers = new Headers({ etag }); + + // https://datatracker.ietf.org/doc/html/rfc7232#section-4.1 + set-cookie + for (const key of [ + 'cache-control', + 'content-location', + 'date', + 'expires', + 'vary', + 'set-cookie' + ]) { + const value = response.headers.get(key); + if (value) headers.set(key, value); + } + + return new Response(undefined, { + status: 304, + headers + }); + } + } + + // Edge case: If user does `return Response(30x)` in handle hook while processing a data request, + // we need to transform the redirect response to a corresponding JSON response. + if (is_data_request && response.status >= 300 && response.status <= 308) { + const location = response.headers.get('location'); + if (location) { + return redirect_json_response(new Redirect(/** @type {any} */ (response.status), location)); + } + } + + return response; + } + /** * @param {import('@sveltejs/kit').RequestEvent} event * @param {PageNodes | undefined} page_nodes From 10d6d84cf792fbbbc3f19e58329c4c456282ffca Mon Sep 17 00:00:00 2001 From: Tee Ming Date: Fri, 22 May 2026 01:21:14 +0800 Subject: [PATCH 4/4] move this back --- packages/kit/src/runtime/server/respond.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index 8b63dc707d37..d2a29f73ddd8 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -268,9 +268,6 @@ export async function internal_respond(request, options, manifest, state) { return await handle(); } - /** @type {import('types').SSRRoute | null} */ - let route = null; - // try to serve the rerouted prerendered resource if it exists if ( // the resolved path has been decoded so it should be compared to the decoded url pathname @@ -305,6 +302,9 @@ export async function internal_respond(request, options, manifest, state) { } } + /** @type {import('types').SSRRoute | null} */ + let route = null; + if (base && !state.prerendering?.fallback) { if (!resolved_path.startsWith(base)) { return text('Not found', { status: 404 });