Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] don't decode URL when finding matching route #2354

Merged
merged 1 commit into from
Sep 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
10 changes: 10 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,16 @@ 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 route with an encoded slash', '/encoded/AC%2fDC', async ({ page }) => {
assert.equal(await page.innerHTML('h2'), '/encoded/AC%2fDC: AC/DC');
assert.equal(await page.innerHTML('h3'), '/encoded/AC%2fDC: AC/DC');
});

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