fix(@angular/ssr): ensure correct Location header for redirects beh…

…ind a proxy Previously, when the application was served behind a proxy, server-side redirects generated an incorrect Location header, causing navigation issues. This fix updates `createRequestUrl` to use the port from the Host header, ensuring accurate in proxy environments. Additionally, the Location header now only contains the pathname, improving compliance with redirect handling in such setups. Closes #29151 (cherry picked from commit ad1d7d7)
angular · Dec 17, 2024 · 0789a9e · 0789a9e
1 parent 5ebeb37
commit 0789a9e
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 14 deletions.
diff --git a/packages/angular/ssr/node/src/request.ts b/packages/angular/ssr/node/src/request.ts
@@ -83,18 +83,40 @@ function createRequestUrl(nodeRequest: IncomingMessage | Http2ServerRequest): UR
     originalUrl,
   } = nodeRequest as IncomingMessage & { originalUrl?: string };
   const protocol =
-    headers['x-forwarded-proto'] ?? ('encrypted' in socket && socket.encrypted ? 'https' : 'http');
-  const hostname = headers['x-forwarded-host'] ?? headers.host ?? headers[':authority'];
-  const port = headers['x-forwarded-port'] ?? socket.localPort;
+    getFirstHeaderValue(headers['x-forwarded-proto']) ??
+    ('encrypted' in socket && socket.encrypted ? 'https' : 'http');
+  const hostname =
+    getFirstHeaderValue(headers['x-forwarded-host']) ?? headers.host ?? headers[':authority'];
 
   if (Array.isArray(hostname)) {
     throw new Error('host value cannot be an array.');
   }
 
   let hostnameWithPort = hostname;
-  if (port && !hostname?.includes(':')) {
-    hostnameWithPort += `:${port}`;
+  if (!hostname?.includes(':')) {
+    const port = getFirstHeaderValue(headers['x-forwarded-port']);
+    if (port) {
+      hostnameWithPort += `:${port}`;
+    }
   }
 
   return new URL(originalUrl ?? url, `${protocol}://${hostnameWithPort}`);
 }
+
+/**
+ * Extracts the first value from a multi-value header string.
+ *
+ * @param value - A string or an array of strings representing the header values.
+ *                           If it's a string, values are expected to be comma-separated.
+ * @returns The first trimmed value from the multi-value header, or `undefined` if the input is invalid or empty.
+ *
+ * @example
+ * ```typescript
+ * getFirstHeaderValue("value1, value2, value3"); // "value1"
+ * getFirstHeaderValue(["value1", "value2"]); // "value1"
+ * getFirstHeaderValue(undefined); // undefined
+ * ```
+ */
+function getFirstHeaderValue(value: string | string[] | undefined): string | undefined {
+  return value?.toString().split(',', 1)[0]?.trim();
+}
diff --git a/packages/angular/ssr/src/app.ts b/packages/angular/ssr/src/app.ts
@@ -161,14 +161,15 @@ export class AngularServerApp {
 
     const { redirectTo, status, renderMode } = matchedRoute;
     if (redirectTo !== undefined) {
-      return Response.redirect(
-        new URL(buildPathWithParams(redirectTo, url.pathname), url),
+      return new Response(null, {
         // Note: The status code is validated during route extraction.
         // 302 Found is used by default for redirections
         // See: https://developer.mozilla.org/en-US/docs/Web/API/Response/redirect_static#status
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        (status as any) ?? 302,
-      );
+        status: status ?? 302,
+        headers: {
+          'Location': buildPathWithParams(redirectTo, url.pathname),
+        },
+      });
     }
 
     if (renderMode === RenderMode.Prerender) {

diff --git a/packages/angular/ssr/test/app_spec.ts b/packages/angular/ssr/test/app_spec.ts
@@ -106,25 +106,25 @@ describe('AngularServerApp', () => {
 
       it('should correctly handle top level redirects', async () => {
         const response = await app.handle(new Request('http://localhost/redirect'));
-        expect(response?.headers.get('location')).toContain('http://localhost/home');
+        expect(response?.headers.get('location')).toContain('/home');
         expect(response?.status).toBe(302);
       });
 
       it('should correctly handle relative nested redirects', async () => {
         const response = await app.handle(new Request('http://localhost/redirect/relative'));
-        expect(response?.headers.get('location')).toContain('http://localhost/redirect/home');
+        expect(response?.headers.get('location')).toContain('/redirect/home');
         expect(response?.status).toBe(302);
       });
 
       it('should correctly handle relative nested redirects with parameter', async () => {
         const response = await app.handle(new Request('http://localhost/redirect/param/relative'));
-        expect(response?.headers.get('location')).toContain('http://localhost/redirect/param/home');
+        expect(response?.headers.get('location')).toContain('/redirect/param/home');
         expect(response?.status).toBe(302);
       });
 
       it('should correctly handle absolute nested redirects', async () => {
         const response = await app.handle(new Request('http://localhost/redirect/absolute'));
-        expect(response?.headers.get('location')).toContain('http://localhost/home');
+        expect(response?.headers.get('location')).toContain('/home');
         expect(response?.status).toBe(302);
       });