From 951ad8afdf76d8bebc886affbe78c41341a2e4bf Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Wed, 15 Nov 2023 13:40:33 -0500 Subject: [PATCH 1/7] feat(i18n): domain with lookup table --- packages/astro/src/@types/astro.ts | 10 ++++-- packages/astro/src/core/app/index.ts | 25 +++++++++++++-- packages/astro/src/core/app/types.ts | 3 +- packages/astro/src/core/build/generate.ts | 1 + .../src/core/build/plugins/plugin-manifest.ts | 31 ++++++++++++++++++- packages/astro/src/core/config/schema.ts | 15 +++++++-- .../src/vite-plugin-astro-server/plugin.ts | 1 + .../src/vite-plugin-astro-server/route.ts | 6 +--- .../i18n-routing-subdomain/astro.config.mjs | 19 ++++++++++++ .../i18n-routing-subdomain/package.json | 8 +++++ .../src/pages/en/blog/[id].astro | 18 +++++++++++ .../src/pages/en/index.astro | 8 +++++ .../src/pages/en/start.astro | 8 +++++ .../src/pages/pt/blog/[id].astro | 18 +++++++++++ .../src/pages/pt/start.astro | 8 +++++ packages/astro/test/i18-routing.test.js | 26 ++++++++++++++++ .../test/units/config/config-validate.test.js | 23 ++++++++++++++ pnpm-lock.yaml | 6 ++++ 18 files changed, 219 insertions(+), 15 deletions(-) create mode 100644 packages/astro/test/fixtures/i18n-routing-subdomain/astro.config.mjs create mode 100644 packages/astro/test/fixtures/i18n-routing-subdomain/package.json create mode 100644 packages/astro/test/fixtures/i18n-routing-subdomain/src/pages/en/blog/[id].astro create mode 100644 packages/astro/test/fixtures/i18n-routing-subdomain/src/pages/en/index.astro create mode 100644 packages/astro/test/fixtures/i18n-routing-subdomain/src/pages/en/start.astro create mode 100644 packages/astro/test/fixtures/i18n-routing-subdomain/src/pages/pt/blog/[id].astro create mode 100644 packages/astro/test/fixtures/i18n-routing-subdomain/src/pages/pt/start.astro diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts index 378999ec293a..f3a2bc8a25ef 100644 --- a/packages/astro/src/@types/astro.ts +++ b/packages/astro/src/@types/astro.ts @@ -1520,7 +1520,7 @@ export interface AstroUserConfig { * @docs * @kind h4 * @name experimental.i18n.routingStrategy - * @type {'prefix-always' | 'prefix-other-locales'} + * @type {'prefix-always' | 'prefix-other-locales' | 'domain'} * @default 'prefix-other-locales' * @version 3.5.0 * @description @@ -1533,9 +1533,12 @@ export interface AstroUserConfig { * - `prefix-always`: All URLs will display a language prefix. * URLs will be of the form `example.com/[locale]/content/` for every route, including the default language. * Localized folders are used for every language, including the default. + * - `domain`: SSR only, it enables support for different domains. When a locale is mapped to domain, all the URLs won't have the language prefix. + * You map `fr` to `fr.example.com`, if you want a to have a blog page to look like `fr.example.com/blog` instead of `example.com/fr/blog`. + * The localised folders be must in the `src/pages/` folder. * */ - routingStrategy?: 'prefix-always' | 'prefix-other-locales'; + routingStrategy?: 'prefix-always' | 'prefix-other-locales' | 'domain'; /** * @docs @@ -1556,7 +1559,8 @@ export interface AstroUserConfig { * locales: ["en", "fr", "pt-br", "es"], * domains: { * fr: "https://fr.example.com", - * } + * }, + * routingStrategy: "domain" * } * } * }) diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index 2d5cd16ed6bb..23f217a3f59f 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -131,8 +131,29 @@ export class App { const url = new URL(request.url); // ignore requests matching public assets if (this.#manifest.assets.has(url.pathname)) return undefined; - const pathname = prependForwardSlash(this.removeBase(url.pathname)); - const routeData = matchRoute(pathname, this.#manifestData); + let pathname; + if (this.#manifest.i18n && this.#manifest.i18n.routingStrategy === 'domain') { + const host = request.headers.get('X-Forwarded-Host'); + if (host) { + try { + let hostUrl = new URL(this.removeBase(url.pathname), host); + const maybePathname = this.#manifest.i18n.domainLookupTable[hostUrl.toString()]; + if (maybePathname) { + pathname = maybePathname; + } + } catch (e) { + // waiting to decide what to do here + // eslint-disable-next-line no-console + console.error(e); + // TODO: What kind of error should we try? This happens if we have an invalid value inside the X-Forwarded-Host header + } + } + } + if (!pathname) { + pathname = prependForwardSlash(this.removeBase(url.pathname)); + } + let routeData = matchRoute(pathname, this.#manifestData); + // missing routes fall-through, prerendered are handled by static layer if (!routeData || routeData.prerender) return undefined; return routeData; diff --git a/packages/astro/src/core/app/types.ts b/packages/astro/src/core/app/types.ts index 9f9d80f44511..70271aafc04f 100644 --- a/packages/astro/src/core/app/types.ts +++ b/packages/astro/src/core/app/types.ts @@ -55,9 +55,10 @@ export type SSRManifest = { export type SSRManifestI18n = { fallback?: Record; - routingStrategy?: 'prefix-always' | 'prefix-other-locales'; + routingStrategy?: 'prefix-always' | 'prefix-other-locales' | 'domain'; locales: string[]; defaultLocale: string; + domainLookupTable: Record; }; export type SerializedSSRManifest = Omit< diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index 6d8f51df2acd..f9bf4917ce18 100644 --- a/packages/astro/src/core/build/generate.ts +++ b/packages/astro/src/core/build/generate.ts @@ -634,6 +634,7 @@ export function createBuildManifest( routingStrategy: settings.config.experimental.i18n.routingStrategy, defaultLocale: settings.config.experimental.i18n.defaultLocale, locales: settings.config.experimental.i18n.locales, + domainLookupTable: {}, }; } return { diff --git a/packages/astro/src/core/build/plugins/plugin-manifest.ts b/packages/astro/src/core/build/plugins/plugin-manifest.ts index 83065ecacbe4..33785d62cd67 100644 --- a/packages/astro/src/core/build/plugins/plugin-manifest.ts +++ b/packages/astro/src/core/build/plugins/plugin-manifest.ts @@ -9,13 +9,14 @@ import type { SerializedRouteInfo, SerializedSSRManifest, } from '../../app/types.js'; -import { joinPaths, prependForwardSlash } from '../../path.js'; +import { appendForwardSlash, joinPaths, prependForwardSlash } from '../../path.js'; import { serializeRouteData } from '../../routing/index.js'; import { addRollupInput } from '../add-rollup-input.js'; import { getOutFile, getOutFolder } from '../common.js'; import { cssOrder, mergeInlineCss, type BuildInternals } from '../internal.js'; import type { AstroBuildPlugin } from '../plugin.js'; import type { StaticBuildOptions } from '../types.js'; +import { shouldAppendForwardSlash } from '../util.js'; const manifestReplace = '@@ASTRO_MANIFEST_REPLACE@@'; const replaceExp = new RegExp(`['"](${manifestReplace})['"]`, 'g'); @@ -161,6 +162,7 @@ function buildManifest( const { settings } = opts; const routes: SerializedRouteInfo[] = []; + const domainLookupTable: Record = {}; const entryModules = Object.fromEntries(internals.entrySpecifierToBundleMap.entries()); if (settings.scripts.some((script) => script.stage === 'page')) { staticFiles.push(entryModules[PAGE_SCRIPT_ID]); @@ -234,6 +236,32 @@ function buildManifest( styles, routeData: serializeRouteData(route, settings.config.trailingSlash), }); + + /** + * logic meant for i18n domain support, where we fill the lookup table + */ + const i18n = settings.config.experimental.i18n; + if (i18n && i18n.domains && i18n.routingStrategy === 'domain') { + // Domain routing strategy assumes that the locale starts at the beginning of the path, so it's safe to get the first item + // This is something like /es/guides/getting-started + const segments = route.route.split('/'); + const pathnameLocale = segments[1]; + const maybeLocaleDomain = i18n.domains[pathnameLocale]; + if (maybeLocaleDomain) { + // This is something like guides/getting-started + let pathnameWithoutLocale = segments + .filter((segment) => segment !== pathnameLocale) + .slice(1) + .join('/'); + if (shouldAppendForwardSlash(settings.config.trailingSlash, settings.config.build.format)) { + pathnameWithoutLocale = appendForwardSlash(pathnameWithoutLocale); + } + // We build a URL object like this: https://es.example.com/guides/getting-started + const domainPath = new URL(pathnameWithoutLocale, maybeLocaleDomain); + // We map https://es.example.com/guides/getting-started -> /es/guides/getting-started + domainLookupTable[domainPath.toString()] = route.route; + } + } } // HACK! Patch this special one. @@ -248,6 +276,7 @@ function buildManifest( routingStrategy: settings.config.experimental.i18n.routingStrategy, locales: settings.config.experimental.i18n.locales, defaultLocale: settings.config.experimental.i18n.defaultLocale, + domainLookupTable, }; } diff --git a/packages/astro/src/core/config/schema.ts b/packages/astro/src/core/config/schema.ts index 70852c598e39..08b9b0203bd0 100644 --- a/packages/astro/src/core/config/schema.ts +++ b/packages/astro/src/core/config/schema.ts @@ -356,16 +356,15 @@ export const AstroConfigSchema = z.object({ ) ) .optional(), - // TODO: properly add default when the feature goes of experimental routingStrategy: z - .enum(['prefix-always', 'prefix-other-locales']) + .enum(['prefix-always', 'prefix-other-locales', 'domain']) .optional() .default('prefix-other-locales'), }) .optional() .superRefine((i18n, ctx) => { if (i18n) { - const { defaultLocale, locales, fallback, domains } = i18n; + const { defaultLocale, locales, fallback, domains, routingStrategy } = i18n; if (!locales.includes(defaultLocale)) { ctx.addIssue({ code: z.ZodIssueCode.custom, @@ -397,6 +396,16 @@ export const AstroConfigSchema = z.object({ } } if (domains) { + const entries = Object.entries(domains); + if (entries.length > 0) { + if (routingStrategy !== 'domain') { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: `When specifying some domains, the property \`i18n.routingStrategy\` must be set to \`"domain"\`.`, + }); + } + } + for (const [domainKey, domainValue] of Object.entries(domains)) { if (!locales.includes(domainKey)) { ctx.addIssue({ diff --git a/packages/astro/src/vite-plugin-astro-server/plugin.ts b/packages/astro/src/vite-plugin-astro-server/plugin.ts index b8f4ab661aba..354baf555277 100644 --- a/packages/astro/src/vite-plugin-astro-server/plugin.ts +++ b/packages/astro/src/vite-plugin-astro-server/plugin.ts @@ -93,6 +93,7 @@ export function createDevelopmentManifest(settings: AstroSettings): SSRManifest routingStrategy: settings.config.experimental.i18n.routingStrategy, defaultLocale: settings.config.experimental.i18n.defaultLocale, locales: settings.config.experimental.i18n.locales, + domainLookupTable: {}, }; } return { diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts index 580ceb0b6468..d1978fade014 100644 --- a/packages/astro/src/vite-plugin-astro-server/route.ts +++ b/packages/astro/src/vite-plugin-astro-server/route.ts @@ -277,11 +277,7 @@ export async function handleRoute({ const onRequest = middleware?.onRequest as MiddlewareEndpointHandler | undefined; if (config.experimental.i18n) { - const i18Middleware = createI18nMiddleware( - config.experimental.i18n, - config.base, - config.trailingSlash - ); + const i18Middleware = createI18nMiddleware(manifest.i18n, config.base, config.trailingSlash); if (i18Middleware) { if (onRequest) { diff --git a/packages/astro/test/fixtures/i18n-routing-subdomain/astro.config.mjs b/packages/astro/test/fixtures/i18n-routing-subdomain/astro.config.mjs new file mode 100644 index 000000000000..254c44ca8e96 --- /dev/null +++ b/packages/astro/test/fixtures/i18n-routing-subdomain/astro.config.mjs @@ -0,0 +1,19 @@ +import { defineConfig} from "astro/config"; + +export default defineConfig({ + trailingSlash: "never", + experimental: { + i18n: { + defaultLocale: 'en', + locales: [ + 'en', 'pt', 'it' + ], + domains: { + pt: "https://example.pt" + }, + routingStrategy: "domain" + }, + + }, + base: "/new-site" +}) diff --git a/packages/astro/test/fixtures/i18n-routing-subdomain/package.json b/packages/astro/test/fixtures/i18n-routing-subdomain/package.json new file mode 100644 index 000000000000..931425fa6206 --- /dev/null +++ b/packages/astro/test/fixtures/i18n-routing-subdomain/package.json @@ -0,0 +1,8 @@ +{ + "name": "@test/i18n-routing-subdomain", + "version": "0.0.0", + "private": true, + "dependencies": { + "astro": "workspace:*" + } +} diff --git a/packages/astro/test/fixtures/i18n-routing-subdomain/src/pages/en/blog/[id].astro b/packages/astro/test/fixtures/i18n-routing-subdomain/src/pages/en/blog/[id].astro new file mode 100644 index 000000000000..97b41230d6e9 --- /dev/null +++ b/packages/astro/test/fixtures/i18n-routing-subdomain/src/pages/en/blog/[id].astro @@ -0,0 +1,18 @@ +--- +export function getStaticPaths() { + return [ + {params: {id: '1'}, props: { content: "Hello world" }}, + {params: {id: '2'}, props: { content: "Eat Something" }}, + {params: {id: '3'}, props: { content: "How are you?" }}, + ]; +} +const { content } = Astro.props; +--- + + + Astro + + +{content} + + diff --git a/packages/astro/test/fixtures/i18n-routing-subdomain/src/pages/en/index.astro b/packages/astro/test/fixtures/i18n-routing-subdomain/src/pages/en/index.astro new file mode 100644 index 000000000000..3e50ac6bf3cb --- /dev/null +++ b/packages/astro/test/fixtures/i18n-routing-subdomain/src/pages/en/index.astro @@ -0,0 +1,8 @@ + + + Astro + + +Hello + + diff --git a/packages/astro/test/fixtures/i18n-routing-subdomain/src/pages/en/start.astro b/packages/astro/test/fixtures/i18n-routing-subdomain/src/pages/en/start.astro new file mode 100644 index 000000000000..990baecd9a8c --- /dev/null +++ b/packages/astro/test/fixtures/i18n-routing-subdomain/src/pages/en/start.astro @@ -0,0 +1,8 @@ + + + Astro + + +Start + + diff --git a/packages/astro/test/fixtures/i18n-routing-subdomain/src/pages/pt/blog/[id].astro b/packages/astro/test/fixtures/i18n-routing-subdomain/src/pages/pt/blog/[id].astro new file mode 100644 index 000000000000..e37f83a30243 --- /dev/null +++ b/packages/astro/test/fixtures/i18n-routing-subdomain/src/pages/pt/blog/[id].astro @@ -0,0 +1,18 @@ +--- +export function getStaticPaths() { + return [ + {params: {id: '1'}, props: { content: "Hola mundo" }}, + {params: {id: '2'}, props: { content: "Eat Something" }}, + {params: {id: '3'}, props: { content: "How are you?" }}, + ]; +} +const { content } = Astro.props; +--- + + + Astro + + +{content} + + diff --git a/packages/astro/test/fixtures/i18n-routing-subdomain/src/pages/pt/start.astro b/packages/astro/test/fixtures/i18n-routing-subdomain/src/pages/pt/start.astro new file mode 100644 index 000000000000..5a4a84c2cf0c --- /dev/null +++ b/packages/astro/test/fixtures/i18n-routing-subdomain/src/pages/pt/start.astro @@ -0,0 +1,8 @@ + + + Astro + + +Oi essa e start + + diff --git a/packages/astro/test/i18-routing.test.js b/packages/astro/test/i18-routing.test.js index 311040c77f5f..153fe297169d 100644 --- a/packages/astro/test/i18-routing.test.js +++ b/packages/astro/test/i18-routing.test.js @@ -1009,4 +1009,30 @@ describe('[SSR] i18n routing', () => { }); }); }); + + describe('i18n routing with routing strategy [subdomain]', () => { + /** @type {import('./test-utils').Fixture} */ + let fixture; + + before(async () => { + fixture = await loadFixture({ + root: './fixtures/i18n-routing-subdomain/', + output: 'server', + adapter: testAdapter(), + }); + await fixture.build(); + app = await fixture.loadTestAdapterApp(); + }); + + it('should render the en locale', async () => { + let request = new Request('http://example.pt/new-site/start', { + headers: { + 'X-Forwarded-Host': 'https://example.pt', + }, + }); + let response = await app.render(request); + expect(response.status).to.equal(200); + expect(await response.text()).includes('Oi essa e start\n'); + }); + }); }); diff --git a/packages/astro/test/units/config/config-validate.test.js b/packages/astro/test/units/config/config-validate.test.js index d8a9e7b7d84d..d20280a3563e 100644 --- a/packages/astro/test/units/config/config-validate.test.js +++ b/packages/astro/test/units/config/config-validate.test.js @@ -191,6 +191,7 @@ describe('Config Validation', () => { domains: { en: 'www.example.com', }, + routingStrategy: 'domain', }, }, }, @@ -212,6 +213,7 @@ describe('Config Validation', () => { domains: { en: 'https://www.example.com/blog/page/', }, + routingStrategy: 'domain', }, }, }, @@ -222,5 +224,26 @@ describe('Config Validation', () => { "The URL `https://www.example.com/blog/page/` must contain only the origin. A subsequent pathname isn't allowed here. Remove `/blog/page/`." ); }); + + it('errors if there are domains, and the routing strategy is not correct', async () => { + const configError = await validateConfig( + { + experimental: { + i18n: { + defaultLocale: 'en', + locales: ['es', 'en'], + domains: { + en: 'https://www.example.com/', + }, + }, + }, + }, + process.cwd() + ).catch((err) => err); + expect(configError instanceof z.ZodError).to.equal(true); + expect(configError.errors[0].message).to.equal( + 'When specifying some domains, the property `i18n.routingStrategy` must be set to `"domain"`.' + ); + }); }); }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0ca2231f2cf5..24b69b4afa82 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2833,6 +2833,12 @@ importers: specifier: workspace:* version: link:../../.. + packages/astro/test/fixtures/i18n-routing-subdomain: + dependencies: + astro: + specifier: workspace:* + version: link:../../.. + packages/astro/test/fixtures/import-ts-with-js: dependencies: astro: From 4d1f42c782466f256f02f04c62f52e1148c966b0 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 16 Nov 2023 09:05:41 -0600 Subject: [PATCH 2/7] make logic simpler --- packages/astro/src/core/app/index.ts | 13 +++++--- .../src/core/build/plugins/plugin-manifest.ts | 33 +++++-------------- 2 files changed, 18 insertions(+), 28 deletions(-) diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index 23f217a3f59f..97cdc97fe81c 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -13,7 +13,9 @@ import { consoleLogDestination } from '../logger/console.js'; import { AstroIntegrationLogger, Logger } from '../logger/core.js'; import { sequence } from '../middleware/index.js'; import { + appendForwardSlash, collapseDuplicateSlashes, + joinPaths, prependForwardSlash, removeTrailingForwardSlash, } from '../path.js'; @@ -28,6 +30,7 @@ import { import { matchRoute } from '../routing/match.js'; import { EndpointNotFoundError, SSRRoutePipeline } from './ssrPipeline.js'; import type { RouteInfo } from './types.js'; +import { shouldAppendForwardSlash } from '../build/util.js'; export { deserializeManifest } from './common.js'; const clientLocalsSymbol = Symbol.for('astro.locals'); @@ -136,10 +139,12 @@ export class App { const host = request.headers.get('X-Forwarded-Host'); if (host) { try { - let hostUrl = new URL(this.removeBase(url.pathname), host); - const maybePathname = this.#manifest.i18n.domainLookupTable[hostUrl.toString()]; - if (maybePathname) { - pathname = maybePathname; + const locale = this.#manifest.i18n.domainLookupTable[host]; + if (locale) { + pathname = prependForwardSlash(joinPaths(locale, this.removeBase(url.pathname))); + if (url.pathname.endsWith('/')) { + pathname = appendForwardSlash(pathname); + } } } catch (e) { // waiting to decide what to do here diff --git a/packages/astro/src/core/build/plugins/plugin-manifest.ts b/packages/astro/src/core/build/plugins/plugin-manifest.ts index 33785d62cd67..fbb0602aaa5f 100644 --- a/packages/astro/src/core/build/plugins/plugin-manifest.ts +++ b/packages/astro/src/core/build/plugins/plugin-manifest.ts @@ -17,6 +17,7 @@ import { cssOrder, mergeInlineCss, type BuildInternals } from '../internal.js'; import type { AstroBuildPlugin } from '../plugin.js'; import type { StaticBuildOptions } from '../types.js'; import { shouldAppendForwardSlash } from '../util.js'; +import { normalizeTheLocale } from '../../../i18n/index.js'; const manifestReplace = '@@ASTRO_MANIFEST_REPLACE@@'; const replaceExp = new RegExp(`['"](${manifestReplace})['"]`, 'g'); @@ -236,31 +237,15 @@ function buildManifest( styles, routeData: serializeRouteData(route, settings.config.trailingSlash), }); + } - /** - * logic meant for i18n domain support, where we fill the lookup table - */ - const i18n = settings.config.experimental.i18n; - if (i18n && i18n.domains && i18n.routingStrategy === 'domain') { - // Domain routing strategy assumes that the locale starts at the beginning of the path, so it's safe to get the first item - // This is something like /es/guides/getting-started - const segments = route.route.split('/'); - const pathnameLocale = segments[1]; - const maybeLocaleDomain = i18n.domains[pathnameLocale]; - if (maybeLocaleDomain) { - // This is something like guides/getting-started - let pathnameWithoutLocale = segments - .filter((segment) => segment !== pathnameLocale) - .slice(1) - .join('/'); - if (shouldAppendForwardSlash(settings.config.trailingSlash, settings.config.build.format)) { - pathnameWithoutLocale = appendForwardSlash(pathnameWithoutLocale); - } - // We build a URL object like this: https://es.example.com/guides/getting-started - const domainPath = new URL(pathnameWithoutLocale, maybeLocaleDomain); - // We map https://es.example.com/guides/getting-started -> /es/guides/getting-started - domainLookupTable[domainPath.toString()] = route.route; - } + /** + * logic meant for i18n domain support, where we fill the lookup table + */ + const i18n = settings.config.experimental.i18n; + if (i18n && i18n.domains && i18n.routingStrategy === 'domain') { + for (const [locale, domainValue] of Object.entries(i18n.domains)) { + domainLookupTable[domainValue] = normalizeTheLocale(locale); } } From 0737f40c9b52b5a5e60ad486fb0ee0635ca1e29d Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 16 Nov 2023 09:07:34 -0600 Subject: [PATCH 3/7] fix validation error --- packages/astro/test/fixtures/i18n-routing/astro.config.mjs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/astro/test/fixtures/i18n-routing/astro.config.mjs b/packages/astro/test/fixtures/i18n-routing/astro.config.mjs index 46630562e9b7..ff4614759bd8 100644 --- a/packages/astro/test/fixtures/i18n-routing/astro.config.mjs +++ b/packages/astro/test/fixtures/i18n-routing/astro.config.mjs @@ -9,7 +9,8 @@ export default defineConfig({ ], domains: { it: "https://it.example.com" - } + }, + routingStrategy: "domain" } }, }) From c441b32f18d03c08758ba6fb1a2cea7776651709 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Fri, 17 Nov 2023 10:05:03 -0600 Subject: [PATCH 4/7] fix test case --- packages/astro/test/units/config/config-validate.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/astro/test/units/config/config-validate.test.js b/packages/astro/test/units/config/config-validate.test.js index d20280a3563e..beb40103c271 100644 --- a/packages/astro/test/units/config/config-validate.test.js +++ b/packages/astro/test/units/config/config-validate.test.js @@ -170,6 +170,7 @@ describe('Config Validation', () => { domains: { lorem: 'https://example.com', }, + routingStrategy: 'domain', }, }, }, From c2c292e0f084c27a42be555861b13164e93cb518 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Fri, 17 Nov 2023 11:49:27 -0600 Subject: [PATCH 5/7] chore: add more cases --- packages/astro/src/core/app/index.ts | 42 +++++++++++++++++++++---- packages/astro/test/i18-routing.test.js | 27 ++++++++++++++-- 2 files changed, 61 insertions(+), 8 deletions(-) diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index 97cdc97fe81c..6d9457303188 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -31,6 +31,7 @@ import { matchRoute } from '../routing/match.js'; import { EndpointNotFoundError, SSRRoutePipeline } from './ssrPipeline.js'; import type { RouteInfo } from './types.js'; import { shouldAppendForwardSlash } from '../build/util.js'; +import { normalizeTheLocale } from '../../i18n/index.js'; export { deserializeManifest } from './common.js'; const clientLocalsSymbol = Symbol.for('astro.locals'); @@ -136,12 +137,41 @@ export class App { if (this.#manifest.assets.has(url.pathname)) return undefined; let pathname; if (this.#manifest.i18n && this.#manifest.i18n.routingStrategy === 'domain') { - const host = request.headers.get('X-Forwarded-Host'); - if (host) { + // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Host + let host = request.headers.get('X-Forwarded-Host'); + // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Proto + let protocol = request.headers.get('X-Forwarded-Proto'); + if (!host) { + // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Host + host = request.headers.get('Host'); + } + // If we don't have a host and a protocol, it's impossible to proceed + if (host && protocol) { + // The header might have a port in their name, so we remove it + host = host.split(':')[0]; try { - const locale = this.#manifest.i18n.domainLookupTable[host]; + let locale; + const hostAsUrl = new URL('', `${protocol}://${host}`); + for (const [domainKey, localeValue] of Object.entries( + this.#manifest.i18n.domainLookupTable + )) { + // This operation should be safe because we force the protocol via zod inside the configuration + // If not, then it means that the manifest was tampered + const domainKeyAsUrl = new URL('', domainKey); + + if ( + hostAsUrl.host === domainKeyAsUrl.host && + hostAsUrl.protocol === domainKeyAsUrl.protocol + ) { + locale = localeValue; + break; + } + } + if (locale) { - pathname = prependForwardSlash(joinPaths(locale, this.removeBase(url.pathname))); + pathname = prependForwardSlash( + joinPaths(normalizeTheLocale(locale), this.removeBase(url.pathname)) + ); if (url.pathname.endsWith('/')) { pathname = appendForwardSlash(pathname); } @@ -149,8 +179,8 @@ export class App { } catch (e) { // waiting to decide what to do here // eslint-disable-next-line no-console + // TODO: What kind of error should we try? This happens if we have an invalid value inside the X-Forwarded-Host and X-Forwarded-Proto headers console.error(e); - // TODO: What kind of error should we try? This happens if we have an invalid value inside the X-Forwarded-Host header } } } @@ -159,7 +189,7 @@ export class App { } let routeData = matchRoute(pathname, this.#manifestData); - // missing routes fall-through, prerendered are handled by static layer + // missing routes fall-through, pre rendered are handled by static layer if (!routeData || routeData.prerender) return undefined; return routeData; } diff --git a/packages/astro/test/i18-routing.test.js b/packages/astro/test/i18-routing.test.js index 153fe297169d..a4f1113c8765 100644 --- a/packages/astro/test/i18-routing.test.js +++ b/packages/astro/test/i18-routing.test.js @@ -1024,15 +1024,38 @@ describe('[SSR] i18n routing', () => { app = await fixture.loadTestAdapterApp(); }); - it('should render the en locale', async () => { + it('should render the en locale when X-Forwarded-Host header is passed', async () => { + let request = new Request('http://example.pt/new-site/start', { + headers: { + 'X-Forwarded-Host': 'example.pt', + 'X-Forwarded-Proto': 'https', + }, + }); + let response = await app.render(request); + expect(response.status).to.equal(200); + expect(await response.text()).includes('Oi essa e start\n'); + }); + + it('should render the en locale when Host header is passed', async () => { let request = new Request('http://example.pt/new-site/start', { headers: { - 'X-Forwarded-Host': 'https://example.pt', + Host: 'example.pt', + 'X-Forwarded-Proto': 'https', }, }); let response = await app.render(request); expect(response.status).to.equal(200); expect(await response.text()).includes('Oi essa e start\n'); }); + + it('should render a 404 because we could not compute the domain', async () => { + let request = new Request('http://example.pt/new-site/start', { + headers: { + Host: 'example.pt', + }, + }); + let response = await app.render(request); + expect(response.status).to.equal(404); + }); }); }); From f244a0d3d6692fdefde4526094bb65e75f79ed45 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Fri, 17 Nov 2023 12:13:39 -0600 Subject: [PATCH 6/7] chore: address comments --- packages/astro/src/core/app/index.ts | 37 +++++++++++++------ .../src/core/build/plugins/plugin-manifest.ts | 3 +- packages/astro/test/i18-routing.test.js | 14 ++++++- 3 files changed, 39 insertions(+), 15 deletions(-) diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index 6d9457303188..c0e516e3288c 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -30,7 +30,6 @@ import { import { matchRoute } from '../routing/match.js'; import { EndpointNotFoundError, SSRRoutePipeline } from './ssrPipeline.js'; import type { RouteInfo } from './types.js'; -import { shouldAppendForwardSlash } from '../build/util.js'; import { normalizeTheLocale } from '../../i18n/index.js'; export { deserializeManifest } from './common.js'; @@ -135,12 +134,33 @@ export class App { const url = new URL(request.url); // ignore requests matching public assets if (this.#manifest.assets.has(url.pathname)) return undefined; - let pathname; + let pathname = this.#computePathnameFromDomain(request); + if (!pathname) { + pathname = prependForwardSlash(this.removeBase(url.pathname)); + } + let routeData = matchRoute(pathname, this.#manifestData); + + // missing routes fall-through, pre rendered are handled by static layer + if (!routeData || routeData.prerender) return undefined; + return routeData; + } + + #computePathnameFromDomain(request: Request): string | undefined { + let pathname: string | undefined = undefined; + const url = new URL(request.url); + if (this.#manifest.i18n && this.#manifest.i18n.routingStrategy === 'domain') { // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Host let host = request.headers.get('X-Forwarded-Host'); // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Proto let protocol = request.headers.get('X-Forwarded-Proto'); + if (protocol) { + // this header doesn't have the colum at the end, so we added to be in line with URL#protocol, which has it + protocol = protocol + ':'; + } else { + // we fall back to the protocol of the request + protocol = url.protocol; + } if (!host) { // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Host host = request.headers.get('Host'); @@ -151,7 +171,7 @@ export class App { host = host.split(':')[0]; try { let locale; - const hostAsUrl = new URL('', `${protocol}://${host}`); + const hostAsUrl = new URL('', `${protocol}//${host}`); for (const [domainKey, localeValue] of Object.entries( this.#manifest.i18n.domainLookupTable )) { @@ -178,20 +198,13 @@ export class App { } } catch (e) { // waiting to decide what to do here - // eslint-disable-next-line no-console // TODO: What kind of error should we try? This happens if we have an invalid value inside the X-Forwarded-Host and X-Forwarded-Proto headers + // eslint-disable-next-line no-console console.error(e); } } } - if (!pathname) { - pathname = prependForwardSlash(this.removeBase(url.pathname)); - } - let routeData = matchRoute(pathname, this.#manifestData); - - // missing routes fall-through, pre rendered are handled by static layer - if (!routeData || routeData.prerender) return undefined; - return routeData; + return pathname; } async render(request: Request, routeData?: RouteData, locals?: object): Promise { diff --git a/packages/astro/src/core/build/plugins/plugin-manifest.ts b/packages/astro/src/core/build/plugins/plugin-manifest.ts index fbb0602aaa5f..e364bc92d580 100644 --- a/packages/astro/src/core/build/plugins/plugin-manifest.ts +++ b/packages/astro/src/core/build/plugins/plugin-manifest.ts @@ -9,14 +9,13 @@ import type { SerializedRouteInfo, SerializedSSRManifest, } from '../../app/types.js'; -import { appendForwardSlash, joinPaths, prependForwardSlash } from '../../path.js'; +import { joinPaths, prependForwardSlash } from '../../path.js'; import { serializeRouteData } from '../../routing/index.js'; import { addRollupInput } from '../add-rollup-input.js'; import { getOutFile, getOutFolder } from '../common.js'; import { cssOrder, mergeInlineCss, type BuildInternals } from '../internal.js'; import type { AstroBuildPlugin } from '../plugin.js'; import type { StaticBuildOptions } from '../types.js'; -import { shouldAppendForwardSlash } from '../util.js'; import { normalizeTheLocale } from '../../../i18n/index.js'; const manifestReplace = '@@ASTRO_MANIFEST_REPLACE@@'; diff --git a/packages/astro/test/i18-routing.test.js b/packages/astro/test/i18-routing.test.js index a4f1113c8765..c9f58537a227 100644 --- a/packages/astro/test/i18-routing.test.js +++ b/packages/astro/test/i18-routing.test.js @@ -1048,8 +1048,20 @@ describe('[SSR] i18n routing', () => { expect(await response.text()).includes('Oi essa e start\n'); }); - it('should render a 404 because we could not compute the domain', async () => { + it('should render the en locale when Host header is passed and it has the port', async () => { let request = new Request('http://example.pt/new-site/start', { + headers: { + Host: 'example.pt:8080', + 'X-Forwarded-Proto': 'https', + }, + }); + let response = await app.render(request); + expect(response.status).to.equal(200); + expect(await response.text()).includes('Oi essa e start\n'); + }); + + it('should render when the protocol header we fallback to the one of the host', async () => { + let request = new Request('https://example.pt/new-site/start', { headers: { Host: 'example.pt', }, From 28f5a7cba12f5a16fa11ff860363ec0ed8d67fd1 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Fri, 17 Nov 2023 12:46:00 -0600 Subject: [PATCH 7/7] test assertion --- packages/astro/test/i18-routing.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/astro/test/i18-routing.test.js b/packages/astro/test/i18-routing.test.js index c9f58537a227..0c50a2c525e5 100644 --- a/packages/astro/test/i18-routing.test.js +++ b/packages/astro/test/i18-routing.test.js @@ -1067,7 +1067,8 @@ describe('[SSR] i18n routing', () => { }, }); let response = await app.render(request); - expect(response.status).to.equal(404); + expect(response.status).to.equal(200); + expect(await response.text()).includes('Oi essa e start\n'); }); }); });