Skip to content

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

Closed
alan-agius4 wants to merge 1 commit intoangular:mainfrom
alan-agius4:vary-by
Closed

refactor: remove X-Forwarded-Prefix from Vary header in SSR redirect utility#33063
alan-agius4 wants to merge 1 commit intoangular:mainfrom
alan-agius4:vary-by

Conversation

@alan-agius4
Copy link
Copy Markdown
Collaborator

This is no longer needed since now X-Forwarded-Prefix is validated by the users.

…utility

This is no longer needed since now `X-Forwarded-Prefix` is validated by the users.
@alan-agius4 alan-agius4 requested a review from dgp1130 April 28, 2026 07:55
@alan-agius4 alan-agius4 added action: review The PR is still awaiting reviews from at least one requested reviewer target: major This PR is targeted for the next major release labels Apr 28, 2026
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request removes the logic that automatically adds the X-Forwarded-Prefix header to the Vary response header during redirects. However, this change introduces a high-severity cache poisoning risk; since the redirect Location depends on the X-Forwarded-Prefix value, omitting it from the Vary header can cause CDNs or reverse proxies to serve incorrect cached redirects to users on different proxy paths.

}

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: review The PR is still awaiting reviews from at least one requested reviewer target: major This PR is targeted for the next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant