[fix] revert #2354 and fix double character decoding a different way

.changeset/hot-taxis-jam.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+[fix] revert #2354 and fix double character decoding a different way

packages/kit/src/core/build/index.js

@@ -353,7 +353,21 @@ async function build_server(
 				};
 			}
 
-			const d = decodeURIComponent;
+			// input has already been decoded by decodeURI
+			// now handle the rest that decodeURIComponent would do
+			const d = s => s
+				.replace(/%23/g, '#')
+				.replace(/%3[Bb]/g, ';')
+				.replace(/%2[Cc]/g, ',')
+				.replace(/%2[Ff]/g, '/')
+				.replace(/%3[Ff]/g, '?')
+				.replace(/%3[Aa]/g, ':')
+				.replace(/%40/g, '@')
+				.replace(/%26/g, '&')
+				.replace(/%3[Dd]/g, '=')
+				.replace(/%2[Bb]/g, '+')
+				.replace(/%24/g, '$');
+
 			const empty = () => ({});
 
 			const manifest = {

packages/kit/src/core/create_manifest_data/index.js

@@ -4,13 +4,15 @@ import mime from 'mime';
 import { posixify } from '../utils.js';
 import glob from 'tiny-glob/sync.js';
 
-/** @typedef {{
+/**
+ * A portion of a file or directory name where the name has been split into
+ * static and dynamic parts
+ * @typedef {{
  *   content: string;
  *   dynamic: boolean;
  *   spread: boolean;
- * }} Part */
-
-/** @typedef {{
+ * }} Part
+ * @typedef {{
  *   basename: string;
  *   ext: string;
  *   parts: Part[],
@@ -19,7 +21,8 @@ import glob from 'tiny-glob/sync.js';
  *   is_index: boolean;
  *   is_page: boolean;
  *   route_suffix: string
- * }} Item */
+ * }} Item
+ */
 
 const specials = new Set(['__layout', '__layout.reset', '__error']);
 
@@ -389,10 +392,22 @@ function get_pattern(segments, add_trailing_slash) {
 							.map((part) => {
 								return part.dynamic
 									? '([^/]+?)'
-									: encodeURIComponent(part.content.normalize()).replace(
-											/[.*+?^${}()|[\]\\]/g,
-											'\\$&'
-									  );
+									: // allow users to specify characters on the file system in an encoded manner
+									  part.content
+											.normalize()
+											// We use [ and ] to denote parameters, so users must encode these on the file
+											// system to match against them. We don't decode all characters since others
+											// can already be epressed and so that '%' can be easily used directly in filenames
+											.replace(/%5[Bb]/g, '[')
+											.replace(/%5[Dd]/g, ']')
+											// '#', '/', and '?' can only appear in URL path segments in an encoded manner.
+											// They will not be touched by decodeURI so need to be encoded here, so
+											// that we can match against them.
+											// We skip '/' since you can't create a file with it on any OS
+											.replace(/#/g, '%23')
+											.replace(/\?/g, '%3F')
+											// escape characters that have special meaning in regex
+											.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
 							})
 							.join('');
 		})

packages/kit/src/core/create_manifest_data/index.spec.js

@@ -132,20 +132,18 @@ test('creates routes with layout', () => {
 	]);
 });
 
