fix(@angular/ssr): improve header validation logic

Updates the `validateRequest` function to correctly handle the `allowedHost=true` option.

Author: alan-agius4

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

@@ -14,6 +14,7 @@ export class AngularAppEngine {
     constructor(options?: AngularAppEngineOptions);
     handle(request: Request, requestContext?: unknown): Promise<Response | null>;
     static ɵallowStaticRouteRender: boolean;
+    static ɵdisableAllowedHostsCheck: boolean;
     static ɵhooks: Hooks;
 }
 

packages/angular/build/src/tools/vite/middlewares/ssr-middleware.ts

@@ -90,6 +90,10 @@ export async function createAngularSsrExternalMiddleware(
     '@angular/ssr/node' as string
   )) as typeof import('@angular/ssr/node', { with: { 'resolution-mode': 'import' } });
 
+  // Disable host check if allowed hosts is true meaning allow all hosts.
+  const { allowedHosts } = server.config.server;
+  const disableAllowedHostsCheck = allowedHosts === true;
+
   return function angularSsrExternalMiddleware(
     req: Connect.IncomingMessage,
     res: ServerResponse,
@@ -123,6 +127,7 @@ export async function createAngularSsrExternalMiddleware(
       }
 
       if (cachedAngularAppEngine !== AngularAppEngine) {
+        AngularAppEngine.ɵdisableAllowedHostsCheck = disableAllowedHostsCheck;
         AngularAppEngine.ɵallowStaticRouteRender = true;
         AngularAppEngine.ɵhooks.on('html:transform:pre', async ({ html, url }) => {
           const processedHtml = await server.transformIndexHtml(url.pathname, html);

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

@@ -42,6 +42,15 @@ export class AngularAppEngine {
    */
   static ɵallowStaticRouteRender = false;
 
+  /**
+   * A flag to enable or disable the allowed hosts check.
+   *
+   * Typically used during development to avoid the allowed hosts check.
+   *
+   * @private
+   */
+  static ɵdisableAllowedHostsCheck = false;
+
   /**
    * Hooks for extending or modifying the behavior of the server application.
    * These hooks are used by the Angular CLI when running the development server and
@@ -106,25 +115,33 @@ export class AngularAppEngine {
    */
   async handle(request: Request, requestContext?: unknown): Promise<Response | null> {
     const allowedHost = this.allowedHosts;
+    const disableAllowedHostsCheck = AngularAppEngine.ɵdisableAllowedHostsCheck;
 
     try {
-      validateRequest(request, allowedHost);
+      validateRequest(request, allowedHost, disableAllowedHostsCheck);
     } catch (error) {
       return this.handleValidationError(request.url, error as Error);
     }
 
     // Clone request with patched headers to prevent unallowed host header access.
-    const { request: securedRequest, onError: onHeaderValidationError } =
-      cloneRequestAndPatchHeaders(request, allowedHost);
+    const { request: securedRequest, onError: onHeaderValidationError } = disableAllowedHostsCheck
+      ? { request, onError: null }
+      : cloneRequestAndPatchHeaders(request, allowedHost);
 
     const serverApp = await this.getAngularServerAppForRequest(securedRequest);
     if (serverApp) {
-      return Promise.race([
-        onHeaderValidationError.then((error) =>
-          this.handleValidationError(securedRequest.url, error),
-        ),
-        serverApp.handle(securedRequest, requestContext),
-      ]);
+      const promises: Promise<Response | null>[] = [];
+      if (onHeaderValidationError) {
+        promises.push(
+          onHeaderValidationError.then((error) =>
+            this.handleValidationError(securedRequest.url, error),
+          ),
+        );
+      }
+
+      promises.push(serverApp.handle(securedRequest, requestContext));
+
+      return Promise.race(promises);
     }
 
     if (this.supportedLocales.length > 1) {

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

@@ -56,11 +56,19 @@ export function getFirstHeaderValue(
  *
  * @param request - The incoming `Request` object to validate.
  * @param allowedHosts - A set of allowed hostnames.
+ * @param disableHostCheck - Whether to disable the host check.
  * @throws Error if any of the validated headers contain invalid values.
  */
-export function validateRequest(request: Request, allowedHosts: ReadonlySet<string>): void {
+export function validateRequest(
+  request: Request,
+  allowedHosts: ReadonlySet<string>,
+  disableHostCheck: boolean,
+): void {
   validateHeaders(request);
-  validateUrl(new URL(request.url), allowedHosts);
+
+  if (!disableHostCheck) {
+    validateUrl(new URL(request.url), allowedHosts);
+  }
 }
 
 /**

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

@@ -373,4 +373,61 @@ describe('AngularAppEngine', () => {
       );
     });
   });
+
+  describe('Disable host check', () => {
+    let consoleErrorSpy: jasmine.Spy;
+
+    beforeAll(() => {
+      setAngularAppEngineManifest({
+        allowedHosts: ['example.com'],
+        entryPoints: {
+          '': async () => {
+            setAngularAppTestingManifest(
+              [{ path: 'home', component: TestHomeComponent }],
+              [{ path: '**', renderMode: RenderMode.Server }],
+            );
+
+            return {
+              ɵgetOrCreateAngularServerApp: getOrCreateAngularServerApp,
+              ɵdestroyAngularServerApp: destroyAngularServerApp,
+            };
+          },
+        },
+        basePath: '/',
+        supportedLocales: { 'en-US': '' },
+      });
+
+      appEngine = new AngularAppEngine();
+
+      AngularAppEngine.ɵdisableAllowedHostsCheck = true;
+    });
+
+    afterAll(() => {
+      AngularAppEngine.ɵdisableAllowedHostsCheck = false;
+    });
+
+    beforeEach(() => {
+      consoleErrorSpy = spyOn(console, 'error');
+    });
+
+    it('should allow requests to disallowed hosts', async () => {
+      const request = new Request('https://evil.com/home');
+      const response = await appEngine.handle(request);
+      expect(response).toBeDefined();
+      expect(response?.status).toBe(200);
+      expect(await response?.text()).toContain('Home works');
+      expect(consoleErrorSpy).not.toHaveBeenCalled();
+    });
+
+    it('should allow requests with disallowed host header', async () => {
+      const request = new Request('https://example.com/home', {
+        headers: { 'host': 'evil.com' },
+      });
+      const response = await appEngine.handle(request);
+      expect(response).toBeDefined();
+      expect(response?.status).toBe(200);
+      expect(await response?.text()).toContain('Home works');
+      expect(consoleErrorSpy).not.toHaveBeenCalled();
+    });
+  });
 });

packages/angular/ssr/test/utils/validation_spec.ts

@@ -77,13 +77,19 @@ describe('Validation Utils', () => {
         },
       });
 
-      expect(() => validateRequest(req, allowedHosts)).not.toThrow();
+      expect(() => validateRequest(req, allowedHosts, false)).not.toThrow();
+    });
+
+    it('should pass for valid request when disableHostCheck is true', () => {
+      const req = new Request('http://evil.com');
+
+      expect(() => validateRequest(req, allowedHosts, true)).not.toThrow();
     });
 
     it('should throw if URL hostname is invalid', () => {
       const req = new Request('http://evil.com');
 
-      expect(() => validateRequest(req, allowedHosts)).toThrowError(
+      expect(() => validateRequest(req, allowedHosts, false)).toThrowError(
         /URL with hostname "evil.com" is not allowed/,
       );
     });
@@ -93,7 +99,7 @@ describe('Validation Utils', () => {
         headers: { 'x-forwarded-port': 'abc' },
       });
 
-      expect(() => validateRequest(req, allowedHosts)).toThrowError(
+      expect(() => validateRequest(req, allowedHosts, false)).toThrowError(
         'Header "x-forwarded-port" must be a numeric value.',
       );
     });
@@ -102,16 +108,32 @@ describe('Validation Utils', () => {
       const req = new Request('http://example.com', {
         headers: { 'x-forwarded-proto': 'ftp' },
       });
-      expect(() => validateRequest(req, allowedHosts)).toThrowError(
+      expect(() => validateRequest(req, allowedHosts, false)).toThrowError(
         'Header "x-forwarded-proto" must be either "http" or "https".',
       );
     });
 
+    it('should pass for valid x-forwarded-proto (case-insensitive)', () => {
+      const req = new Request('http://example.com', {
+        headers: { 'x-forwarded-proto': 'HTTP' },
+      });
+      expect(() => validateRequest(req, allowedHosts, false)).not.toThrow();
+    });
+
     it('should throw if host contains path separators', () => {
       const req = new Request('http://example.com', {
         headers: { 'host': 'example.com/bad' },
       });
-      expect(() => validateRequest(req, allowedHosts)).toThrowError(
+      expect(() => validateRequest(req, allowedHosts, false)).toThrowError(
+        'Header "host" contains characters that are not allowed.',
+      );
+    });
+
+    it('should throw if host contains invalid characters', () => {
+      const req = new Request('http://example.com', {
+        headers: { 'host': 'example.com?query=1' },
+      });
+      expect(() => validateRequest(req, allowedHosts, false)).toThrowError(
         'Header "host" contains characters that are not allowed.',
       );
     });
@@ -120,7 +142,7 @@ describe('Validation Utils', () => {
       const req = new Request('http://example.com', {
         headers: { 'x-forwarded-host': 'example.com/bad' },
       });
-      expect(() => validateRequest(req, allowedHosts)).toThrowError(
+      expect(() => validateRequest(req, allowedHosts, false)).toThrowError(
         'Header "x-forwarded-host" contains characters that are not allowed.',
       );
     });
@@ -135,7 +157,7 @@ describe('Validation Utils', () => {
           },
         });
 
-        expect(() => validateRequest(request, allowedHosts))
+        expect(() => validateRequest(request, allowedHosts, false))
           .withContext(`Prefix: "${prefix}"`)
           .toThrowError(
             'Header "x-forwarded-prefix" must not start with multiple "/" or "\\" or contain ".", ".." path segments.',
@@ -168,7 +190,7 @@ describe('Validation Utils', () => {
           },
         });
 
-        expect(() => validateRequest(request, allowedHosts))
+        expect(() => validateRequest(request, allowedHosts, false))
           .withContext(`Prefix: "${prefix}"`)
           .toThrowError(
             'Header "x-forwarded-prefix" must not start with multiple "/" or "\\" or contain ".", ".." path segments.',
@@ -186,7 +208,7 @@ describe('Validation Utils', () => {
           },
         });
 
-        expect(() => validateRequest(request, allowedHosts))
+        expect(() => validateRequest(request, allowedHosts, false))
           .withContext(`Prefix: "${prefix}"`)
           .not.toThrow();
       }