fixup! fix(@angular/ssr): introduce trustProxyHeaders option to safely validate and sanitize proxy headers

Author: alan-agius4

packages/angular/ssr/node/src/request.ts

@@ -8,7 +8,11 @@
 
 import type { IncomingHttpHeaders, IncomingMessage } from 'node:http';
 import type { Http2ServerRequest } from 'node:http2';
-import { getFirstHeaderValue } from '../../src/utils/validation';
+import {
+  getFirstHeaderValue,
+  isProxyHeaderAllowed,
+  normalizeTrustProxyHeaders,
+} from '../../src/utils/validation';
 
 /**
  * A set containing all the pseudo-headers defined in the HTTP/2 specification.
@@ -33,7 +37,7 @@ const HTTP2_PSEUDO_HEADERS: ReadonlySet<string> = new Set([
  * be used by web platform APIs.
  *
  * @param nodeRequest - The Node.js request object (`IncomingMessage` or `Http2ServerRequest`) to convert.
- * @param trustProxyHeaders - A boolean or an array of allowed proxy headers.
+ * @param trustProxyHeaders - A boolean or an array of proxy headers to trust when constructing the request URL.
  *
  * @remarks
  * When `trustProxyHeaders` is enabled, headers such as `X-Forwarded-Host` and
@@ -46,18 +50,14 @@ export function createWebRequestFromNodeRequest(
   nodeRequest: IncomingMessage | Http2ServerRequest,
   trustProxyHeaders?: boolean | readonly string[],
 ): Request {
-  const trustProxyHeadersNormalized =
-    trustProxyHeaders && typeof trustProxyHeaders !== 'boolean'
-      ? new Set(trustProxyHeaders.map((h) => h.toLowerCase()))
-      : trustProxyHeaders;
-
+  const trustProxyHeadersNormalized = normalizeTrustProxyHeaders(trustProxyHeaders);
   const { headers, method = 'GET' } = nodeRequest;
   const withBody = method !== 'GET' && method !== 'HEAD';
   const referrer = headers.referer && URL.canParse(headers.referer) ? headers.referer : undefined;
 
   return new Request(createRequestUrl(nodeRequest, trustProxyHeadersNormalized), {
     method,
-    headers: createRequestHeaders(headers, trustProxyHeadersNormalized),
+    headers: createRequestHeaders(headers),
     body: withBody ? nodeRequest : undefined,
     duplex: withBody ? 'half' : undefined,
     referrer,
@@ -68,27 +68,16 @@ export function createWebRequestFromNodeRequest(
  * Creates a `Headers` object from Node.js `IncomingHttpHeaders`.
  *
  * @param nodeHeaders - The Node.js `IncomingHttpHeaders` object to convert.
- * @param trustProxyHeaders - A boolean or a set of allowed proxy headers.
  * @returns A `Headers` object containing the converted headers.
  */
