From 83ed1cc1f20411ec876757f199cc0a1f182f2a80 Mon Sep 17 00:00:00 2001 From: Bjorn Lu Date: Tue, 27 Sep 2022 02:03:14 +0800 Subject: [PATCH] Prevent /undefined catch-all routes in dev (#4873) --- .changeset/mighty-garlics-exercise.md | 5 +++ packages/astro/src/core/render/route-cache.ts | 6 +-- packages/astro/src/core/routing/params.ts | 2 +- .../src/pages/empty-slug/[...slug].astro | 22 +++++++++++ packages/astro/test/routing-priority.test.js | 39 +++++++++++++++++-- packages/astro/test/test-utils.js | 1 + 6 files changed, 66 insertions(+), 9 deletions(-) create mode 100644 .changeset/mighty-garlics-exercise.md create mode 100644 packages/astro/test/fixtures/routing-priority/src/pages/empty-slug/[...slug].astro diff --git a/.changeset/mighty-garlics-exercise.md b/.changeset/mighty-garlics-exercise.md new file mode 100644 index 000000000..5c2df8d1b --- /dev/null +++ b/.changeset/mighty-garlics-exercise.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Prevent /undefined catch-all routes in dev diff --git a/packages/astro/src/core/render/route-cache.ts b/packages/astro/src/core/render/route-cache.ts index 3f1179b85..657d5fe69 100644 --- a/packages/astro/src/core/render/route-cache.ts +++ b/packages/astro/src/core/render/route-cache.ts @@ -108,13 +108,9 @@ export class RouteCache { export function findPathItemByKey(staticPaths: GetStaticPathsResultKeyed, params: Params) { const paramsKey = stringifyParams(params); - let matchedStaticPath = staticPaths.keyed.get(paramsKey); + const matchedStaticPath = staticPaths.keyed.get(paramsKey); if (matchedStaticPath) { return matchedStaticPath; } - debug('findPathItemByKey', `Unexpected cache miss looking for ${paramsKey}`); - matchedStaticPath = staticPaths.find( - ({ params: _params }) => JSON.stringify(_params) === paramsKey - ); } diff --git a/packages/astro/src/core/routing/params.ts b/packages/astro/src/core/routing/params.ts index 3ccbd1475..e68861c0a 100644 --- a/packages/astro/src/core/routing/params.ts +++ b/packages/astro/src/core/routing/params.ts @@ -32,7 +32,7 @@ export function stringifyParams(params: Params) { const validatedParams = Object.entries(params).reduce((acc, next) => { validateGetStaticPathsParameter(next); const [key, value] = next; - acc[key] = `${value}`; + acc[key] = typeof value === 'undefined' ? undefined : `${value}`; return acc; }, {} as Params); diff --git a/packages/astro/test/fixtures/routing-priority/src/pages/empty-slug/[...slug].astro b/packages/astro/test/fixtures/routing-priority/src/pages/empty-slug/[...slug].astro new file mode 100644 index 000000000..da548ce40 --- /dev/null +++ b/packages/astro/test/fixtures/routing-priority/src/pages/empty-slug/[...slug].astro @@ -0,0 +1,22 @@ +--- +export function getStaticPaths() { + return [{ + params: { slug: undefined }, + }, { + params: { slug: 'potato' }, + }] +} +const { slug } = Astro.params +--- + + + + + + {slug} + + +

empty-slug/[...slug].astro

+

slug: {slug}

+ + diff --git a/packages/astro/test/routing-priority.test.js b/packages/astro/test/routing-priority.test.js index 359b6981e..dba4094a2 100644 --- a/packages/astro/test/routing-priority.test.js +++ b/packages/astro/test/routing-priority.test.js @@ -95,6 +95,17 @@ const routes = [ h1: '[id].astro', p: 'injected-2', }, + { + description: 'matches /empty-slug to empty-slug/[...slug].astro', + url: '/empty-slug', + h1: 'empty-slug/[...slug].astro', + p: 'slug: ', + }, + { + description: 'do not match /empty-slug/undefined to empty-slug/[...slug].astro', + url: '/empty-slug/undefined', + fourOhFour: true, + }, ]; function appendForwardSlash(path) { @@ -112,9 +123,16 @@ describe('Routing priority', () => { await fixture.build(); }); - routes.forEach(({ description, url, h1, p }) => { + routes.forEach(({ description, url, fourOhFour, h1, p }) => { it(description, async () => { - const html = await fixture.readFile(`${appendForwardSlash(url)}index.html`); + const htmlFile = `${appendForwardSlash(url)}index.html`; + + if (fourOhFour) { + expect(fixture.pathExists(htmlFile)).to.be.false; + return; + } + + const html = await fixture.readFile(htmlFile); const $ = cheerioLoad(html); expect($('h1').text()).to.equal(h1); @@ -142,12 +160,17 @@ describe('Routing priority', () => { await devServer.stop(); }); - routes.forEach(({ description, url, h1, p }) => { + routes.forEach(({ description, url, fourOhFour, h1, p }) => { // checks URLs as written above it(description, async () => { const html = await fixture.fetch(url).then((res) => res.text()); const $ = cheerioLoad(html); + if (fourOhFour) { + expect($('title').text()).to.equal('404: Not Found'); + return; + } + expect($('h1').text()).to.equal(h1); if (p) { @@ -160,6 +183,11 @@ describe('Routing priority', () => { const html = await fixture.fetch(appendForwardSlash(url)).then((res) => res.text()); const $ = cheerioLoad(html); + if (fourOhFour) { + expect($('title').text()).to.equal('404: Not Found'); + return; + } + expect($('h1').text()).to.equal(h1); if (p) { @@ -174,6 +202,11 @@ describe('Routing priority', () => { .then((res) => res.text()); const $ = cheerioLoad(html); + if (fourOhFour) { + expect($('title').text()).to.equal('404: Not Found'); + return; + } + expect($('h1').text()).to.equal(h1); if (p) { diff --git a/packages/astro/test/test-utils.js b/packages/astro/test/test-utils.js index c70e6bc1d..c1fe6958c 100644 --- a/packages/astro/test/test-utils.js +++ b/packages/astro/test/test-utils.js @@ -156,6 +156,7 @@ export async function loadFixture(inlineConfig) { const previewServer = await preview(settings, { logging, telemetry, ...opts }); return previewServer; }, + pathExists: (p) => fs.existsSync(new URL(p.replace(/^\//, ''), config.outDir)), readFile: (filePath, encoding) => fs.promises.readFile(new URL(filePath.replace(/^\//, ''), config.outDir), encoding ?? 'utf8'), readdir: (fp) => fs.promises.readdir(new URL(fp.replace(/^\//, ''), config.outDir)),