Skip to content

Commit 43657c4

Browse files
matthewpmatthewp
authored
Fix i18n domain host validation (#18096)
1 parent ecf1ff0 commit 43657c4

7 files changed

Lines changed: 124 additions & 29 deletions

File tree

‎.changeset/shiny-places-wish.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'astro': patch
3+
---
4+
5+
Fixes domain-based i18n routing to respect `security.allowedDomains` when selecting a locale from request host headers

‎packages/astro/src/core/app/base.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,7 @@ export abstract class BaseApp {
346346
this.manifest.i18n,
347347
this.manifest.base,
348348
this.manifest.trailingSlash,
349+
this.manifest.allowedDomains,
349350
this.logger,
350351
);
351352
}

‎packages/astro/src/core/fetch/fetch-state.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,7 @@ export class FetchState implements AstroFetchState {
353353
manifest.i18n,
354354
manifest.base,
355355
manifest.trailingSlash,
356+
manifest.allowedDomains,
356357
this.logger,
357358
pathname,
358359
);

‎packages/astro/src/core/i18n/domain.ts‎

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ import {
77
} from '@astrojs/internal-helpers/path';
88
import { normalizeTheLocale } from '../../i18n/path.js';
99
import type { SSRManifest } from '../app/types.js';
10+
import {
11+
getFirstForwardedValue,
12+
validateForwardedHeaders,
13+
validateHost,
14+
} from '../app/validate-headers.js';
1015
import type { AstroLogger } from '../logger/core.js';
1116

