Skip to content

Commit b42cfee

Browse files
SkyZeroZxclydin
authored andcommitted
fix(@angular/ssr): use only the first value of the X-Forwarded-Prefix header
The `X-Forwarded-Prefix` header is validated using its first comma-separated value, like the other `X-Forwarded-*` headers. However, redirects used the entire header value. With a trusted header, any value after the first one was added to the `Location` header without being validated. The first value is now used wherever the header is read, which matches how `X-Forwarded-Host`, `X-Forwarded-Proto` and `X-Forwarded-Port` are handled.
1 parent adcdcc6 commit b42cfee

5 files changed

Lines changed: 31 additions & 3 deletions

File tree

‎packages/angular/ssr/src/app-engine.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { EntryPointExports, getAngularAppEngineManifest } from './manifest';
1313
import { createRedirectResponse } from './utils/redirect';
1414
import { joinUrlParts } from './utils/url';
1515
import {
16+
getFirstHeaderValue,
1617
normalizeTrustProxyHeaders,
1718
sanitizeRequestHeaders,
1819
validateRequest,
@@ -208,7 +209,7 @@ export class AngularAppEngine {
208209
if (preferredLocale) {
209210
const subPath = supportedLocales[preferredLocale];
210211
if (subPath !== undefined) {
211-
const prefix = request.headers.get('X-Forwarded-Prefix') ?? '';
212+
const prefix = getFirstHeaderValue(request.headers.get('X-Forwarded-Prefix')) ?? '';
212213

213214
return createRedirectResponse(
214215
joinUrlParts(prefix, pathname, subPath),

‎packages/angular/ssr/src/app.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { AngularBootstrap, renderAngular } from './utils/ng';
2525
import { promiseWithAbort } from './utils/promise';
2626
import { createRedirectResponse } from './utils/redirect';
2727
import { buildPathWithParams, joinUrlParts, stripLeadingSlash } from './utils/url';
28+
import { getFirstHeaderValue } from './utils/validation';
2829

2930
/**
3031
* A set of well-known URLs that are not handled by Angular.
@@ -167,7 +168,7 @@ export class AngularServerApp {
167168

168169
if (redirectTo !== undefined) {
169170
const location = joinUrlParts(
170-
request.headers.get('X-Forwarded-Prefix') ?? '',
171+
getFirstHeaderValue(request.headers.get('X-Forwarded-Prefix')) ?? '',
171172
buildPathWithParams(redirectTo, url.pathname),
172173
);
173174

‎packages/angular/ssr/src/utils/ng.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
import { ActivatedRoute, Router } from '@angular/router';
2626
import { Console } from '../console';
2727
import { addTrailingSlash, joinUrlParts, stripIndexHtmlFromURL, stripTrailingSlash } from './url';
28+
import { getFirstHeaderValue } from './validation';
2829

2930
/**
3031
* Represents the bootstrap mechanism for an Angular application.
@@ -126,7 +127,9 @@ export async function renderAngular(
126127

127128
const requestPrefix =
128129
envInjector.get(APP_BASE_HREF, null, { optional: true }) ??
129-
envInjector.get(REQUEST, null, { optional: true })?.headers.get('X-Forwarded-Prefix');
130+
getFirstHeaderValue(
131+
envInjector.get(REQUEST, null, { optional: true })?.headers.get('X-Forwarded-Prefix'),
132+
);
130133

131134
const { pathname, search, hash } = envInjector.get(PlatformLocation);
132135
const finalUrl = constructSerializedUrl(router, { pathname, search, hash }, requestPrefix);

‎packages/angular/ssr/test/app-engine_spec.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,19 @@ describe('AngularAppEngine', () => {
202202
expect(response?.headers.get('Vary')).toBe('X-Forwarded-Prefix, Accept-Language');
203203
});
204204

205+
it('should only use the first value of the forwarded prefix in locale redirect location', async () => {
206+
const request = new Request('https://example.com', {
207+
headers: {
208+
'Accept-Language': 'it',
209+
'X-Forwarded-Prefix': '/app, /"<other>',
210+
},
211+
});
212+
213+
const response = await appEngine.handle(request);
214+
expect(response?.status).toBe(302);
215+
expect(response?.headers.get('Location')).toBe('/app/it');
216+
});
217+
205218
it('should completely ignore proxy headers if not allowed', async () => {
206219
const strictEngine = new AngularAppEngine({ trustProxyHeaders: false });
207220
const request = new Request('https://example.com', {

‎packages/angular/ssr/test/app_spec.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,16 @@ describe('AngularServerApp', () => {
538538
expect(response?.headers.get('location')).toBe('/base/home?filter=test');
539539
expect(response?.status).toBe(302);
540540
});
541+
542+
it('should only use the first value of the "X-Forwarded-Prefix" header in redirects', async () => {
543+
const response = await app.handle(
544+
new Request('http://localhost/redirect', {
545+
headers: { 'X-Forwarded-Prefix': '/base, /"<other>' },
546+
}),
547+
);
548+
expect(response?.headers.get('location')).toBe('/base/home');
549+
expect(response?.status).toBe(302);
550+
});
541551
});
542552

543553
describe('Critical CSS inlining', () => {

0 commit comments

Comments
 (0)