From f05e9c64af70e8f245c7c1064e52b097beae0bee Mon Sep 17 00:00:00 2001 From: Chew Tee Ming Date: Mon, 2 Dec 2024 10:57:50 +0800 Subject: [PATCH 01/10] strip internal url data before passing to reroute --- packages/kit/src/runtime/server/respond.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index f81f52ef2557..bef671bd0448 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -82,9 +82,16 @@ export async function respond(request, options, manifest, state) { } // reroute could alter the given URL, so we pass a copy + const url_copy = new URL(url); + if (has_data_suffix(url_copy.pathname)) { + url_copy.pathname = strip_data_suffix(url_copy.pathname) || '/'; + url_copy.searchParams.delete(TRAILING_SLASH_PARAM); + url_copy.searchParams.delete(INVALIDATED_PARAM); + } + let rerouted_path; try { - rerouted_path = options.hooks.reroute({ url: new URL(url) }) ?? url.pathname; + rerouted_path = options.hooks.reroute({ url: url_copy }) ?? url.pathname; } catch { return text('Internal Server Error', { status: 500 From d6a5c730ff1e1d395a0be669a363e04d8c8e6e5e Mon Sep 17 00:00:00 2001 From: Chew Tee Ming Date: Mon, 2 Dec 2024 10:58:34 +0800 Subject: [PATCH 02/10] changeset --- .changeset/curvy-trains-dress.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/curvy-trains-dress.md diff --git a/.changeset/curvy-trains-dress.md b/.changeset/curvy-trains-dress.md new file mode 100644 index 000000000000..2d29ff1b47b2 --- /dev/null +++ b/.changeset/curvy-trains-dress.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +fix: strip internal data before passing URL to `reroute` From 370e321ec9b60d5847b7b81c95f11cfed2d3ffb6 Mon Sep 17 00:00:00 2001 From: Chew Tee Ming Date: Mon, 2 Dec 2024 12:49:37 +0800 Subject: [PATCH 03/10] strip suffix only once --- 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 bef671bd0448..bf47ede55f31 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -83,7 +83,9 @@ export async function respond(request, options, manifest, state) { // reroute could alter the given URL, so we pass a copy const url_copy = new URL(url); - if (has_data_suffix(url_copy.pathname)) { + + const is_data_request = has_data_suffix(url.pathname); + if (is_data_request) { url_copy.pathname = strip_data_suffix(url_copy.pathname) || '/'; url_copy.searchParams.delete(TRAILING_SLASH_PARAM); url_copy.searchParams.delete(INVALIDATED_PARAM); @@ -129,11 +131,9 @@ export async function respond(request, options, manifest, state) { return text('Not found', { status: 404, headers }); } - const is_data_request = has_data_suffix(decoded); /** @type {boolean[] | undefined} */ let invalidated_data_nodes; if (is_data_request) { - decoded = strip_data_suffix(decoded) || '/'; url.pathname = strip_data_suffix(url.pathname) + (url.searchParams.get(TRAILING_SLASH_PARAM) === '1' ? '/' : '') || '/'; From 80819ecc970e883206a76714949f8e4a2a9c7f3a Mon Sep 17 00:00:00 2001 From: Chew Tee Ming Date: Mon, 2 Dec 2024 13:09:51 +0800 Subject: [PATCH 04/10] add test --- packages/kit/test/apps/basics/src/hooks.js | 3 ++- .../src/routes/reroute/invalidate/+page.server.js | 5 +++++ .../basics/src/routes/reroute/invalidate/+page.svelte | 11 +++++++++++ packages/kit/test/apps/basics/test/client.test.js | 6 ++++++ 4 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 packages/kit/test/apps/basics/src/routes/reroute/invalidate/+page.server.js create mode 100644 packages/kit/test/apps/basics/src/routes/reroute/invalidate/+page.svelte diff --git a/packages/kit/test/apps/basics/src/hooks.js b/packages/kit/test/apps/basics/src/hooks.js index 302ab5ec823c..ab90ba2982de 100644 --- a/packages/kit/test/apps/basics/src/hooks.js +++ b/packages/kit/test/apps/basics/src/hooks.js @@ -3,7 +3,8 @@ import { browser } from '$app/environment'; const mapping = { '/reroute/basic/a': '/reroute/basic/b', '/reroute/client-only-redirect/a': '/reroute/client-only-redirect/b', - '/reroute/preload-data/a': '/reroute/preload-data/b' + '/reroute/preload-data/a': '/reroute/preload-data/b', + '/reroute/invalidate/a': '/reroute/invalidate' }; /** @type {import("@sveltejs/kit").Reroute} */ diff --git a/packages/kit/test/apps/basics/src/routes/reroute/invalidate/+page.server.js b/packages/kit/test/apps/basics/src/routes/reroute/invalidate/+page.server.js new file mode 100644 index 000000000000..e410319baae7 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/reroute/invalidate/+page.server.js @@ -0,0 +1,5 @@ +export function load({ isDataRequest }) { + return { + request: isDataRequest + }; +} diff --git a/packages/kit/test/apps/basics/src/routes/reroute/invalidate/+page.svelte b/packages/kit/test/apps/basics/src/routes/reroute/invalidate/+page.svelte new file mode 100644 index 000000000000..4f4589198f8d --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/reroute/invalidate/+page.svelte @@ -0,0 +1,11 @@ + + + + +{#if data.request} +

