gh-3403: Remove encodeURI call for redirect load property (#3404)

.changeset/early-clocks-sneeze.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+The redirect property returned from a module's load function must now be a properly encoded URI string value.

documentation/docs/03-loading.md

@@ -143,6 +143,8 @@ If something goes wrong during `load`, return an `Error` object or a `string` de
 
 If the page should redirect (because the page is deprecated, or the user needs to be logged in, or whatever else) return a `string` containing the location to which they should be redirected alongside a `3xx` status code.
 
+The `redirect` string should be a [properly encoded](https://en.wikipedia.org/wiki/Percent-encoding) URI.  Both absolute and relative URIs are acceptable.
+
 #### maxage
 
 To cause pages to be cached, return a `number` describing the page's max age in seconds. The resulting cache header will include `private` if user data was involved in rendering the page (either via `session`, or because a credentialed `fetch` was made in a `load` function), but otherwise will include `public` so that it can be cached by CDNs.

packages/kit/src/runtime/server/page/respond.js

@@ -115,7 +115,7 @@ export async function respond(opts) {
 							new Response(undefined, {
 								status: loaded.loaded.status,
 								headers: {
-									location: encodeURI(loaded.loaded.redirect)
+									location: loaded.loaded.redirect
 								}
 							}),
 							set_cookie_headers

packages/kit/test/apps/basics/src/routes/encoded/index.svelte

@@ -1,3 +1,4 @@
 <a href="/encoded/苗条">苗条</a>
 <a href="/encoded/土豆">土豆</a>
 <a href="/encoded/反应">反应</a>
+<a href="/encoded/redirect">Redirect</a>

packages/kit/test/apps/basics/src/routes/encoded/redirect.svelte

@@ -0,0 +1,9 @@
+<script context="module">
+	/** @type {import('@sveltejs/kit').Load} */
+	export function load() {
+		return {
+			status: 307,
+			redirect: 'redirected?embedded=' + encodeURIComponent('/苗条?foo=bar&fizz=buzz')
+		};
+	}
+</script>

packages/kit/test/apps/basics/src/routes/encoded/redirected.svelte

@@ -0,0 +1,17 @@
+<script context="module">
+	/** @type {import('@sveltejs/kit').Load} */
+	export function load({ url }) {
+		return {
+			props: {
+				// nb: .get() on URLSearchParams does a decoding pass, so we should see the raw values.
+				embedded: url.searchParams.get('embedded')
+			}
+		};
+	}
+</script>
+<script>
+	/** @type {string} */
+	export let embedded;
+</script>
+
+<pre>{embedded}</pre>

packages/kit/test/apps/basics/src/routes/encoded/反应.svelte

@@ -2,7 +2,7 @@
 	export function load() {
 		return {
 			status: 307,
-			redirect: '苗条'
+			redirect: encodeURI('苗条')
 		};
 	}
 </script>

packages/kit/test/apps/basics/test/test.js

@@ -497,6 +497,22 @@ test.describe.parallel('Encoded paths', () => {
 		expect(decodeURI(await page.innerHTML('h3'))).toBe('/encoded/苗条');
 	});
 
+	test('redirects do not re-encode the redirect string', async ({ page, clicknav }) => {
+		await page.goto('/encoded');
+
+		await clicknav('[href="/encoded/redirect"]');
+
+		// check innerText instead of innerHTML because innerHTML would return the '&' character reference instead of '&' character.
+		expect(await page.innerText('pre')).toBe('/苗条?foo=bar&fizz=buzz');
+	});
+
+	test('redirects do not re-encode the redirect string during ssr', async ({ page }) => {
+		await page.goto('/encoded/redirect');
+
+		// check innerText instead of innerHTML because innerHTML would return the '&' character reference instead of '&' character.
+		expect(await page.innerText('pre')).toBe('/苗条?foo=bar&fizz=buzz');
+	});
+
 	test('sets charset on JSON Content-Type', async ({ request }) => {
 		const response = await request.get('/encoded/endpoint');
 		expect(response.headers()['content-type']).toBe('application/json; charset=utf-8');