Fix SSR redirect location length

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

@@ -22,6 +22,7 @@ export class AngularAppEngine {
 // @public
 export interface AngularAppEngineOptions {
     allowedHosts?: readonly string[];
+    maxRedirectLocationLength?: number;
     trustProxyHeaders?: boolean | readonly string[];
 }
 

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

@@ -10,9 +10,10 @@ import type { AngularServerApp, getOrCreateAngularServerApp } from './app';
 import { Hooks } from './hooks';
 import { getPotentialLocaleIdFromUrl, getPreferredLocale } from './i18n';
 import { EntryPointExports, getAngularAppEngineManifest } from './manifest';
-import { createRedirectResponse } from './utils/redirect';
+import { DEFAULT_MAX_REDIRECT_LOCATION_LENGTH, createRedirectResponse } from './utils/redirect';
 import { joinUrlParts } from './utils/url';
 import {
+  getFirstHeaderValue,
   normalizeTrustProxyHeaders,
   sanitizeRequestHeaders,
   validateRequest,
@@ -45,6 +46,22 @@ export interface AngularAppEngineOptions {
    * @default false
    */
   trustProxyHeaders?: boolean | readonly string[];
+
+  /**
+   * The maximum length of the `Location` header in redirect responses.
+   *
+   * When the location of a redirect is longer, a 400 (Bad Request) response is returned instead.
+   *
+   * @remarks
+   * Redirect locations can contain data from the request, such as the path, the query string and
+   * the `X-Forwarded-Prefix` header. Reverse proxies and load balancers read the response headers
+   * into a buffer of limited size (4 KB by default in NGINX) and fail the request when they do not
+   * fit. Only increase this value when every proxy in front of the server accepts larger response
+   * headers.
+   *
+   * @default 2048
+   */
+  maxRedirectLocationLength?: number;
 }
 
 /**
@@ -106,6 +123,11 @@ export class AngularAppEngine {
    */
   private readonly trustProxyHeaders: ReadonlySet<string>;
 
+  /**
+   * The maximum length of the `Location` header in redirect responses.
+   */
+  readonly #maxRedirectLocationLength: number;
+
   /**
    * A cache that holds entry points, keyed by their potential locale string.
    */
@@ -118,6 +140,16 @@ export class AngularAppEngine {
   constructor(options?: AngularAppEngineOptions) {
     this.allowedHosts = this.getAllowedHosts(options);
     this.trustProxyHeaders = normalizeTrustProxyHeaders(options?.trustProxyHeaders);
+
+    const maxRedirectLocationLength =
+      options?.maxRedirectLocationLength ?? DEFAULT_MAX_REDIRECT_LOCATION_LENGTH;
+    if (!Number.isInteger(maxRedirectLocationLength) || maxRedirectLocationLength < 1) {
+      throw new Error(
+        `"maxRedirectLocationLength" must be a positive integer. Received: ${maxRedirectLocationLength}.`,
+      );
+    }
+
+    this.#maxRedirectLocationLength = maxRedirectLocationLength;
   }
 
   private getAllowedHosts(options: AngularAppEngineOptions | undefined): ReadonlySet<string> {
@@ -208,13 +240,14 @@ export class AngularAppEngine {
     if (preferredLocale) {
       const subPath = supportedLocales[preferredLocale];
       if (subPath !== undefined) {
-        const prefix = request.headers.get('X-Forwarded-Prefix') ?? '';
+        const prefix = getFirstHeaderValue(request.headers.get('X-Forwarded-Prefix')) ?? '';
 
         return createRedirectResponse(
           joinUrlParts(prefix, pathname, subPath),
           302,
           // Use a 302 redirect as language preference may change.
           { 'Vary': 'Accept-Language' },
+          this.#maxRedirectLocationLength,
         );
       }
     }
@@ -249,6 +282,7 @@ export class AngularAppEngine {
     const serverApp = ɵgetOrCreateAngularServerApp({
       allowStaticRouteRender: AngularAppEngine.ɵallowStaticRouteRender,
       hooks: AngularAppEngine.ɵhooks,
+      maxRedirectLocationLength: this.#maxRedirectLocationLength,
     });
 
     return serverApp;

packages/angular/ssr/src/app.ts

@@ -25,6 +25,7 @@ import { AngularBootstrap, renderAngular } from './utils/ng';
 import { promiseWithAbort } from './utils/promise';
 import { createRedirectResponse } from './utils/redirect';
 import { buildPathWithParams, joinUrlParts, stripLeadingSlash } from './utils/url';
+import { getFirstHeaderValue } from './utils/validation';
 
 /**
  * A set of well-known URLs that are not handled by Angular.
@@ -76,6 +77,14 @@ interface AngularServerAppOptions {
    * If not provided, a new `Hooks` instance is created.
    */
   hooks?: Hooks;
+
+  /**
+   * The maximum length of the `Location` header in redirect responses.
+   *
+   * When a redirect location is longer, a 400 (Bad Request) response is returned instead.
+   * If not provided, the length is not limited, such as when prerendering.
+   */
+  maxRedirectLocationLength?: number;
 }
 
 /**
@@ -167,7 +176,7 @@ export class AngularServerApp {
 
     if (redirectTo !== undefined) {
       const location = joinUrlParts(
-        request.headers.get('X-Forwarded-Prefix') ?? '',
+        getFirstHeaderValue(request.headers.get('X-Forwarded-Prefix')) ?? '',
         buildPathWithParams(redirectTo, url.pathname),
       );
 
@@ -176,6 +185,7 @@ export class AngularServerApp {
         relativeRedirect ? location + url.search : location,
         status,
         headers,
+        this.options.maxRedirectLocationLength,
       );
     }
 
@@ -329,7 +339,12 @@ export class AngularServerApp {
     }
 
     if (result.redirectTo) {
-      return createRedirectResponse(result.redirectTo, responseInit.status, responseInit.headers);
+      return createRedirectResponse(
+        result.redirectTo,
+        responseInit.status,
+        responseInit.headers,
+        this.options.maxRedirectLocationLength,
+      );
     }
 
     if (renderMode === RenderMode.Prerender) {

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

@@ -25,6 +25,7 @@ import {
 import { ActivatedRoute, Router } from '@angular/router';
 import { Console } from '../console';
 import { addTrailingSlash, joinUrlParts, stripIndexHtmlFromURL, stripTrailingSlash } from './url';
+import { getFirstHeaderValue } from './validation';
 
 /**
  * Represents the bootstrap mechanism for an Angular application.
@@ -126,7 +127,9 @@ export async function renderAngular(
 
       const requestPrefix =
         envInjector.get(APP_BASE_HREF, null, { optional: true }) ??
-        envInjector.get(REQUEST, null, { optional: true })?.headers.get('X-Forwarded-Prefix');
+        getFirstHeaderValue(
+          envInjector.get(REQUEST, null, { optional: true })?.headers.get('X-Forwarded-Prefix'),
+        );
 
       const { pathname, search, hash } = envInjector.get(PlatformLocation);
       const finalUrl = constructSerializedUrl(router, { pathname, search, hash }, requestPrefix);

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

@@ -13,6 +13,17 @@ export const VALID_REDIRECT_RESPONSE_CODES: ReadonlySet<number> = new Set([
   301, 302, 303, 307, 308,
 ]);
 
+/**
+ * The default maximum length of the `Location` header in redirect responses.
+ *
+ * Redirect locations can contain data from the request, such as the path, the query string and
+ * the `X-Forwarded-Prefix` header, which the Angular router can encode into a longer string than
+ * it received. Reverse proxies read the response headers into a buffer of limited size (4 KB by
+ * default in NGINX) and fail the request when they do not fit. The default leaves room in such a
+ * buffer for the status line and the other response headers.
+ */
+export const DEFAULT_MAX_REDIRECT_LOCATION_LENGTH = 2048;
+
 /**
  * Checks if the given HTTP status code is a valid redirect response code.
  *
@@ -30,12 +41,16 @@ export function isValidRedirectResponseCode(code: number): boolean {
  * @param status - The HTTP status code for the redirection. Defaults to 302 (Found).
  *                 See: https://developer.mozilla.org/en-US/docs/Web/API/Response/redirect_static#status
  * @param headers - Additional headers to include in the response.
+ * @param maxLocationLength - The maximum allowed length of the location. When the location is
+ *                            longer, a 400 (Bad Request) response is returned instead. When not
+ *                            provided, the length of the location is not limited.
  * @returns A `Response` object representing the HTTP redirect.
  */
 export function createRedirectResponse(
   location: string,
   status = 302,
   headers?: Record<string, string> | Headers,
+  maxLocationLength?: number,
 ): Response {
   if (ngDevMode && !isValidRedirectResponseCode(status)) {
     throw new Error(
@@ -44,6 +59,24 @@ export function createRedirectResponse(
     );
   }
 
+  if (maxLocationLength !== undefined && location.length > maxLocationLength) {
+    const errorMessage =
+      `Redirect location is ${location.length} characters long, which exceeds ` +
+      `the maximum allowed length of ${maxLocationLength} characters.`;
+
+    // eslint-disable-next-line no-console
+    console.error(
+      `ERROR: Bad Request.\n${errorMessage}\n` +
+        'The limit can be changed with the "maxRedirectLocationLength" option of "AngularAppEngine" or "AngularNodeAppEngine".',
+    );
+
+    return new Response(errorMessage, {
+      status: 400,
+      statusText: 'Bad Request',
+      headers: { 'Content-Type': 'text/plain' },
+    });
+  }
+
   const resHeaders = headers instanceof Headers ? headers : new Headers(headers);
   if (ngDevMode && resHeaders.has('location')) {
     // eslint-disable-next-line no-console

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

@@ -16,6 +16,7 @@ import { destroyAngularServerApp, getOrCreateAngularServerApp } from '../src/app
 import { AngularAppEngine } from '../src/app-engine';
 import { setAngularAppEngineManifest } from '../src/manifest';
 import { RenderMode } from '../src/routes/route-config';
+import { DEFAULT_MAX_REDIRECT_LOCATION_LENGTH } from '../src/utils/redirect';
 import { setAngularAppTestingManifest } from './testing-utils';
 
 @Component({
@@ -202,6 +203,54 @@ describe('AngularAppEngine', () => {
         expect(response?.headers.get('Vary')).toBe('X-Forwarded-Prefix, Accept-Language');
       });
 
+      it('should only use the first value of the forwarded prefix in locale redirect location', async () => {
+        const request = new Request('https://example.com', {
+          headers: {
+            'Accept-Language': 'it',
+            'X-Forwarded-Prefix': '/app, /"<other>',
+          },
+        });
+
+        const response = await appEngine.handle(request);
+        expect(response?.status).toBe(302);
+        expect(response?.headers.get('Location')).toBe('/app/it');
+      });
+
+      it('should return a 400 status when the locale redirect location is too long', async () => {
+        const consoleErrorSpy = spyOn(console, 'error');
+        const request = new Request('https://example.com', {
+          headers: {
+            'Accept-Language': 'it',
+            'X-Forwarded-Prefix': '/' + 'a'.repeat(DEFAULT_MAX_REDIRECT_LOCATION_LENGTH),
+          },
+        });
+
+        const response = await appEngine.handle(request);
+        expect(response?.status).toBe(400);
+        expect(response?.headers.get('Location')).toBeNull();
+        expect(consoleErrorSpy).toHaveBeenCalledWith(
+          jasmine.stringMatching('exceeds the maximum allowed length of 2048 characters'),
+        );
+      });
+
+      it('should allow longer locale redirect locations when "maxRedirectLocationLength" is set', async () => {
+        const prefix = '/' + 'a'.repeat(DEFAULT_MAX_REDIRECT_LOCATION_LENGTH);
+        const engine = new AngularAppEngine({
+          trustProxyHeaders: true,
+          maxRedirectLocationLength: 4096,
+        });
+        const request = new Request('https://example.com', {
+          headers: {
+            'Accept-Language': 'it',
+            'X-Forwarded-Prefix': prefix,
+          },
+        });
+
+        const response = await engine.handle(request);
+        expect(response?.status).toBe(302);
+        expect(response?.headers.get('Location')).toBe(`${prefix}/it`);
+      });
+
       it('should completely ignore proxy headers if not allowed', async () => {
         const strictEngine = new AngularAppEngine({ trustProxyHeaders: false });
         const request = new Request('https://example.com', {
@@ -299,7 +348,10 @@ describe('AngularAppEngine', () => {
         entryPoints: {
           '': async () => {
             setAngularAppTestingManifest(
-              [{ path: 'home', component: HomeComponent }],
+              [
+                { path: 'home', component: HomeComponent },
+                { path: 'redirect', redirectTo: 'home' },
+              ],
               [{ path: '**', renderMode: RenderMode.Server }],
             );
 
@@ -339,6 +391,39 @@ describe('AngularAppEngine', () => {
       const response = await appEngine.handle(request);
       expect(await response?.text()).toContain('Home works');
     });
+
+    it('should return a 400 status when the redirect location exceeds the default maximum length', async () => {
+      const consoleErrorSpy = spyOn(console, 'error');
+      const engine = new AngularAppEngine();
+      const request = new Request(
+        `https://example.com/redirect?q=${'a'.repeat(DEFAULT_MAX_REDIRECT_LOCATION_LENGTH)}`,
+      );
+
+      const response = await engine.handle(request);
+      expect(response?.status).toBe(400);
+      expect(response?.headers.get('Location')).toBeNull();
+      expect(consoleErrorSpy).toHaveBeenCalledWith(
+        jasmine.stringMatching('exceeds the maximum allowed length of 2048 characters'),
+      );
+    });
+
+    it('should allow longer redirect locations when "maxRedirectLocationLength" is set', async () => {
+      const query = 'a'.repeat(DEFAULT_MAX_REDIRECT_LOCATION_LENGTH);
+      const engine = new AngularAppEngine({ maxRedirectLocationLength: 4096 });
+      const request = new Request(`https://example.com/redirect?q=${query}`);
+
+      const response = await engine.handle(request);
+      expect(response?.status).toBe(302);
+      expect(response?.headers.get('Location')).toBe(`/home?q=${query}`);
+    });
+
+    it('should throw an error when "maxRedirectLocationLength" is not a positive integer', () => {
+      for (const maxRedirectLocationLength of [0, -1, 1.5, NaN, Infinity]) {
+        expect(() => new AngularAppEngine({ maxRedirectLocationLength }))
+          .withContext(`maxRedirectLocationLength: ${maxRedirectLocationLength}`)
+          .toThrowError(/"maxRedirectLocationLength" must be a positive integer/);
+      }
+    });
   });
 
   describe('Invalid host headers', () => {