-function createRequestHeaders(
-  nodeHeaders: IncomingHttpHeaders,
-  trustProxyHeaders: boolean | ReadonlySet<string> | undefined,
-): Headers {
+function createRequestHeaders(nodeHeaders: IncomingHttpHeaders): Headers {
   const headers = new Headers();
 
   for (const [name, value] of Object.entries(nodeHeaders)) {
     if (HTTP2_PSEUDO_HEADERS.has(name)) {
       continue;
     }
 
-    if (
-      name.toLowerCase().startsWith('x-forwarded-') &&
-      !isProxyHeaderAllowed(name.toLowerCase(), trustProxyHeaders)
-    ) {
-      continue;
-    }
-
     if (typeof value === 'string') {
       headers.append(name, value);
     } else if (Array.isArray(value)) {
@@ -105,7 +94,7 @@ function createRequestHeaders(
  * Creates a `URL` object from a Node.js `IncomingMessage`, taking into account the protocol, host, and port.
  *
  * @param nodeRequest - The Node.js `IncomingMessage` or `Http2ServerRequest` object to extract URL information from.
- * @param trustProxyHeaders - A boolean or a set of allowed proxy headers.
+ * @param trustProxyHeaders - A set of allowed proxy headers.
  *
  * @remarks
  * When `trustProxyHeaders` is enabled, headers such as `X-Forwarded-Host` and
@@ -116,7 +105,7 @@ function createRequestHeaders(
  */
 export function createRequestUrl(
   nodeRequest: IncomingMessage | Http2ServerRequest,
-  trustProxyHeaders?: boolean | ReadonlySet<string>,
+  trustProxyHeaders: ReadonlySet<string>,
 ): URL {
   const {
     headers,
@@ -154,43 +143,15 @@ export function createRequestUrl(
  *
  * @param headers - The Node.js incoming HTTP headers.
  * @param headerName - The name of the proxy header to retrieve.
- * @param trustProxyHeaders - A boolean or a set of allowed proxy headers.
+ * @param trustProxyHeaders - A set of allowed proxy headers.
  * @returns The value of the allowed proxy header, or `undefined` if not allowed or not present.
  */
 function getAllowedProxyHeaderValue(
   headers: IncomingHttpHeaders,
   headerName: string,
-  trustProxyHeaders: boolean | ReadonlySet<string> | undefined,
+  trustProxyHeaders: ReadonlySet<string>,
 ): string | undefined {
   return isProxyHeaderAllowed(headerName, trustProxyHeaders)
     ? getFirstHeaderValue(headers[headerName])
     : undefined;
 }
-
-/**
- * Checks if a specific proxy header is allowed.
- *
- * @param headerName - The name of the proxy header to check.
- * @param allowedProxyHeaders - A boolean or a set of allowed proxy headers.
- * @returns `true` if the header is allowed, `false` otherwise.
- */
-function isProxyHeaderAllowed(
-  headerName: string,
-  trustProxyHeaders: boolean | ReadonlySet<string> | undefined,
-): boolean {
-  if (trustProxyHeaders === undefined) {
-    const lower = headerName.toLowerCase();
-
-    return lower === 'x-forwarded-host' || lower === 'x-forwarded-proto';
-  }
-
-  if (trustProxyHeaders === false) {
-    return false;
-  }
-
-  if (trustProxyHeaders === true) {
-    return true;
-  }
-
-  return trustProxyHeaders.has(headerName.toLowerCase());
-}

packages/angular/ssr/node/test/request_spec.ts

@@ -7,9 +7,9 @@
  */
 
 import { IncomingMessage } from 'node:http';
-import { Http2ServerRequest } from 'node:http2';
 import { Socket } from 'node:net';
 import { createRequestUrl } from '../src/request';
+import { normalizeTrustProxyHeaders } from '../../src/utils/validation';
 
 // Helper to create a mock request object for testing.
 function createRequest(details: {
@@ -26,25 +26,14 @@ function createRequest(details: {
   } as unknown as IncomingMessage;
 }
 
-// Helper to create a mock Http2ServerRequest object for testing.
-function createHttp2Request(details: {
-  headers: Record<string, string | string[] | undefined>;
-  url?: string;
-}): Http2ServerRequest {
-  return {
-    headers: details.headers,
-    socket: new Socket(),
-    url: details.url,
-  } as Http2ServerRequest;
-}
-
 describe('createRequestUrl', () => {
   it('should create a http URL with hostname and port from the host header', () => {
     const url = createRequestUrl(
       createRequest({
         headers: { host: 'localhost:8080' },
         url: '/test',
       }),
+      new Set(),
     );
     expect(url.href).toBe('http://localhost:8080/test');
   });
@@ -56,6 +45,7 @@ describe('createRequestUrl', () => {
         encryptedSocket: true,
         url: '/test',
       }),
+      new Set(),
     );
     expect(url.href).toBe('https://example.com/test');
   });
@@ -67,6 +57,7 @@ describe('createRequestUrl', () => {
         encryptedSocket: true,
         url: '',
       }),
+      new Set(),
     );
     expect(url.href).toBe('https://example.com/');
   });
@@ -78,6 +69,7 @@ describe('createRequestUrl', () => {
         encryptedSocket: true,
         url: '/test?a=1',
       }),
+      new Set(),
     );
     expect(url.href).toBe('https://example.com/test?a=1');
   });
@@ -90,6 +82,7 @@ describe('createRequestUrl', () => {
         url: '/test',
         originalUrl: '/original',
       }),
+      new Set(),
     );
     expect(url.href).toBe('https://example.com/original');
   });
@@ -102,6 +95,7 @@ describe('createRequestUrl', () => {
         url: undefined,
         originalUrl: undefined,
       }),
+      new Set(),
     );
     expect(url.href).toBe('https://example.com/');
   });
