From f244a0d3d6692fdefde4526094bb65e75f79ed45 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Fri, 17 Nov 2023 12:13:39 -0600 Subject: [PATCH] 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', },