Skip to content

Commit

Permalink
fix(@angular/ssr): ensure correct Location header for redirects beh…
Browse files Browse the repository at this point in the history
…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 angular#29151
  • Loading branch information
alan-agius4 committed Dec 17, 2024
1 parent f7c0a83 commit 9386765
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 14 deletions.
32 changes: 27 additions & 5 deletions packages/angular/ssr/node/src/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
11 changes: 6 additions & 5 deletions packages/angular/ssr/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
8 changes: 4 additions & 4 deletions packages/angular/ssr/test/app_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});

Expand Down

0 comments on commit 9386765

Please sign in to comment.