Skip to content

refactor: remove X-Forwarded-Prefix from Vary header in SSR redirect utility #33063

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
alan-agius4 wants to merge 1 commit into from
+2 −32
Closed

refactor: remove X-Forwarded-Prefix from Vary header in SSR redirect utility #33063

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions packages/angular/ssr/src/utils/redirect.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,6 @@ export function createRedirectResponse(
);
}

// Ensure unique values for Vary header
const varyArray = resHeaders.get('Vary')?.split(',') ?? [];
const varySet = new Set(['X-Forwarded-Prefix']);
for (const vary of varyArray) {
const value = vary.trim();

if (value) {
varySet.add(value);
}
}

resHeaders.set('Vary', [...varySet].join(', '));
resHeaders.set('Location', location);
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

Removing the logic that automatically adds X-Forwarded-Prefix to the Vary header can lead to cache poisoning. Even if the header is validated, the redirect Location still depends on its value (as seen in AngularAppEngine.redirectBasedOnAcceptLanguage in app-engine.ts).

When a redirect response is cached by a CDN or reverse proxy, it must vary by all headers used to generate the Location URL. If X-Forwarded-Prefix is omitted from Vary, a user accessing the site through a different proxy prefix might receive a cached redirect to the wrong path. If the intention is to make this utility more generic, the responsibility for adding X-Forwarded-Prefix to Vary should be moved to the caller, but currently, AngularAppEngine does not appear to handle this, which introduces a regression in cache safety.


return new Response(null, {
Expand Down
4 changes: 2 additions & 2 deletions packages/angular/ssr/test/app-engine_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ describe('AngularAppEngine', () => {
const response = await appEngine.handle(request);
expect(response?.status).toBe(302);
expect(response?.headers.get('Location')).toBe('/it');
expect(response?.headers.get('Vary')).toBe('X-Forwarded-Prefix, Accept-Language');
expect(response?.headers.get('Vary')).toBe('Accept-Language');
});

it('should include forwarded prefix in locale redirect location when present', async () => {
Expand All @@ -174,7 +174,7 @@ describe('AngularAppEngine', () => {
const response = await appEngine.handle(request);
expect(response?.status).toBe(302);
expect(response?.headers.get('Location')).toBe('/app/it');
expect(response?.headers.get('Vary')).toBe('X-Forwarded-Prefix, Accept-Language');
expect(response?.headers.get('Vary')).toBe('Accept-Language');
});

it('should completely ignore proxy headers if not allowed', async () => {
Expand Down
18 changes: 0 additions & 18 deletions packages/angular/ssr/test/utils/redirect_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ describe('Redirect Utils', () => {
const response = createRedirectResponse('/home');
expect(response.status).toBe(302);
expect(response.headers.get('Location')).toBe('/home');
expect(response.headers.get('Vary')).toBe('X-Forwarded-Prefix');
});

it('should create a redirect response with a custom status', () => {
Expand All @@ -27,23 +26,6 @@ describe('Redirect Utils', () => {
const response = createRedirectResponse('/home', 302, { 'X-Custom': 'value' });
expect(response.headers.get('X-Custom')).toBe('value');
expect(response.headers.get('Location')).toBe('/home');
expect(response.headers.get('Vary')).toBe('X-Forwarded-Prefix');
});

it('should append to Vary header instead of overriding it', () => {
const response = createRedirectResponse('/home', 302, {
'Location': '/evil',
'Vary': 'Host',
});
expect(response.headers.get('Location')).toBe('/home');
expect(response.headers.get('Vary')).toBe('X-Forwarded-Prefix, Host');
});

it('should NOT add duplicate X-Forwarded-Prefix if already present in Vary header', () => {
const response = createRedirectResponse('/home', 302, {
'Vary': 'X-Forwarded-Prefix, Host',
});
expect(response.headers.get('Vary')).toBe('X-Forwarded-Prefix, Host');
});

it('should warn if Location header is provided in extra headers in dev mode', () => {
Expand Down
Loading