Skip to content

Commit de0085d

Browse files
SkyZeroZxSkyZeroZx
committed
fix(@angular/ssr): limit the length of redirect locations
Redirects created on the server can contain data from the request. Route redirects keep the path and the query string, redirects done by the router while rendering use its final URL, and the `X-Forwarded-Prefix` header is prepended. The router also encodes some characters again, so the `Location` header can be several times longer than the request URL. Reverse proxies read the response headers into a buffer of limited size, 4 KB by default in NGINX. When the headers do not fit, the proxy returns a 502 and, with passive health checks, counts it against the server. A few requests could take every server out of rotation. `AngularAppEngine` and `AngularNodeAppEngine` now return a 400 when a redirect location is longer than 2048 characters. The limit can be changed with the new `maxRedirectLocationLength` option. Redirects created while prerendering are not limited. Fixes #34365
1 parent 04222d6 commit de0085d

7 files changed

Lines changed: 244 additions & 3 deletions

File tree

‎goldens/public-api/angular/ssr/index.api.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ export class AngularAppEngine {
2222
// @public
2323
export interface AngularAppEngineOptions {
2424
allowedHosts?: readonly string[];
25+
maxRedirectLocationLength?: number;
2526
trustProxyHeaders?: boolean | readonly string[];
2627
}
2728

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

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import type { AngularServerApp, getOrCreateAngularServerApp } from './app';
1010
import { Hooks } from './hooks';
1111
import { getPotentialLocaleIdFromUrl, getPreferredLocale } from './i18n';
1212
import { EntryPointExports, getAngularAppEngineManifest } from './manifest';
13-
import { createRedirectResponse } from './utils/redirect';
13+
import { DEFAULT_MAX_REDIRECT_LOCATION_LENGTH, createRedirectResponse } from './utils/redirect';
1414
import { joinUrlParts } from './utils/url';
1515
import {
1616
getFirstHeaderValue,
@@ -46,6 +46,22 @@ export interface AngularAppEngineOptions {
4646
* @default false
4747
*/
4848
trustProxyHeaders?: boolean | readonly string[];
49+
50+
/**
51+
* The maximum length of the `Location` header in redirect responses.
52+
*
53+
* When the location of a redirect is longer, a 400 (Bad Request) response is returned instead.
54+
*
55+
* @remarks
56+
* Redirect locations can contain data from the request, such as the path, the query string and
57+
* the `X-Forwarded-Prefix` header. Reverse proxies and load balancers read the response headers
58+
* into a buffer of limited size (4 KB by default in NGINX) and fail the request when they do not
59+
* fit. Only increase this value when every proxy in front of the server accepts larger response
60+
* headers.
61+
*
62+
* @default 2048
63+
*/
64+
maxRedirectLocationLength?: number;
4965
}
5066

5167
/**
@@ -107,6 +123,11 @@ export class AngularAppEngine {
107123
*/
108124
private readonly trustProxyHeaders: ReadonlySet<string>;
109125

126+
/**
127+
* The maximum length of the `Location` header in redirect responses.
128+
*/
129+
readonly #maxRedirectLocationLength: number;
130+
110131
/**
111132
* A cache that holds entry points, keyed by their potential locale string.
112133
*/
@@ -119,6 +140,16 @@ export class AngularAppEngine {
119140
constructor(options?: AngularAppEngineOptions) {
120141
this.allowedHosts = this.getAllowedHosts(options);
121142
this.trustProxyHeaders = normalizeTrustProxyHeaders(options?.trustProxyHeaders);
143+
144+
const maxRedirectLocationLength =
145+
options?.maxRedirectLocationLength ?? DEFAULT_MAX_REDIRECT_LOCATION_LENGTH;
146+
if (!Number.isInteger(maxRedirectLocationLength) || maxRedirectLocationLength < 1) {
147+
throw new Error(
148+
`"maxRedirectLocationLength" must be a positive integer. Received: ${maxRedirectLocationLength}.`,
149+
);
150+
}
151+
152+
this.#maxRedirectLocationLength = maxRedirectLocationLength;
122153
}
123154

124155
private getAllowedHosts(options: AngularAppEngineOptions | undefined): ReadonlySet<string> {
@@ -216,6 +247,7 @@ export class AngularAppEngine {
216247
302,
217248
// Use a 302 redirect as language preference may change.
218249
{ 'Vary': 'Accept-Language' },
250+
this.#maxRedirectLocationLength,
219251
);
220252
}
221253
}
@@ -250,6 +282,7 @@ export class AngularAppEngine {
250282
const serverApp = ɵgetOrCreateAngularServerApp({
251283
allowStaticRouteRender: AngularAppEngine.ɵallowStaticRouteRender,
252284
hooks: AngularAppEngine.ɵhooks,
285+
maxRedirectLocationLength: this.#maxRedirectLocationLength,
253286
});
254287

255288
return serverApp;

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,14 @@ interface AngularServerAppOptions {
7777
* If not provided, a new `Hooks` instance is created.
7878
*/
7979
hooks?: Hooks;
80+
81+
/**
82+
* The maximum length of the `Location` header in redirect responses.
83+
*
84+
* When a redirect location is longer, a 400 (Bad Request) response is returned instead.
85+
* If not provided, the length is not limited, such as when prerendering.
86+
*/
87+
maxRedirectLocationLength?: number;
8088
}
8189

8290
/**
@@ -177,6 +185,7 @@ export class AngularServerApp {
177185
relativeRedirect ? location + url.search : location,
178186
status,
179187
headers,
188+
this.options.maxRedirectLocationLength,
180189
);
181190
}
182191

@@ -330,7 +339,12 @@ export class AngularServerApp {
330339
}
331340

332341
if (result.redirectTo) {
333-
return createRedirectResponse(result.redirectTo, responseInit.status, responseInit.headers);
342+
return createRedirectResponse(
343+
result.redirectTo,
344+
responseInit.status,
345+
responseInit.headers,
346+
this.options.maxRedirectLocationLength,
347+
);
334348
}
335349

336350
if (renderMode === RenderMode.Prerender) {

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,17 @@ export const VALID_REDIRECT_RESPONSE_CODES: ReadonlySet<number> = new Set([
1313
301, 302, 303, 307, 308,
1414
]);
1515

16+
/**
17+
* The default maximum length of the `Location` header in redirect responses.
18+
*
19+
* Redirect locations can contain data from the request, such as the path, the query string and
20+
* the `X-Forwarded-Prefix` header, which the Angular router can encode into a longer string than
21+
* it received. Reverse proxies read the response headers into a buffer of limited size (4 KB by
22+
* default in NGINX) and fail the request when they do not fit. The default leaves room in such a
23+
* buffer for the status line and the other response headers.
24+
*/
25+
export const DEFAULT_MAX_REDIRECT_LOCATION_LENGTH = 2048;
26+
1627
/**
1728
* Checks if the given HTTP status code is a valid redirect response code.
1829
*
@@ -30,12 +41,16 @@ export function isValidRedirectResponseCode(code: number): boolean {
3041
* @param status - The HTTP status code for the redirection. Defaults to 302 (Found).
3142
* See: https://developer.mozilla.org/en-US/docs/Web/API/Response/redirect_static#status
3243
* @param headers - Additional headers to include in the response.
44+
* @param maxLocationLength - The maximum allowed length of the location. When the location is
45+
* longer, a 400 (Bad Request) response is returned instead. When not
46+
* provided, the length of the location is not limited.
3347
* @returns A `Response` object representing the HTTP redirect.
3448
*/
3549
export function createRedirectResponse(
3650
location: string,
3751
status = 302,
3852
headers?: Record<string, string> | Headers,
53+
maxLocationLength?: number,
3954
): Response {
4055
if (ngDevMode && !isValidRedirectResponseCode(status)) {
4156
throw new Error(
@@ -44,6 +59,24 @@ export function createRedirectResponse(
4459
);
4560
}
4661

62+
if (maxLocationLength !== undefined && location.length > maxLocationLength) {
63+
const errorMessage =
64+
`Redirect location is ${location.length} characters long, which exceeds ` +
65+
`the maximum allowed length of ${maxLocationLength} characters.`;
66+
67+
// eslint-disable-next-line no-console
68+
console.error(
69+
`ERROR: Bad Request.\n${errorMessage}\n` +
70+
'The limit can be changed with the "maxRedirectLocationLength" option of "AngularAppEngine" or "AngularNodeAppEngine".',
71+
);
72+
73+
return new Response(errorMessage, {
74+
status: 400,
75+
statusText: 'Bad Request',
76+
headers: { 'Content-Type': 'text/plain' },
77+
});
78+
}
79+
4780
const resHeaders = headers instanceof Headers ? headers : new Headers(headers);
4881
if (ngDevMode && resHeaders.has('location')) {
4982
// eslint-disable-next-line no-console

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

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { destroyAngularServerApp, getOrCreateAngularServerApp } from '../src/app
1616
import { AngularAppEngine } from '../src/app-engine';
1717
import { setAngularAppEngineManifest } from '../src/manifest';
1818
import { RenderMode } from '../src/routes/route-config';
19+
import { DEFAULT_MAX_REDIRECT_LOCATION_LENGTH } from '../src/utils/redirect';
1920
import { setAngularAppTestingManifest } from './testing-utils';
2021

2122
@Component({
@@ -215,6 +216,41 @@ describe('AngularAppEngine', () => {
215216
expect(response?.headers.get('Location')).toBe('/app/it');
216217
});
217218

219+
it('should return a 400 status when the locale redirect location is too long', async () => {
220+
const consoleErrorSpy = spyOn(console, 'error');
221+
const request = new Request('https://example.com', {
222+
headers: {
223+
'Accept-Language': 'it',
224+
'X-Forwarded-Prefix': '/' + 'a'.repeat(DEFAULT_MAX_REDIRECT_LOCATION_LENGTH),
225+
},
226+
});
227+
228+
const response = await appEngine.handle(request);
229+
expect(response?.status).toBe(400);
230+
expect(response?.headers.get('Location')).toBeNull();
231+
expect(consoleErrorSpy).toHaveBeenCalledWith(
232+
jasmine.stringMatching('exceeds the maximum allowed length of 2048 characters'),
233+
);
234+
});
235+
236+
it('should allow longer locale redirect locations when "maxRedirectLocationLength" is set', async () => {
237+
const prefix = '/' + 'a'.repeat(DEFAULT_MAX_REDIRECT_LOCATION_LENGTH);
238+
const engine = new AngularAppEngine({
239+
trustProxyHeaders: true,
240+
maxRedirectLocationLength: 4096,
241+
});
242+
const request = new Request('https://example.com', {
243+
headers: {
244+
'Accept-Language': 'it',
245+
'X-Forwarded-Prefix': prefix,
246+
},
247+
});
248+
249+
const response = await engine.handle(request);
250+
expect(response?.status).toBe(302);
251+
expect(response?.headers.get('Location')).toBe(`${prefix}/it`);
252+
});
253+
218254
it('should completely ignore proxy headers if not allowed', async () => {
219255
const strictEngine = new AngularAppEngine({ trustProxyHeaders: false });
220256
const request = new Request('https://example.com', {
@@ -312,7 +348,10 @@ describe('AngularAppEngine', () => {
312348
entryPoints: {
313349
'': async () => {
314350
setAngularAppTestingManifest(
315-
[{ path: 'home', component: HomeComponent }],
351+
[
352+
{ path: 'home', component: HomeComponent },
353+
{ path: 'redirect', redirectTo: 'home' },
354+
],
316355
[{ path: '**', renderMode: RenderMode.Server }],
317356
);
318357

@@ -352,6 +391,39 @@ describe('AngularAppEngine', () => {
352391
const response = await appEngine.handle(request);
353392
expect(await response?.text()).toContain('Home works');
354393
});
394+
395+
it('should return a 400 status when the redirect location exceeds the default maximum length', async () => {
396+
const consoleErrorSpy = spyOn(console, 'error');
397+
const engine = new AngularAppEngine();
398+
const request = new Request(
399+
`https://example.com/redirect?q=${'a'.repeat(DEFAULT_MAX_REDIRECT_LOCATION_LENGTH)}`,
400+
);
401+
402+
const response = await engine.handle(request);
403+
expect(response?.status).toBe(400);
404+
expect(response?.headers.get('Location')).toBeNull();
405+
expect(consoleErrorSpy).toHaveBeenCalledWith(
406+
jasmine.stringMatching('exceeds the maximum allowed length of 2048 characters'),
407+
);
408+
});
409+
410+
it('should allow longer redirect locations when "maxRedirectLocationLength" is set', async () => {
411+
const query = 'a'.repeat(DEFAULT_MAX_REDIRECT_LOCATION_LENGTH);
412+
const engine = new AngularAppEngine({ maxRedirectLocationLength: 4096 });
413+
const request = new Request(`https://example.com/redirect?q=${query}`);
414+
415+
const response = await engine.handle(request);
416+
expect(response?.status).toBe(302);
417+
expect(response?.headers.get('Location')).toBe(`/home?q=${query}`);
418+
});
419+
420+
it('should throw an error when "maxRedirectLocationLength" is not a positive integer', () => {
421+
for (const maxRedirectLocationLength of [0, -1, 1.5, NaN, Infinity]) {
422+
expect(() => new AngularAppEngine({ maxRedirectLocationLength }))
423+
.withContext(`maxRedirectLocationLength: ${maxRedirectLocationLength}`)
424+
.toThrowError(/"maxRedirectLocationLength" must be a positive integer/);
425+
}
426+
});
355427
});
356428

357429
describe('Invalid host headers', () => {

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,59 @@ describe('AngularServerApp', () => {
313313
});
314314
});
315315

316+
describe('redirect location length', () => {
317+
let appWithLimit: AngularServerApp;
318+
319+
beforeEach(() => {
320+
// Other tests replace the manifest, which is read when the app is created.
321+
setupManifest();
322+
appWithLimit = new AngularServerApp({ maxRedirectLocationLength: 100 });
323+
});
324+
325+
it('should not limit the length of redirect locations when "maxRedirectLocationLength" is not set', async () => {
326+
const filter = 'a'.repeat(5000);
327+
const response = await app.handle(
328+
new Request(`http://localhost/redirect?filter=${filter}`),
329+
);
330+
expect(response?.headers.get('location')).toBe(`/home?filter=${filter}`);
331+
expect(response?.status).toBe(302);
332+
});
333+
334+
it('should redirect when the location has the maximum allowed length', async () => {
335+
// '/home?filter=' is 13 characters long.
336+
const filter = 'a'.repeat(87);
337+
const response = await appWithLimit.handle(
338+
new Request(`http://localhost/redirect?filter=${filter}`),
339+
);
340+
expect(response?.headers.get('location')).toBe(`/home?filter=${filter}`);
341+
expect(response?.status).toBe(302);
342+
});
343+
344+
it('should return a 400 status when the location of a route redirect is too long', async () => {
345+
const consoleErrorSpy = spyOn(console, 'error');
346+
const response = await appWithLimit.handle(
347+
new Request(`http://localhost/redirect?filter=${'a'.repeat(88)}`),
348+
);
349+
expect(response?.headers.get('location')).toBeNull();
350+
expect(response?.status).toBe(400);
351+
expect(await response?.text()).toContain(
352+
'exceeds the maximum allowed length of 100 characters.',
353+
);
354+
expect(consoleErrorSpy).toHaveBeenCalledWith(
355+
jasmine.stringMatching('Redirect location is 101 characters long'),
356+
);
357+
});
358+
359+
it('should return a 400 status when the location of a redirect during rendering is too long', async () => {
360+
spyOn(console, 'error');
361+
const response = await appWithLimit.handle(
362+
new Request(`http://localhost/redirect-to-function?filter=${'a'.repeat(200)}`),
363+
);
364+
expect(response?.headers.get('location')).toBeNull();
365+
expect(response?.status).toBe(400);
366+
});
367+
});
368+
316369
describe('SSG pages', () => {
317370
it('should correctly serve the content for the requested prerendered page', async () => {
318371
const response = await app.handle(new Request('http://localhost/home-ssg'));

‎packages/angular/ssr/test/utils/redirect_spec.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,41 @@ describe('Redirect Utils', () => {
4646
expect(response.headers.get('Vary')).toBe('X-Forwarded-Prefix, Host');
4747
});
4848

49+
it('should create a redirect response when the location has the maximum allowed length', () => {
50+
const location = '/' + 'a'.repeat(99);
51+
const response = createRedirectResponse(location, 302, undefined, 100);
52+
expect(response.status).toBe(302);
53+
expect(response.headers.get('Location')).toBe(location);
54+
});
55+
56+
it('should return a 400 response when the location exceeds the maximum allowed length', async () => {
57+
const errorSpy = spyOn(console, 'error');
58+
const response = createRedirectResponse(
59+
'/' + 'a'.repeat(100),
60+
301,
61+
{ 'Cache-Control': 'public, max-age=3600' },
62+
100,
63+
);
64+
65+
expect(response.status).toBe(400);
66+
expect(response.headers.get('Location')).toBeNull();
67+
expect(response.headers.get('Cache-Control')).toBeNull();
68+
expect(response.headers.get('Content-Type')).toBe('text/plain');
69+
expect(await response.text()).toBe(
70+
'Redirect location is 101 characters long, which exceeds the maximum allowed length of 100 characters.',
71+
);
72+
expect(errorSpy).toHaveBeenCalledWith(
73+
jasmine.stringMatching('Redirect location is 101 characters long'),
74+
);
75+
});
76+
77+
it('should not limit the length of the location when no maximum length is provided', () => {
78+
const location = '/' + 'a'.repeat(10_000);
79+
const response = createRedirectResponse(location);
80+
expect(response.status).toBe(302);
81+
expect(response.headers.get('Location')).toBe(location);
82+
});
83+
4984
it('should warn if Location header is provided in extra headers in dev mode', () => {
5085
// @ts-expect-error accessing global
5186
globalThis.ngDevMode = true;

0 commit comments

Comments
 (0)