data request: {data.request}

+{/if} diff --git a/packages/kit/test/apps/basics/test/client.test.js b/packages/kit/test/apps/basics/test/client.test.js index 56b2c96bc744..1c935d53c1cf 100644 --- a/packages/kit/test/apps/basics/test/client.test.js +++ b/packages/kit/test/apps/basics/test/client.test.js @@ -1184,6 +1184,12 @@ test.describe('reroute', () => { expect(await page.textContent('h1')).toContain('Full Navigation'); }); + + test('reroute works with invalidate', async ({ page }) => { + await page.goto('/reroute/invalidate/a'); + await page.click('button'); + await expect(page.locator('p')).toHaveText('data request: true'); + }); }); test.describe('INP', () => { From 0f54ea72010877f0ca4372e50290555fb25a2e28 Mon Sep 17 00:00:00 2001 From: Chew Tee Ming Date: Mon, 2 Dec 2024 13:16:27 +0800 Subject: [PATCH 05/10] strip suffix in case reroute returns undefined --- packages/kit/src/runtime/server/respond.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index bf47ede55f31..74e909d51e1b 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -12,7 +12,7 @@ import { disable_search, has_data_suffix, normalize_path, - strip_data_suffix + strip_data_suffix, } from '../../utils/url.js'; import { exec } from '../../utils/routing.js'; import { redirect_json_response, render_data } from './data/index.js'; @@ -134,6 +134,9 @@ export async function respond(request, options, manifest, state) { /** @type {boolean[] | undefined} */ let invalidated_data_nodes; if (is_data_request) { + if (has_data_suffix(decoded)) { + decoded = strip_data_suffix(decoded) || '/'; + } url.pathname = strip_data_suffix(url.pathname) + (url.searchParams.get(TRAILING_SLASH_PARAM) === '1' ? '/' : '') || '/'; From c3317601d24bd71cd093fbe45a5f56652ca47b21 Mon Sep 17 00:00:00 2001 From: Chew Tee Ming Date: Mon, 2 Dec 2024 13:20:00 +0800 Subject: [PATCH 06/10] format --- packages/kit/src/runtime/server/respond.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index 74e909d51e1b..dd3ebf5dd18e 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -12,7 +12,7 @@ import { disable_search, has_data_suffix, normalize_path, - strip_data_suffix, + strip_data_suffix } from '../../utils/url.js'; import { exec } from '../../utils/routing.js'; import { redirect_json_response, render_data } from './data/index.js'; From 9750ea461f0f16e6747f475d3458004051538387 Mon Sep 17 00:00:00 2001 From: Chew Tee Ming Date: Mon, 2 Dec 2024 13:44:23 +0800 Subject: [PATCH 07/10] add comment --- packages/kit/src/runtime/server/respond.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index dd3ebf5dd18e..1d232412288a 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -134,6 +134,7 @@ export async function respond(request, options, manifest, state) { /** @type {boolean[] | undefined} */ let invalidated_data_nodes; if (is_data_request) { + // if reroute didn't run, the decoded URL still has the data suffix if (has_data_suffix(decoded)) { decoded = strip_data_suffix(decoded) || '/'; } From fe9e165b77caf4be1681555c7369c171363d692c Mon Sep 17 00:00:00 2001 From: Chew Tee Ming Date: Mon, 2 Dec 2024 13:51:41 +0800 Subject: [PATCH 08/10] more accurate comment --- packages/kit/src/runtime/server/respond.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index 1d232412288a..ea409f799736 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -134,7 +134,7 @@ export async function respond(request, options, manifest, state) { /** @type {boolean[] | undefined} */ let invalidated_data_nodes; if (is_data_request) { - // if reroute didn't run, the decoded URL still has the data suffix + // if reroute doesn't return anything, the decoded URL still has the data suffix if (has_data_suffix(decoded)) { decoded = strip_data_suffix(decoded) || '/'; } From 6d54a6117c4a393ecbc14488009c182fe8de48ab Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Fri, 17 Jan 2025 11:08:27 +0100 Subject: [PATCH 09/10] try this instead --- packages/kit/src/runtime/server/respond.js | 38 ++++++++-------------- 1 file changed, 13 insertions(+), 25 deletions(-) diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index ea409f799736..7d21643fbe5c 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -81,19 +81,25 @@ export async function respond(request, options, manifest, state) { } } - // reroute could alter the given URL, so we pass a copy - const url_copy = new URL(url); - const is_data_request = has_data_suffix(url.pathname); + /** @type {boolean[] | undefined} */ + let invalidated_data_nodes; if (is_data_request) { - url_copy.pathname = strip_data_suffix(url_copy.pathname) || '/'; - url_copy.searchParams.delete(TRAILING_SLASH_PARAM); - url_copy.searchParams.delete(INVALIDATED_PARAM); + url.pathname = + strip_data_suffix(url.pathname) + + (url.searchParams.get(TRAILING_SLASH_PARAM) === '1' ? '/' : '') || '/'; + url.searchParams.delete(TRAILING_SLASH_PARAM); + invalidated_data_nodes = url.searchParams + .get(INVALIDATED_PARAM) + ?.split('') + .map((node) => node === '1'); + url.searchParams.delete(INVALIDATED_PARAM); } + // reroute could alter the given URL, so we pass a copy let rerouted_path; try { - rerouted_path = options.hooks.reroute({ url: url_copy }) ?? url.pathname; + rerouted_path = options.hooks.reroute({ url: new URL(url) }) ?? url.pathname; } catch { return text('Internal Server Error', { status: 500 @@ -131,24 +137,6 @@ export async function respond(request, options, manifest, state) { return text('Not found', { status: 404, headers }); } - /** @type {boolean[] | undefined} */ - let invalidated_data_nodes; - if (is_data_request) { - // if reroute doesn't return anything, the decoded URL still has the data suffix - if (has_data_suffix(decoded)) { - decoded = strip_data_suffix(decoded) || '/'; - } - url.pathname = - strip_data_suffix(url.pathname) + - (url.searchParams.get(TRAILING_SLASH_PARAM) === '1' ? '/' : '') || '/'; - url.searchParams.delete(TRAILING_SLASH_PARAM); - invalidated_data_nodes = url.searchParams - .get(INVALIDATED_PARAM) - ?.split('') - .map((node) => node === '1'); - url.searchParams.delete(INVALIDATED_PARAM); - } - if (!state.prerendering?.fallback) { // TODO this could theoretically break — should probably be inside a try-catch const matchers = await manifest._.matchers(); From bcecf13bea9c6a4d225918895d11bf3f582b95ad Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Fri, 17 Jan 2025 11:27:59 +0100 Subject: [PATCH 10/10] Update packages/kit/src/runtime/server/respond.js --- packages/kit/src/runtime/server/respond.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index 8ba1e17b876b..ab51505a897c 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -98,7 +98,7 @@ export async function respond(request, options, manifest, state) { ?.split('') .map((node) => node === '1'); url.searchParams.delete(INVALIDATED_PARAM); - } + } // reroute could alter the given URL, so we pass a copy let rerouted_path;