From f4b1f7eabba27d70a1d9fa851c7fda2f552cb858 Mon Sep 17 00:00:00 2001 From: jervtub Date: Fri, 15 May 2020 12:43:44 +0200 Subject: [PATCH 1/3] Pull request to resolve issue #1442 https://github.com/sveltejs/sapper/issues/1142 Route with address "/client/" is not working. The first request to any route under "/client" - for instance "http://localhost/client/new" - would incorrectly get past the filter function at [`req.start.startsWith(prefix)`](https://github.com/sveltejs/sapper/blob/facbd96e0bd2539acf32c5ac14cd9478c7c635e5/runtime/src/server/middleware/index.ts#L104). Fix: add an extra test in the filter, whether there is a dot present in the requested path. Reasoning: I assume all the content served by sapper has an extension, such as "/client/chunk_slug.js". As well I assume there aren't requests to paths both startign with "/client" _and_ containing a dot, such as "http://localhost/client/new.123". That would still result in a 404. --- runtime/src/server/middleware/index.ts | 4 +++- test/apps/basics/src/routes/apple/new.svelte | 1 + test/apps/basics/src/routes/client/client/new.svelte | 1 + test/apps/basics/src/routes/client/index.svelte | 1 + test/apps/basics/src/routes/client/new.svelte | 1 + test/apps/basics/test.ts | 9 +++++++++ 6 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 test/apps/basics/src/routes/apple/new.svelte create mode 100644 test/apps/basics/src/routes/client/client/new.svelte create mode 100644 test/apps/basics/src/routes/client/index.svelte create mode 100644 test/apps/basics/src/routes/client/new.svelte diff --git a/runtime/src/server/middleware/index.ts b/runtime/src/server/middleware/index.ts index 96fd9ea05..121781f22 100644 --- a/runtime/src/server/middleware/index.ts +++ b/runtime/src/server/middleware/index.ts @@ -99,9 +99,11 @@ export function serve({ prefix, pathname, cache_control }: { pathname?: string, cache_control: string }) { + // [#1442](https://github.com/sveltejs/sapper/issues/1142) + // Requests to addresses under "/client" cannot be filtered out on their prefix, therefore additionally check for the presence of an extension. const filter = pathname ? (req: Req) => req.path === pathname - : (req: Req) => req.path.startsWith(prefix); + : (req: Req) => req.path.startsWith(prefix) && /\..*$/.test(req.path); const cache: Map = new Map(); diff --git a/test/apps/basics/src/routes/apple/new.svelte b/test/apps/basics/src/routes/apple/new.svelte new file mode 100644 index 000000000..a9d787cc5 --- /dev/null +++ b/test/apps/basics/src/routes/apple/new.svelte @@ -0,0 +1 @@ +Success. diff --git a/test/apps/basics/src/routes/client/client/new.svelte b/test/apps/basics/src/routes/client/client/new.svelte new file mode 100644 index 000000000..ef8e05020 --- /dev/null +++ b/test/apps/basics/src/routes/client/client/new.svelte @@ -0,0 +1 @@ +

Success.

diff --git a/test/apps/basics/src/routes/client/index.svelte b/test/apps/basics/src/routes/client/index.svelte new file mode 100644 index 000000000..ef8e05020 --- /dev/null +++ b/test/apps/basics/src/routes/client/index.svelte @@ -0,0 +1 @@ +

Success.

diff --git a/test/apps/basics/src/routes/client/new.svelte b/test/apps/basics/src/routes/client/new.svelte new file mode 100644 index 000000000..ef8e05020 --- /dev/null +++ b/test/apps/basics/src/routes/client/new.svelte @@ -0,0 +1 @@ +

Success.

diff --git a/test/apps/basics/test.ts b/test/apps/basics/test.ts index f1313b81a..9467bfb95 100644 --- a/test/apps/basics/test.ts +++ b/test/apps/basics/test.ts @@ -383,6 +383,15 @@ describe('basics', function() { assert.equal(await r.text('h2'), 'Called 1 time'); }); + it('finds routes under /client/', async () => { + await r.load('/client'); + assert.equal(await r.text('h1'), 'Success.'); + await r.load('/client/new'); + assert.equal(await r.text('h1'), 'Success.'); + await r.load('/client/client/new'); + assert.equal(await r.text('h1'), 'Success.'); + }); + it('survives the tests with no server errors', () => { assert.deepEqual(r.errors, []); }); From 23b6ccaae16dc57ba4aac8f8d335b97bb3359e0c Mon Sep 17 00:00:00 2001 From: jervtub Date: Sat, 16 May 2020 16:11:02 +0200 Subject: [PATCH 2/3] Move logic to error handling, as mentioned as the solution in the issue. --- runtime/src/server/middleware/index.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/runtime/src/server/middleware/index.ts b/runtime/src/server/middleware/index.ts index 121781f22..7c8e3fe76 100644 --- a/runtime/src/server/middleware/index.ts +++ b/runtime/src/server/middleware/index.ts @@ -99,11 +99,9 @@ export function serve({ prefix, pathname, cache_control }: { pathname?: string, cache_control: string }) { - // [#1442](https://github.com/sveltejs/sapper/issues/1142) - // Requests to addresses under "/client" cannot be filtered out on their prefix, therefore additionally check for the presence of an extension. const filter = pathname ? (req: Req) => req.path === pathname - : (req: Req) => req.path.startsWith(prefix) && /\..*$/.test(req.path); + : (req: Req) => req.path.startsWith(prefix); const cache: Map = new Map(); @@ -123,8 +121,15 @@ export function serve({ prefix, pathname, cache_control }: { res.setHeader('Cache-Control', cache_control); res.end(data); } catch (err) { - res.statusCode = 404; - res.end('not found'); + // [#1442](https://github.com/sveltejs/sapper/issues/1142) + // Requests to addresses under "/client" cannot be filtered out on their prefix, and can therefore end up here. + console.log(path.posix.normalize(decodeURIComponent(req.path))); + if (req.path.startsWith(prefix) && !/\..*$/.test(req.path)) { + next(); + } else { + res.statusCode = 404; + res.end('not found'); + } } } else { next(); From 8e4e1b533930afba2dae9a78860076e35d0f83e4 Mon Sep 17 00:00:00 2001 From: jervtub Date: Sat, 16 May 2020 16:12:22 +0200 Subject: [PATCH 3/3] Remove debug statement... --- runtime/src/server/middleware/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/runtime/src/server/middleware/index.ts b/runtime/src/server/middleware/index.ts index 7c8e3fe76..823b1aaa8 100644 --- a/runtime/src/server/middleware/index.ts +++ b/runtime/src/server/middleware/index.ts @@ -123,7 +123,6 @@ export function serve({ prefix, pathname, cache_control }: { } catch (err) { // [#1442](https://github.com/sveltejs/sapper/issues/1142) // Requests to addresses under "/client" cannot be filtered out on their prefix, and can therefore end up here. - console.log(path.posix.normalize(decodeURIComponent(req.path))); if (req.path.startsWith(prefix) && !/\..*$/.test(req.path)) { next(); } else {