@@ -112,6 +106,7 @@ describe('createRequestUrl', () => {
         headers: { host: 'localhost:8080' },
         url: '//example.com/test',
       }),
+      new Set(),
     );
     expect(url.href).toBe('http://localhost:8080//example.com/test');
   });
@@ -123,6 +118,7 @@ describe('createRequestUrl', () => {
         url: '/test',
         originalUrl: '//example.com/original',
       }),
+      new Set(),
     );
     expect(url.href).toBe('http://localhost:8080//example.com/original');
   });
@@ -137,7 +133,7 @@ describe('createRequestUrl', () => {
         },
         url: '/test',
       }),
-      true,
+      normalizeTrustProxyHeaders(true),
     );
     expect(url.href).toBe('https://example.com/test');
   });
@@ -153,7 +149,7 @@ describe('createRequestUrl', () => {
         },
         url: '/test',
       }),
-      true,
+      normalizeTrustProxyHeaders(true),
     );
     expect(url.href).toBe('https://example.com:8443/test');
   });

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

@@ -12,7 +12,11 @@ import { getPotentialLocaleIdFromUrl, getPreferredLocale } from './i18n';
 import { EntryPointExports, getAngularAppEngineManifest } from './manifest';
 import { createRedirectResponse } from './utils/redirect';
 import { joinUrlParts } from './utils/url';
-import { sanitizeRequestHeaders, validateRequest } from './utils/validation';
+import {
+  normalizeTrustProxyHeaders,
+  sanitizeRequestHeaders,
+  validateRequest,
+} from './utils/validation';
 
 /**
  * Options for the Angular server application engine.
@@ -97,7 +101,7 @@ export class AngularAppEngine {
   /**
    * The normalized allowed proxy headers.
    */
-  private readonly trustProxyHeaders: ReadonlySet<string> | boolean | undefined;
+  private readonly trustProxyHeaders: ReadonlySet<string>;
 
   /**
    * A cache that holds entry points, keyed by their potential locale string.
@@ -110,14 +114,7 @@ export class AngularAppEngine {
    */
   constructor(options?: AngularAppEngineOptions) {
     this.allowedHosts = this.getAllowedHosts(options);
-
-    const trustProxyHeaders = options?.trustProxyHeaders;
-    this.trustProxyHeaders =
-      trustProxyHeaders === undefined
-        ? undefined
-        : typeof trustProxyHeaders === 'boolean'
-          ? trustProxyHeaders
-          : new Set(trustProxyHeaders.map((h) => h.toLowerCase()));
+    this.trustProxyHeaders = normalizeTrustProxyHeaders(options?.trustProxyHeaders);
   }
 
   private getAllowedHosts(options: AngularAppEngineOptions | undefined): ReadonlySet<string> {
@@ -160,7 +157,10 @@ export class AngularAppEngine {
    */
   async handle(request: Request, requestContext?: unknown): Promise<Response | null> {
     const allowedHost = this.allowedHosts;
-    const securedRequest = sanitizeRequestHeaders(request, this.trustProxyHeaders);
+    const { request: securedRequest, deoptToCSR } = sanitizeRequestHeaders(
+      request,
+      this.trustProxyHeaders,
+    );
 
     try {
       validateRequest(securedRequest, allowedHost, AngularAppEngine.ɵdisableAllowedHostsCheck);
@@ -170,6 +170,10 @@ export class AngularAppEngine {
 
     const serverApp = await this.getAngularServerAppForRequest(securedRequest);
     if (serverApp) {
+      if (deoptToCSR) {
+        return serverApp.serveClientSidePage();
+      }
+
       return serverApp.handle(securedRequest, requestContext);
     }
 

packages/angular/ssr/src/app.ts

@@ -191,20 +191,6 @@ export class AngularServerApp {
     }
 
     const { redirectTo, status, renderMode, headers, preload } = matchedRoute;
-
-    if (request.headers.get('x-angular-deopt-csr') === 'true') {
-      let html = await this.assets.getServerAsset('index.csr.html').text();
-      html = await this.runTransformsOnHtml(html, url, preload);
-
-      return new Response(html, {
-        status: 200,
-        headers: new Headers({
-          'Content-Type': 'text/html;charset=UTF-8',
-          ...headers,
-        }),
-      });
-    }
-
     if (redirectTo !== undefined) {
       return createRedirectResponse(
         joinUrlParts(