From e0d22347dbbbc740317c9d1eee5b206553467bbf Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Mon, 13 Jan 2025 15:33:18 +0000 Subject: [PATCH 1/9] feat(routing): external redirects --- packages/astro/src/core/errors/errors-data.ts | 14 ++++++++ packages/astro/src/core/redirects/render.ts | 32 +++++++++++++++---- .../astro/src/core/routing/manifest/create.ts | 16 ++++++---- packages/astro/test/redirects.test.js | 14 +++++--- 4 files changed, 60 insertions(+), 16 deletions(-) diff --git a/packages/astro/src/core/errors/errors-data.ts b/packages/astro/src/core/errors/errors-data.ts index 251063621967..d662a0f71845 100644 --- a/packages/astro/src/core/errors/errors-data.ts +++ b/packages/astro/src/core/errors/errors-data.ts @@ -995,6 +995,20 @@ export const RedirectWithNoLocation = { name: 'RedirectWithNoLocation', title: 'A redirect must be given a location with the `Location` header.', } satisfies ErrorData; + +/** + * @docs + * @see + * - [Astro.redirect](https://docs.astro.build/en/reference/api-reference/#redirect) + * @description + * An external redirect must start with http or https, and must be a valid URL + */ +export const UnsupportedExternalRedirect = { + name: 'UnsupportedExternalRedirect', + title: 'Unsupported or malformed URL.', + message: 'An external redirect must start with http or https, and must be a valid URL', +} satisfies ErrorData; + /** * @docs * @see diff --git a/packages/astro/src/core/redirects/render.ts b/packages/astro/src/core/redirects/render.ts index 379f26e3ba48..6245db9e0fd5 100644 --- a/packages/astro/src/core/redirects/render.ts +++ b/packages/astro/src/core/redirects/render.ts @@ -1,5 +1,14 @@ +import type { RedirectConfig } from '../../types/public/index.js'; import type { RenderContext } from '../render-context.js'; +export function redirectIsExternal(redirect: RedirectConfig): boolean { + if (typeof redirect === 'string') { + return redirect.startsWith('http') || redirect.startsWith('https'); + } else { + return redirect.destination.startsWith('http') || redirect.destination.startsWith('https'); + } +} + export async function renderRedirect(renderContext: RenderContext) { const { request: { method }, @@ -9,6 +18,13 @@ export async function renderRedirect(renderContext: RenderContext) { const status = redirectRoute && typeof redirect === 'object' ? redirect.status : method === 'GET' ? 301 : 308; const headers = { location: encodeURI(redirectRouteGenerate(renderContext)) }; + if (redirect && redirectIsExternal(redirect)) { + if (typeof redirect === 'string') { + return Response.redirect(redirect, status); + } else { + return Response.redirect(redirect.destination, status); + } + } return new Response(null, { status, headers }); } @@ -21,13 +37,17 @@ function redirectRouteGenerate(renderContext: RenderContext): string { if (typeof redirectRoute !== 'undefined') { return redirectRoute?.generate(params) || redirectRoute?.pathname || '/'; } else if (typeof redirect === 'string') { - // TODO: this logic is duplicated between here and manifest/create.ts - let target = redirect; - for (const param of Object.keys(params)) { - const paramValue = params[param]!; - target = target.replace(`[${param}]`, paramValue).replace(`[...${param}]`, paramValue); + if (redirectIsExternal(redirect)) { + return redirect; + } else { + // TODO: this logic is duplicated between here and manifest/create.ts + let target = redirect; + for (const param of Object.keys(params)) { + const paramValue = params[param]!; + target = target.replace(`[${param}]`, paramValue).replace(`[...${param}]`, paramValue); + } + return target; } - return target; } else if (typeof redirect === 'undefined') { return '/'; } diff --git a/packages/astro/src/core/routing/manifest/create.ts b/packages/astro/src/core/routing/manifest/create.ts index b0495a682ad8..9d213a2337ba 100644 --- a/packages/astro/src/core/routing/manifest/create.ts +++ b/packages/astro/src/core/routing/manifest/create.ts @@ -14,7 +14,10 @@ import { getPrerenderDefault } from '../../../prerender/utils.js'; import type { AstroConfig } from '../../../types/public/config.js'; import type { RouteData, RoutePart } from '../../../types/public/internal.js'; import { SUPPORTED_MARKDOWN_FILE_EXTENSIONS } from '../../constants.js'; -import { MissingIndexForInternationalization } from '../../errors/errors-data.js'; +import { + MissingIndexForInternationalization, + UnsupportedExternalRedirect, +} from '../../errors/errors-data.js'; import { AstroError } from '../../errors/index.js'; import { removeLeadingForwardSlash, slash } from '../../path.js'; import { injectServerIslandRoute } from '../../server-islands/endpoint.js'; @@ -348,11 +351,12 @@ function createRedirectRoutes( destination = to.destination; } - if (/^https?:\/\//.test(destination)) { - logger.warn( - 'redirects', - `Redirecting to an external URL is not officially supported: ${from} -> ${destination}`, - ); + // URLs that don't start with leading slash should be considered external + if (!destination.startsWith('/')) { + // check if the link starts with http or https; if not, log a warning + if (!/^https?:\/\//.test(destination) && !URL.canParse(destination)) { + throw new AstroError(UnsupportedExternalRedirect); + } } routes.push({ diff --git a/packages/astro/test/redirects.test.js b/packages/astro/test/redirects.test.js index 8014fb73bb45..3f3e25b3d7fb 100644 --- a/packages/astro/test/redirects.test.js +++ b/packages/astro/test/redirects.test.js @@ -36,13 +36,12 @@ describe('Astro.redirect', () => { assert.equal(response.headers.get('location'), '/login'); }); - // ref: https://github.com/withastro/astro/pull/9287#discussion_r1420739810 - it.skip('Ignores external redirect', async () => { + it('Allows external redirect', async () => { const app = await fixture.loadTestAdapterApp(); const request = new Request('http://example.com/external/redirect'); const response = await app.render(request); - assert.equal(response.status, 404); - assert.equal(response.headers.get('location'), null); + assert.equal(response.status, 301); + assert.equal(response.headers.get('location'), 'https://example.com/'); }); it('Warns when used inside a component', async () => { @@ -131,6 +130,7 @@ describe('Astro.redirect', () => { '/more/old/[dynamic]': '/more/[dynamic]', '/more/old/[dynamic]/[route]': '/more/[dynamic]/[route]', '/more/old/[...spread]': '/more/new/[...spread]', + '/external/redirect': 'https://example.com/', }, }); await fixture.build(); @@ -208,6 +208,12 @@ describe('Astro.redirect', () => { assert.equal(html.includes('http-equiv="refresh'), true); assert.equal(html.includes('url=/more/new/welcome/world'), true); }); + + it('supports redirecting to an external destination', async () => { + const html = await fixture.readFile('/external/redirect/index.html'); + assert.equal(html.includes('http-equiv="refresh'), true); + assert.equal(html.includes('url=https://example.com/'), true); + }); }); describe('dev', () => { From 2d5dce88714c75b1eb3295b897c71cc1b03fe09b Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Mon, 13 Jan 2025 15:59:27 +0000 Subject: [PATCH 2/9] linting --- packages/astro/src/core/routing/manifest/create.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/astro/src/core/routing/manifest/create.ts b/packages/astro/src/core/routing/manifest/create.ts index 9d213a2337ba..343f348b381c 100644 --- a/packages/astro/src/core/routing/manifest/create.ts +++ b/packages/astro/src/core/routing/manifest/create.ts @@ -317,7 +317,6 @@ function createInjectedRoutes({ settings, cwd }: CreateRouteManifestParams): Rou function createRedirectRoutes( { settings }: CreateRouteManifestParams, routeMap: Map, - logger: Logger, ): RouteData[] { const { config } = settings; const trailingSlash = config.trailingSlash; @@ -484,7 +483,7 @@ export async function createRouteManifest( routeMap.set(route.route, route); } - const redirectRoutes = createRedirectRoutes(params, routeMap, logger); + const redirectRoutes = createRedirectRoutes(params, routeMap); // we remove the file based routes that were deemed redirects const filteredFiledBasedRoutes = fileBasedRoutes.filter((fileBasedRoute) => { From e0ed1bdbf61187c5bd5be26dbdd13ca46d708ead Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 14 Jan 2025 10:36:05 +0000 Subject: [PATCH 3/9] add changeset --- .changeset/light-pants-smoke.md | 20 +++++++++++++ packages/astro/src/types/public/config.ts | 24 ++++++++++++---- packages/underscore-redirects/src/astro.ts | 33 +++++++++++----------- 3 files changed, 56 insertions(+), 21 deletions(-) create mode 100644 .changeset/light-pants-smoke.md diff --git a/.changeset/light-pants-smoke.md b/.changeset/light-pants-smoke.md new file mode 100644 index 000000000000..aad756ca500c --- /dev/null +++ b/.changeset/light-pants-smoke.md @@ -0,0 +1,20 @@ +--- +'astro': minor +--- + +Adds support for external redirects `astro.config.mjs`: + +```js +// astro.config.mjs +import {defineConfig} from "astro/config" + +export default defineConfig({ + redirects: { + "/blog": "https://example.com/blog", + "/news": { + status: 302, + destination: "https://example.com/news" + } + } +}) +``` diff --git a/packages/astro/src/types/public/config.ts b/packages/astro/src/types/public/config.ts index 635e57798c66..c86c74961916 100644 --- a/packages/astro/src/types/public/config.ts +++ b/packages/astro/src/types/public/config.ts @@ -264,16 +264,16 @@ export interface ViteUserConfig extends OriginalViteUserConfig { * and the value is the path to redirect to. * * You can redirect both static and dynamic routes, but only to the same kind of route. - * For example you cannot have a `'/article': '/blog/[...slug]'` redirect. + * For example, you cannot have a `'/article': '/blog/[...slug]'` redirect. * * * ```js - * { + * export default defineConfig({ * redirects: { * '/old': '/new', * '/blog/[...slug]': '/articles/[...slug]', * } - * } + * }) * ``` * * @@ -287,14 +287,28 @@ export interface ViteUserConfig extends OriginalViteUserConfig { * You can customize the [redirection status code](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status#redirection_messages) using an object in the redirect config: * * ```js - * { + * export default defineConfig({ * redirects: { * '/other': { * status: 302, * destination: '/place', * }, * } - * } + * }) + * ``` + * + * Since **v5.2.0**, the property accepts external URLs: + * + * ```js + * export default defineConfig({ + * redirects: { + * '/blog': 'https://example.com/blog', + * '/news': { + * status: 302, + * destination: 'https://example.com/news' + * } + * } + * }) * ``` */ redirects?: Record; diff --git a/packages/underscore-redirects/src/astro.ts b/packages/underscore-redirects/src/astro.ts index bd3e3b2ea346..83347fcdc6a3 100644 --- a/packages/underscore-redirects/src/astro.ts +++ b/packages/underscore-redirects/src/astro.ts @@ -36,23 +36,23 @@ export function createRedirectsFromAstroRoutes({ dir, buildOutput, assets, -}: CreateRedirectsFromAstroRoutesParams) { +}: CreateRedirectsFromAstroRoutesParams): Redirects { const base = config.base && config.base !== '/' ? config.base.endsWith('/') ? config.base.slice(0, -1) : config.base : ''; - const _redirects = new Redirects(); + const redirects = new Redirects(); for (const [route, dynamicTarget = ''] of routeToDynamicTargetMap) { const distURL = assets.get(route.pattern); // A route with a `pathname` is as static route. if (route.pathname) { if (route.redirect) { - // A redirect route without dynami§c parts. Get the redirect status + // A redirect route without dynamic parts. Get the redirect status // from the user if provided. - _redirects.add({ + redirects.add({ dynamic: false, input: `${base}${route.pathname}`, target: typeof route.redirect === 'object' ? route.redirect.destination : route.redirect, @@ -65,8 +65,10 @@ export function createRedirectsFromAstroRoutes({ // If this is a static build we don't want to add redirects to the HTML file. if (buildOutput === 'static') { continue; - } else if (distURL) { - _redirects.add({ + } + + if (distURL) { + redirects.add({ dynamic: false, input: `${base}${route.pathname}`, target: prependForwardSlash(distURL.toString().replace(dir.toString(), '')), @@ -74,7 +76,7 @@ export function createRedirectsFromAstroRoutes({ weight: 2, }); } else { - _redirects.add({ + redirects.add({ dynamic: false, input: `${base}${route.pathname}`, target: dynamicTarget, @@ -83,7 +85,7 @@ export function createRedirectsFromAstroRoutes({ }); if (route.pattern === '/404') { - _redirects.add({ + redirects.add({ dynamic: true, input: '/*', target: dynamicTarget, @@ -100,14 +102,13 @@ export function createRedirectsFromAstroRoutes({ // This route was prerendered and should be forwarded to the HTML file. if (distURL) { const targetRoute = route.redirectRoute ?? route; - const targetPattern = generateDynamicPattern(targetRoute); - let target = targetPattern; + let target = generateDynamicPattern(targetRoute); if (config.build.format === 'directory') { target = pathJoin(target, 'index.html'); } else { target += '.html'; } - _redirects.add({ + redirects.add({ dynamic: true, input: `${base}${pattern}`, target, @@ -115,7 +116,7 @@ export function createRedirectsFromAstroRoutes({ weight: 1, }); } else { - _redirects.add({ + redirects.add({ dynamic: true, input: `${base}${pattern}`, target: dynamicTarget, @@ -126,7 +127,7 @@ export function createRedirectsFromAstroRoutes({ } } - return _redirects; + return redirects; } /** @@ -135,7 +136,7 @@ export function createRedirectsFromAstroRoutes({ * With stars replacing spread and :id syntax replacing [id] */ function generateDynamicPattern(route: IntegrationResolvedRoute) { - const pattern = + return ( '/' + route.segments .map(([part]) => { @@ -150,8 +151,8 @@ function generateDynamicPattern(route: IntegrationResolvedRoute) { return part.content; } }) - .join('/'); - return pattern; + .join('/') + ); } function prependForwardSlash(str: string) { From 5a8a7413d07b3b1eb61b28a3fb469629be58e730 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 14 Jan 2025 11:15:12 +0000 Subject: [PATCH 4/9] better check --- packages/astro/src/core/redirects/render.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/astro/src/core/redirects/render.ts b/packages/astro/src/core/redirects/render.ts index 6245db9e0fd5..dc7ad44781f7 100644 --- a/packages/astro/src/core/redirects/render.ts +++ b/packages/astro/src/core/redirects/render.ts @@ -3,9 +3,11 @@ import type { RenderContext } from '../render-context.js'; export function redirectIsExternal(redirect: RedirectConfig): boolean { if (typeof redirect === 'string') { - return redirect.startsWith('http') || redirect.startsWith('https'); + return redirect.startsWith('http://') || redirect.startsWith('https://'); } else { - return redirect.destination.startsWith('http') || redirect.destination.startsWith('https'); + return ( + redirect.destination.startsWith('http://') || redirect.destination.startsWith('https://') + ); } } From b988cfa6fdccab17deab770d715e50bd86154e22 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 14 Jan 2025 13:22:02 +0000 Subject: [PATCH 5/9] Update .changeset/light-pants-smoke.md Co-authored-by: Florian Lefebvre --- .changeset/light-pants-smoke.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/light-pants-smoke.md b/.changeset/light-pants-smoke.md index aad756ca500c..0bf62b53d35f 100644 --- a/.changeset/light-pants-smoke.md +++ b/.changeset/light-pants-smoke.md @@ -2,7 +2,7 @@ 'astro': minor --- -Adds support for external redirects `astro.config.mjs`: +Adds support for configured external redirects: ```js // astro.config.mjs From 5f0deb7087fd615049c132a6ab3500b3249b5b59 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 14 Jan 2025 13:22:40 +0000 Subject: [PATCH 6/9] remove todo --- packages/astro/src/core/redirects/render.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/astro/src/core/redirects/render.ts b/packages/astro/src/core/redirects/render.ts index dc7ad44781f7..4044747beb8d 100644 --- a/packages/astro/src/core/redirects/render.ts +++ b/packages/astro/src/core/redirects/render.ts @@ -42,7 +42,6 @@ function redirectRouteGenerate(renderContext: RenderContext): string { if (redirectIsExternal(redirect)) { return redirect; } else { - // TODO: this logic is duplicated between here and manifest/create.ts let target = redirect; for (const param of Object.keys(params)) { const paramValue = params[param]!; From 6a03c2987500a5901b6ab59cc7db082aedec50bd Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Wed, 15 Jan 2025 13:39:07 +0000 Subject: [PATCH 7/9] address feedback --- packages/astro/src/types/public/config.ts | 26 +++++++---------------- 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/packages/astro/src/types/public/config.ts b/packages/astro/src/types/public/config.ts index c86c74961916..6d7923fbe11d 100644 --- a/packages/astro/src/types/public/config.ts +++ b/packages/astro/src/types/public/config.ts @@ -270,9 +270,14 @@ export interface ViteUserConfig extends OriginalViteUserConfig { * ```js * export default defineConfig({ * redirects: { - * '/old': '/new', - * '/blog/[...slug]': '/articles/[...slug]', - * } + * '/old': '/new', + * '/blog/[...slug]': '/articles/[...slug]', + * '/about': 'https://example.com/about', + * '/news': { + * status: 302, + * destination: 'https://example.com/news' + * } + * } * }) * ``` * @@ -296,21 +301,6 @@ export interface ViteUserConfig extends OriginalViteUserConfig { * } * }) * ``` - * - * Since **v5.2.0**, the property accepts external URLs: - * - * ```js - * export default defineConfig({ - * redirects: { - * '/blog': 'https://example.com/blog', - * '/news': { - * status: 302, - * destination: 'https://example.com/news' - * } - * } - * }) - * ``` - */ redirects?: Record; /** From 092cdd7c2ce321b5219c1f0faa3cff7023339478 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Wed, 15 Jan 2025 14:14:50 +0000 Subject: [PATCH 8/9] Update .changeset/light-pants-smoke.md Co-authored-by: Sarah Rainsberger <5098874+sarah11918@users.noreply.github.com> --- .changeset/light-pants-smoke.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.changeset/light-pants-smoke.md b/.changeset/light-pants-smoke.md index 0bf62b53d35f..3fec03f98346 100644 --- a/.changeset/light-pants-smoke.md +++ b/.changeset/light-pants-smoke.md @@ -2,7 +2,9 @@ 'astro': minor --- -Adds support for configured external redirects: +Adds support for redirecting to external sites with the [`redirects`](https://docs.astro.build/en/reference/configuration-reference/#redirects) configuration option. + +Now, you can redirect routes either internally to another path or externally by providing a URL beginning with `http` or `https`: ```js // astro.config.mjs From 7038546ff38dcc0df94af64dd0afd3c7f3e3ad54 Mon Sep 17 00:00:00 2001 From: Florian Lefebvre Date: Wed, 15 Jan 2025 16:05:45 +0100 Subject: [PATCH 9/9] Update packages/astro/src/types/public/config.ts Co-authored-by: Sarah Rainsberger <5098874+sarah11918@users.noreply.github.com> --- packages/astro/src/types/public/config.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/astro/src/types/public/config.ts b/packages/astro/src/types/public/config.ts index 6d7923fbe11d..b871b352817e 100644 --- a/packages/astro/src/types/public/config.ts +++ b/packages/astro/src/types/public/config.ts @@ -301,6 +301,7 @@ export interface ViteUserConfig extends OriginalViteUserConfig { * } * }) * ``` + */ redirects?: Record; /**