diff --git a/.changeset/hot-taxis-jam.md b/.changeset/hot-taxis-jam.md new file mode 100644 index 000000000000..c7d3c329670e --- /dev/null +++ b/.changeset/hot-taxis-jam.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +[fix] revert #2354 and fix double character decoding a different way diff --git a/packages/kit/src/core/build/index.js b/packages/kit/src/core/build/index.js index 7ea97b239612..0921e1a809a0 100644 --- a/packages/kit/src/core/build/index.js +++ b/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 = { diff --git a/packages/kit/src/core/create_manifest_data/index.js b/packages/kit/src/core/create_manifest_data/index.js index aa93bf7a457b..ca1e41ce761a 100644 --- a/packages/kit/src/core/create_manifest_data/index.js +++ b/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(''); }) diff --git a/packages/kit/src/core/create_manifest_data/index.spec.js b/packages/kit/src/core/create_manifest_data/index.spec.js index 203a6421de59..2f0a80d0fd9d 100644 --- a/packages/kit/src/core/create_manifest_data/index.spec.js +++ b/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\/?$/ diff --git "a/packages/kit/src/core/create_manifest_data/test/samples/encoding/\345\234\237\350\261\206.svelte" "b/packages/kit/src/core/create_manifest_data/test/samples/encoding/\345\234\237\350\261\206.svelte" deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/packages/kit/src/core/dev/index.js b/packages/kit/src/core/dev/index.js index e99f4391f150..42477b0bcccf 100644 --- a/packages/kit/src/core/dev/index.js +++ b/packages/kit/src/core/dev/index.js @@ -221,15 +221,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} */ 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; diff --git a/packages/kit/src/runtime/client/renderer.js b/packages/kit/src/runtime/client/renderer.js index dd5e8e50ee68..81d0eff30bbf 100644 --- a/packages/kit/src/runtime/client/renderer.js +++ b/packages/kit/src/runtime/client/renderer.js @@ -541,8 +541,8 @@ export class Renderer { * @param {boolean} no_cache * @returns {Promise} 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 && { diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 061ad85a9d16..4f1e6ce5fedd 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/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 }; } } diff --git a/packages/kit/src/runtime/client/types.d.ts b/packages/kit/src/runtime/client/types.d.ts index 82a91d903424..e5865fa3591b 100644 --- a/packages/kit/src/runtime/client/types.d.ts +++ b/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; }; diff --git a/packages/kit/src/runtime/server/index.js b/packages/kit/src/runtime/server/index.js index 1f56067ffc84..fedc115c2820 100644 --- a/packages/kit/src/runtime/server/index.js +++ b/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 = diff --git a/packages/kit/test/apps/basics/src/routes/encoded/_tests.js b/packages/kit/test/apps/basics/src/routes/encoded/_tests.js index be9911e55df0..543df5078208 100644 --- a/packages/kit/test/apps/basics/src/routes/encoded/_tests.js +++ b/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',