1217
/**
@@ -24,6 +29,7 @@ export function computePathnameFromDomain(
2429
i18n: SSRManifest['i18n'],
2530
base: SSRManifest['base'],
2631
trailingSlash: SSRManifest['trailingSlash'],
32+
allowedDomains: SSRManifest['allowedDomains'],
2733
logger: AstroLogger,
2834
pathnameFromRequest?: string,
2935
): string | undefined {
@@ -35,21 +41,19 @@ export function computePathnameFromDomain(
3541
i18n.strategy === 'domains-prefix-other-locales' ||
3642
i18n.strategy === 'domains-prefix-always-no-redirect')
3743
) {
38-
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Host
39-
let host = request.headers.get('X-Forwarded-Host');
40-
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Proto
41-
let protocol = request.headers.get('X-Forwarded-Proto');
42-
if (protocol) {
43-
// this header doesn't have a colon at the end, so we add to be in line with URL#protocol, which does have it
44-
protocol = protocol + ':';
45-
} else {
46-
// we fall back to the protocol of the request
47-
protocol = url.protocol;
48-
}
49-
if (!host) {
50-
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Host
51-
host = request.headers.get('Host');
52-
}
44+
const validated = validateForwardedHeaders(
45+
getFirstForwardedValue(request.headers.get('X-Forwarded-Proto') ?? undefined),
46+
getFirstForwardedValue(request.headers.get('X-Forwarded-Host') ?? undefined),
47+
getFirstForwardedValue(request.headers.get('X-Forwarded-Port') ?? undefined),
48+
allowedDomains,
49+
);
50+
const protocol = validated.protocol ? `${validated.protocol}:` : url.protocol;
51+
const requestHost = request.headers.get('Host') ?? undefined;
52+
const validatedRequestHost = allowedDomains?.length
53+
? validateHost(requestHost, protocol.slice(0, -1), allowedDomains)
54+
: requestHost;
55+
// Forwarded and original hosts are validated against security.allowedDomains when configured.
56+
let host = validated.host ?? validatedRequestHost;
5357
// If we don't have a host and a protocol, it's impossible to proceed
5458
if (host && protocol) {
5559
// The header might have a port in their name, so we remove it

‎packages/astro/src/core/routing/match-request.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ export function matchRequest(
5151
manifest.i18n,
5252
manifest.base,
5353
manifest.trailingSlash,
54+
manifest.allowedDomains,
5455
getLogger(manifest),
5556
);
5657
if (!pathname) {

‎packages/astro/test/units/i18n/domain.test.ts‎

Lines changed: 65 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import assert from 'node:assert/strict';
22
import { describe, it } from 'node:test';
33
import { computePathnameFromDomain } from '../../../dist/core/i18n/domain.js';
4-
import type { SSRManifestI18n } from '../../../dist/core/app/types.js';
4+
import type { SSRManifest, SSRManifestI18n } from '../../../dist/core/app/types.js';
55
import type { RoutingStrategies } from '../../../dist/core/app/common.js';
66
import type { Locales } from '../../../dist/types/public/config.js';
7-
import { defaultLogger, SpyLogger } from '../test-utils.ts';
7+
import { defaultLogger } from '../test-utils.ts';
88

99
interface I18nOverrides {
1010
strategy?: RoutingStrategies;
@@ -38,6 +38,7 @@ function run(
3838
i18n?: SSRManifestI18n | undefined;
3939
base?: string;
4040
trailingSlash?: 'always' | 'never' | 'ignore';
41+
allowedDomains?: SSRManifest['allowedDomains'];
4142
logger?: typeof defaultLogger;
4243
} = {},
4344
) {
@@ -46,9 +47,18 @@ function run(
4647
const i18n = 'i18n' in opts ? opts.i18n : makeI18n();
4748
const base = opts.base ?? '/';
4849
const trailingSlash = opts.trailingSlash ?? 'ignore';
50+
const allowedDomains = opts.allowedDomains ?? [{ hostname: 'example.fr' }];
4951
const logger = opts.logger ?? defaultLogger;
5052
const [request, parsedUrl] = reqAndUrl(url, headers);
51-
return computePathnameFromDomain(request, parsedUrl, i18n, base, trailingSlash, logger);
53+
return computePathnameFromDomain(
54+
request,
55+
parsedUrl,
56+
i18n,
57+
base,
58+
trailingSlash,
59+
allowedDomains,
60+
logger,
61+
);
5262
}
5363

5464
describe('computePathnameFromDomain', () => {
@@ -90,10 +100,60 @@ describe('computePathnameFromDomain', () => {
90100
);
91101
});
92102

103+
it('ignores X-Forwarded-Host when allowedDomains is not configured', () => {
104+
assert.equal(
105+
run(
106+
'https://example.com/about',
107+
{ Host: 'example.com', 'X-Forwarded-Host': 'example.fr' },
108+
{ allowedDomains: [] },
109+
),
110+
undefined,
111+
);
112+
});
113+
114+
it('ignores X-Forwarded-Host when it does not match allowedDomains', () => {
115+
assert.equal(
116+
run(
117+
'https://example.com/about',
118+
{ Host: 'example.com', 'X-Forwarded-Host': 'example.fr' },
119+
{ allowedDomains: [{ hostname: 'example.com' }] },
120+
),
121+
undefined,
122+
);
123+
});
124+
93125
it('falls back to the Host header when X-Forwarded-Host is absent', () => {
94126
assert.equal(run('https://example.fr/about', { Host: 'example.fr' }), '/fr/about');
95127
});
96128

129+
it('ignores the Host header when it does not match configured allowedDomains', () => {
130+
assert.equal(
131+
run(
132+
'https://example.fr/about',
133+
{ Host: 'example.fr' },
134+
{ allowedDomains: [{ hostname: 'example.com' }] },
135+
),
136+
undefined,
137+
);
138+
});
139+
140+
it('uses the Host header without validation when allowedDomains is not configured', () => {
141+
assert.equal(
142+
run('https://example.fr/about', { Host: 'example.fr' }, { allowedDomains: [] }),
143+
'/fr/about',
144+
);
145+
});
146+
147+
it('uses the first value from a comma-separated X-Forwarded-Host header', () => {
148+
assert.equal(
149+
run('https://example.fr/about', {
150+
'X-Forwarded-Host': 'example.fr, proxy.internal',
151+
'X-Forwarded-Proto': 'https, http',
152+
}),
153+
'/fr/about',
154+
);
155+
});
156+
97157
it('strips a port from the forwarded host before matching', () => {
98158
assert.equal(
99159
run('https://example.fr/about', {
@@ -186,13 +246,7 @@ describe('computePathnameFromDomain', () => {
186246
);
187247
});
188248

189-
it('logs an error and returns undefined when the host cannot be parsed as a URL', () => {
190-
const logger = new SpyLogger();
191-
const result = run('https://example.fr/about', { 'X-Forwarded-Host': '[' }, { logger });
192-
assert.equal(result, undefined);
193-
assert.ok(
194-
logger.logs.some((entry) => entry.level === 'error' && entry.label === 'router'),
195-
'expected a router error to be logged',
196-
);
249+
it('ignores a forwarded host that cannot be parsed as a URL', () => {
250+
assert.equal(run('https://example.fr/about', { 'X-Forwarded-Host': '[' }), undefined);
197251
});
198252
});

‎packages/astro/test/units/i18n/i18n-app.test.ts‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -183,9 +183,12 @@ describe('i18n via App - domains-prefix-always', () => {
183183

184184
const middleware = createI18nMiddleware(i18n, '/', 'ignore', 'directory');
185185

186-
function createDomainApp() {
186+
function createDomainApp(
187+
allowedDomains = [{ hostname: 'example.pt' }, { hostname: 'it.example.com' }],
188+
) {
187189
return createTestApp([localeCatchAll('en'), localeCatchAll('pt'), localeCatchAll('it')], {
188190
i18n,
191+
allowedDomains,
189192
middleware: () => ({ onRequest: middleware }),
190193
});
191194
}
@@ -214,6 +217,22 @@ describe('i18n via App - domains-prefix-always', () => {
214217
assert.equal($('#locale').text(), 'it');
215218
});
216219

220+
it('ignores a locale domain in X-Forwarded-Host when it is not allowed', async () => {
221+
const app = createDomainApp([{ hostname: 'it.example.com' }]);
222+
const res = await app.render(
223+
new Request('https://it.example.com/about', {
224+
headers: {
225+
Host: 'it.example.com',
226+
'X-Forwarded-Host': 'example.pt',
227+
'X-Forwarded-Proto': 'https',
228+
},
229+
}),
230+
);
231+
assert.equal(res.status, 200);
232+
const $ = cheerio.load(await res.text());
233+
assert.equal($('#locale').text(), 'it');
234+
});
235+
217236
it('renders English locale for non-domain request with /en/ prefix', async () => {
218237
const app = createDomainApp();
219238
const res = await app.render(new Request('http://example.com/en/about'));
@@ -320,6 +339,7 @@ describe('i18n via App - domains-prefix-always with trailingSlash: never', () =>
320339
function createDomainApp() {
321340
return createTestApp([localeSpreadCatchAll('fi'), localeSpreadCatchAll('en')], {
322341
i18n,
342+
allowedDomains: [{ hostname: 'example.com' }, { hostname: 'example.fi' }],
323343
trailingSlash: 'never',
324344
middleware: () => ({ onRequest: middleware }),
325345
});
@@ -395,7 +415,11 @@ describe('i18n via App - domains-prefix-other-locales', () => {
395415
}),
396416
localeCatchAll('pt'),
397417
],
398-
{ i18n, middleware: () => ({ onRequest: middleware }) },
418+
{
419+
i18n,
420+
allowedDomains: [{ hostname: 'example.pt' }],
421+
middleware: () => ({ onRequest: middleware }),
422+
},
399423
);
400424
}
401425

@@ -455,6 +479,7 @@ describe('i18n via App - domains-prefix-other-locales with dynamic params (#1685
455479
enPage.routeData.pathname = undefined;
456480
return createTestApp([fiPage, enPage], {
457481
i18n,
482+
allowedDomains: [{ hostname: 'en.example.com' }],
458483
middleware: () => ({ onRequest: middleware }),
459484
});
460485
}
@@ -588,7 +613,11 @@ describe('i18n via App - domain with localhost and ports (#12385)', () => {
588613
}),
589614
localeCatchAll('zh'),
590615
],
591-
{ i18n, middleware: () => ({ onRequest: middleware }) },
616+
{
617+
i18n,
618+
allowedDomains: [{ hostname: 'zh.test' }],
619+
middleware: () => ({ onRequest: middleware }),
620+
},
592621
);
593622
}
594623

0 commit comments

Comments
 (0)