-test('encoding of characters', () => {
+test('encodes invalid characters', () => {
 	const { components, routes } = create('samples/encoding');
 
 	// had to remove ? and " because windows
 
 	// const quote = 'samples/encoding/".svelte';
 	const hash = 'samples/encoding/#.svelte';
-	const potato = 'samples/encoding/土豆.svelte';
 	// const question_mark = 'samples/encoding/?.svelte';
 
 	assert.equal(components, [
 		layout,
 		error,
-		potato,
 		// quote,
 		hash
 		// question_mark
@@ -154,7 +152,6 @@ test('encoding of characters', () => {
 	assert.equal(
 		routes.map((p) => p.pattern),
 		[
-			/^\/%E5%9C%9F%E8%B1%86\/?$/,
 			// /^\/%22\/?$/,
 			/^\/%23\/?$/
 			// /^\/%3F\/?$/

packages/kit/src/core/dev/index.js

@@ -231,15 +231,31 @@ function get_params(array) {
 	// src/routes/[x]/[y]/[z]/svelte, create a function
 	// that turns a RegExpExecArray into ({ x, y, z })
 
+	// input has already been decoded by decodeURI
+	// now handle the rest that decodeURIComponent would do
+	const d = /** @param {string} s */ (s) =>
+		s
+			.replace(/%23/g, '#')
+			.replace(/%3[Bb]/g, ';')
+			.replace(/%2[Cc]/g, ',')
+			.replace(/%2[Ff]/g, '/')
+			.replace(/%3[Ff]/g, '?')
+			.replace(/%3[Aa]/g, ':')
+			.replace(/%40/g, '@')
+			.replace(/%26/g, '&')
+			.replace(/%3[Dd]/g, '=')
+			.replace(/%2[Bb]/g, '+')
+			.replace(/%24/g, '$');
+
 	/** @param {RegExpExecArray} match */
 	const fn = (match) => {
 		/** @type {Record<string, string>} */
 		const params = {};
 		array.forEach((key, i) => {
 			if (key.startsWith('...')) {
-				params[key.slice(3)] = decodeURIComponent(match[i + 1] || '');
+				params[key.slice(3)] = d(match[i + 1] || '');
 			} else {
-				params[key] = decodeURIComponent(match[i + 1]);
+				params[key] = d(match[i + 1]);
 			}
 		});
 		return params;

packages/kit/src/runtime/client/renderer.js

@@ -541,8 +541,8 @@ export class Renderer {
 	 * @param {boolean} no_cache
 	 * @returns {Promise<import('./types').NavigationResult | undefined>} undefined if fallthrough
 	 */
-	async _load({ route, info: { path, query } }, no_cache) {
-		const key = `${path}?${query}`;
+	async _load({ route, info: { path, decoded_path, query } }, no_cache) {
+		const key = `${decoded_path}?${query}`;
 
 		if (!no_cache) {
 			const cached = this.cache.get(key);
@@ -552,7 +552,7 @@ export class Renderer {
 		const [pattern, a, b, get_params] = route;
 		const params = get_params
 			? // the pattern is for the route which we've already matched to this path
-			  get_params(/** @type {RegExpExecArray}  */ (pattern.exec(path)))
+			  get_params(/** @type {RegExpExecArray}  */ (pattern.exec(decoded_path)))
 			: {};
 
 		const changed = this.current.page && {

packages/kit/src/runtime/client/router.js

@@ -174,12 +174,13 @@ export class Router {
 		if (this.owns(url)) {
 			const path = url.pathname.slice(this.base.length) || '/';
 
-			const routes = this.routes.filter(([pattern]) => pattern.test(path));
+			const decoded_path = decodeURI(path);
+			const routes = this.routes.filter(([pattern]) => pattern.test(decoded_path));
 
 			const query = new URLSearchParams(url.search);
 			const id = `${path}?${query}`;
 
-			return { id, routes, path, query };
+			return { id, routes, path, decoded_path, query };
 		}
 	}
 

packages/kit/src/runtime/client/types.d.ts

@@ -5,6 +5,7 @@ export type NavigationInfo = {
 	id: string;
 	routes: CSRRoute[];
 	path: string;
+	decoded_path: string;
 	query: URLSearchParams;
 };
 

packages/kit/src/runtime/server/index.js

@@ -54,8 +54,9 @@ export async function respond(incoming, options, state = {}) {
 					});
 				}
 
+				const decoded = decodeURI(request.path);
 				for (const route of options.manifest.routes) {
-					const match = route.pattern.exec(request.path);
+					const match = route.pattern.exec(decoded);
 					if (!match) continue;
 
 					const response =

packages/kit/test/apps/basics/src/routes/encoded/_tests.js

@@ -19,6 +19,16 @@ export default function (test) {
 		assert.equal(await page.innerHTML('h3'), '/encoded/AC%2fDC: AC/DC');
 	});
 
+	test('visits a route with an encoded bracket', '/encoded/%5b', async ({ page }) => {
+		assert.equal(await page.innerHTML('h2'), '/encoded/%5b: [');
+		assert.equal(await page.innerHTML('h3'), '/encoded/%5b: [');
+	});
+
+	test('visits a route with an encoded question mark', '/encoded/%3f', async ({ page }) => {
+		assert.equal(await page.innerHTML('h2'), '/encoded/%3f: ?');
+		assert.equal(await page.innerHTML('h3'), '/encoded/%3f: ?');
+	});
+
 	test(
 		'visits a dynamic route with non-ASCII character',
 		'/encoded',