From cc91c51b8878773ce4ad0e6924950c232c12e75b Mon Sep 17 00:00:00 2001 From: lilnasy <69170106+lilnasy@users.noreply.github.com> Date: Mon, 4 Dec 2023 15:46:44 +0000 Subject: [PATCH 1/7] fix(vercel): clear artifacts from redirects --- packages/integrations/vercel/src/lib/redirects.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/integrations/vercel/src/lib/redirects.ts b/packages/integrations/vercel/src/lib/redirects.ts index 8613be2ed72d..7aec1cf3b0b9 100644 --- a/packages/integrations/vercel/src/lib/redirects.ts +++ b/packages/integrations/vercel/src/lib/redirects.ts @@ -21,8 +21,7 @@ function getMatchPattern(segments: RoutePart[][]) { .map((segment) => { return segment[0].spread ? '(?:\\/(.*?))?' - : '\\/' + - segment + : segment .map((part) => { if (part) return part.dynamic From 1aa6de73f6cacd2be637fb8e7be30ac151586f5c Mon Sep 17 00:00:00 2001 From: lilnasy <69170106+lilnasy@users.noreply.github.com> Date: Mon, 4 Dec 2023 21:55:59 +0000 Subject: [PATCH 2/7] align tests with intended behavior --- packages/integrations/vercel/test/redirects.test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/integrations/vercel/test/redirects.test.js b/packages/integrations/vercel/test/redirects.test.js index df17664eec6e..9b6a9d50d828 100644 --- a/packages/integrations/vercel/test/redirects.test.js +++ b/packages/integrations/vercel/test/redirects.test.js @@ -32,15 +32,15 @@ describe('Redirects', () => { it('define static routes', async () => { const config = await getConfig(); - const oneRoute = config.routes.find((r) => r.src === '/\\/one'); + const oneRoute = config.routes.find((r) => r.src === '/one'); expect(oneRoute.headers.Location).to.equal('/'); expect(oneRoute.status).to.equal(301); - const twoRoute = config.routes.find((r) => r.src === '/\\/two'); + const twoRoute = config.routes.find((r) => r.src === '/two'); expect(twoRoute.headers.Location).to.equal('/'); expect(twoRoute.status).to.equal(301); - const threeRoute = config.routes.find((r) => r.src === '/\\/three'); + const threeRoute = config.routes.find((r) => r.src === '/three'); expect(threeRoute.headers.Location).to.equal('/'); expect(threeRoute.status).to.equal(302); }); @@ -48,7 +48,7 @@ describe('Redirects', () => { it('defines dynamic routes', async () => { const config = await getConfig(); - const blogRoute = config.routes.find((r) => r.src.startsWith('/\\/blog')); + const blogRoute = config.routes.find((r) => r.src.startsWith('/blog')); expect(blogRoute).to.not.be.undefined; expect(blogRoute.headers.Location.startsWith('/team/articles')).to.equal(true); expect(blogRoute.status).to.equal(301); @@ -57,7 +57,7 @@ describe('Redirects', () => { it('define trailingSlash redirect for sub pages', async () => { const config = await getConfig(); - const subpathRoute = config.routes.find((r) => r.src === '/\\/subpage'); + const subpathRoute = config.routes.find((r) => r.src === '/subpage'); expect(subpathRoute).to.not.be.undefined; expect(subpathRoute.headers.Location).to.equal('/subpage/'); }); From 934cfaaf3647d84146cfe930f7305b81b690009f Mon Sep 17 00:00:00 2001 From: lilnasy <69170106+lilnasy@users.noreply.github.com> Date: Mon, 4 Dec 2023 16:13:15 +0000 Subject: [PATCH 3/7] add changeset --- .changeset/nervous-chicken-allow.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/nervous-chicken-allow.md diff --git a/.changeset/nervous-chicken-allow.md b/.changeset/nervous-chicken-allow.md new file mode 100644 index 000000000000..214bd306ff4f --- /dev/null +++ b/.changeset/nervous-chicken-allow.md @@ -0,0 +1,5 @@ +--- +'@astrojs/vercel': patch +--- + +Fixes an issue where redirects did not work with the static adapter. From aac50366a515dfec6869739714209dd37a37d271 Mon Sep 17 00:00:00 2001 From: lilnasy <69170106+lilnasy@users.noreply.github.com> Date: Mon, 4 Dec 2023 22:18:09 +0000 Subject: [PATCH 4/7] log error and ignore for external redirects --- packages/astro/src/core/logger/core.ts | 1 + packages/astro/src/core/routing/manifest/create.ts | 13 +++++++++++++ packages/astro/test/redirects.test.js | 9 +++++++++ 3 files changed, 23 insertions(+) diff --git a/packages/astro/src/core/logger/core.ts b/packages/astro/src/core/logger/core.ts index b3f3b725673e..db571d8f14da 100644 --- a/packages/astro/src/core/logger/core.ts +++ b/packages/astro/src/core/logger/core.ts @@ -26,6 +26,7 @@ export type LoggerLabel = | 'watch' | 'middleware' | 'preferences' + | 'redirects' // SKIP_FORMAT: A special label that tells the logger not to apply any formatting. // Useful for messages that are already formatted, like the server start message. | 'SKIP_FORMAT'; diff --git a/packages/astro/src/core/routing/manifest/create.ts b/packages/astro/src/core/routing/manifest/create.ts index c9c0bb071685..7e5d53082311 100644 --- a/packages/astro/src/core/routing/manifest/create.ts +++ b/packages/astro/src/core/routing/manifest/create.ts @@ -451,6 +451,19 @@ export function createRouteManifest( .map(([{ dynamic, content }]) => (dynamic ? `[${content}]` : content)) .join('/')}`.toLowerCase(); + { + let destination: string + if (typeof to === "string") { + destination = to + } + else { + destination = to.destination + } + if (destination.startsWith("http")) { + return logger.error('redirects', `Redirecting to an external URLs is not supported: ${from} -> ${to}`); + } + } + const routeData: RouteData = { type: 'redirect', route, diff --git a/packages/astro/test/redirects.test.js b/packages/astro/test/redirects.test.js index bf9ad15f56f3..b9417643d059 100644 --- a/packages/astro/test/redirects.test.js +++ b/packages/astro/test/redirects.test.js @@ -14,6 +14,7 @@ describe('Astro.redirect', () => { adapter: testAdapter(), redirects: { '/api/redirect': '/test', + '/external/redirect': 'https://example.com/', }, }); await fixture.build(); @@ -27,6 +28,14 @@ describe('Astro.redirect', () => { expect(response.headers.get('location')).to.equal('/login'); }); + it('Ignores external redirect', async () => { + const app = await fixture.loadTestAdapterApp(); + const request = new Request('http://example.com/external/redirect'); + const response = await app.render(request); + expect(response.status).to.equal(404); + expect(response.headers.get('location')).to.equal(null); + }); + it('Warns when used inside a component', async () => { const app = await fixture.loadTestAdapterApp(); const request = new Request('http://example.com/late'); From fe8218192a6928720b0eece58807fa92615f829d Mon Sep 17 00:00:00 2001 From: Arsh <69170106+lilnasy@users.noreply.github.com> Date: Tue, 5 Dec 2023 03:54:21 +0530 Subject: [PATCH 5/7] Apply suggestions from code review --- packages/astro/src/core/routing/manifest/create.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/astro/src/core/routing/manifest/create.ts b/packages/astro/src/core/routing/manifest/create.ts index 7e5d53082311..720a2deaac42 100644 --- a/packages/astro/src/core/routing/manifest/create.ts +++ b/packages/astro/src/core/routing/manifest/create.ts @@ -460,7 +460,7 @@ export function createRouteManifest( destination = to.destination } if (destination.startsWith("http")) { - return logger.error('redirects', `Redirecting to an external URLs is not supported: ${from} -> ${to}`); + return logger.error('redirects', `Redirecting to an external URL is not supported: ${from} -> ${to}`); } } From acb1410d5ba13b40cdda6515ff2cb3db51b5c667 Mon Sep 17 00:00:00 2001 From: lilnasy <69170106+lilnasy@users.noreply.github.com> Date: Wed, 6 Dec 2023 13:20:18 +0000 Subject: [PATCH 6/7] proceed as usual after warning --- packages/astro/src/core/routing/manifest/create.ts | 2 +- packages/astro/test/redirects.test.js | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/astro/src/core/routing/manifest/create.ts b/packages/astro/src/core/routing/manifest/create.ts index 720a2deaac42..379f458eb50f 100644 --- a/packages/astro/src/core/routing/manifest/create.ts +++ b/packages/astro/src/core/routing/manifest/create.ts @@ -460,7 +460,7 @@ export function createRouteManifest( destination = to.destination } if (destination.startsWith("http")) { - return logger.error('redirects', `Redirecting to an external URL is not supported: ${from} -> ${to}`); + logger.warn('redirects', `Redirecting to an external URL is not officially supported: ${from} -> ${to}`); } } diff --git a/packages/astro/test/redirects.test.js b/packages/astro/test/redirects.test.js index b9417643d059..63a09312478f 100644 --- a/packages/astro/test/redirects.test.js +++ b/packages/astro/test/redirects.test.js @@ -28,7 +28,8 @@ describe('Astro.redirect', () => { expect(response.headers.get('location')).to.equal('/login'); }); - it('Ignores external redirect', async () => { + // ref: https://github.com/withastro/astro/pull/9287 + it.skip('Ignores external redirect', async () => { const app = await fixture.loadTestAdapterApp(); const request = new Request('http://example.com/external/redirect'); const response = await app.render(request); From 149121f87545da2fc7a3186a80c7db51b6613dad Mon Sep 17 00:00:00 2001 From: lilnasy <69170106+lilnasy@users.noreply.github.com> Date: Fri, 8 Dec 2023 16:08:34 +0000 Subject: [PATCH 7/7] more precise protocol check --- packages/astro/src/core/routing/manifest/create.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/astro/src/core/routing/manifest/create.ts b/packages/astro/src/core/routing/manifest/create.ts index 379f458eb50f..66f4a74fac39 100644 --- a/packages/astro/src/core/routing/manifest/create.ts +++ b/packages/astro/src/core/routing/manifest/create.ts @@ -459,7 +459,7 @@ export function createRouteManifest( else { destination = to.destination } - if (destination.startsWith("http")) { + if (/^https?:\/\//.test(destination)) { logger.warn('redirects', `Redirecting to an external URL is not officially supported: ${from} -> ${to}`); } }