Skip to content

Commit

Permalink
[fix] don't decode URL when finding matching route
Browse files Browse the repository at this point in the history
  • Loading branch information
benmccann committed Sep 3, 2021
1 parent bb4d373 commit 181ddbd
Show file tree
Hide file tree
Showing 9 changed files with 24 additions and 17 deletions.
5 changes: 5 additions & 0 deletions .changeset/two-tigers-cry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

[fix] don't decode URL when finding matching route
11 changes: 4 additions & 7 deletions packages/kit/src/core/create_manifest_data/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -388,13 +388,10 @@ function get_pattern(segments, add_trailing_slash) {
.map((part) => {
return part.dynamic
? '([^/]+?)'
: part.content
.normalize()
.replace(/\?/g, '%3F')
.replace(/#/g, '%23')
.replace(/%5B/g, '[')
.replace(/%5D/g, ']')
.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
: encodeURIComponent(part.content.normalize()).replace(
/[.*+?^${}()|[\]\\]/g,
'\\$&'
);
})
.join('');
})
Expand Down
5 changes: 4 additions & 1 deletion packages/kit/src/core/create_manifest_data/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,18 +132,20 @@ test('creates routes with layout', () => {
]);
});

test('encodes invalid characters', () => {
test('encoding of 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
Expand All @@ -152,6 +154,7 @@ test('encodes invalid characters', () => {
assert.equal(
routes.map((p) => p.pattern),
[
/^\/%E5%9C%9F%E8%B1%86\/?$/,
// /^\/%22\/?$/,
/^\/%23\/?$/
// /^\/%3F\/?$/
Expand Down
Empty file.
6 changes: 3 additions & 3 deletions packages/kit/src/runtime/client/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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, decoded_path, query } }, no_cache) {
const key = `${decoded_path}?${query}`;
async _load({ route, info: { path, query } }, no_cache) {
const key = `${path}?${query}`;

if (!no_cache) {
const cached = this.cache.get(key);
Expand All @@ -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(decoded_path)))
get_params(/** @type {RegExpExecArray} */ (pattern.exec(path)))
: {};

const changed = this.current.page && {
Expand Down
5 changes: 2 additions & 3 deletions packages/kit/src/runtime/client/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,13 +170,12 @@ export class Router {
if (this.owns(url)) {
const path = url.pathname.slice(this.base.length) || '/';

const decoded_path = decodeURI(path);
const routes = this.routes.filter(([pattern]) => pattern.test(decoded_path));
const routes = this.routes.filter(([pattern]) => pattern.test(path));

const query = new URLSearchParams(url.search);
const id = `${path}?${query}`;

return { id, routes, path, decoded_path, query };
return { id, routes, path, query };
}
}

Expand Down
1 change: 0 additions & 1 deletion packages/kit/src/runtime/client/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ export type NavigationInfo = {
id: string;
routes: CSRRoute[];
path: string;
decoded_path: string;
query: URLSearchParams;
};

Expand Down
3 changes: 1 addition & 2 deletions packages/kit/src/runtime/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,8 @@ export async function respond(incoming, options, state = {}) {
});
}

const decoded = decodeURI(request.path);
for (const route of options.manifest.routes) {
const match = route.pattern.exec(decoded);
const match = route.pattern.exec(request.path);
if (!match) continue;

const response =
Expand Down
5 changes: 5 additions & 0 deletions packages/kit/test/apps/basics/src/routes/encoded/_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ export default function (test) {
assert.equal(decodeURI(await page.innerHTML('h3')), '/encoded/苗条');
});

test('visits a route with a doubly encoded space', '/encoded/test%2520me', async ({ page }) => {
assert.equal(await page.innerHTML('h2'), '/encoded/test%2520me: test%20me');
assert.equal(await page.innerHTML('h3'), '/encoded/test%2520me: test%20me');
});

test(
'visits a dynamic route with non-ASCII character',
'/encoded',
Expand Down

0 comments on commit 181ddbd

Please sign in to comment.