Skip to content
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

Jump to bottom

All X-Forwarded-* transforms should replace rather than append values, strip values if x-forwarded is disabled. #1070

Closed
samsp-msft opened this issue Jun 9, 2021 · 2 comments · Fixed by #1099 or #1234
Closed

All X-Forwarded-* transforms should replace rather than append values, strip values if x-forwarded is disabled. #1070

samsp-msft opened this issue Jun 9, 2021 · 2 comments · Fixed by #1099 or #1234
Assignees
@alnikola
Labels
Type: Feedback This issue is general feedback and doesn't represent actionable work.
Milestone
YARP 1.0.0

Comments

@samsp-msft
Copy link
Member

Default action needs to change.
@samsp-msft samsp-msft added the Type: Feedback This issue is general feedback and doesn't represent actionable work. label Jun 9, 2021
@samsp-msft samsp-msft changed the title X:Forwarded for should replace rather than append values X:Forwarded for should replace rather than append values, strip values if x-forwarded is disabled. Jun 9, 2021
@samsp-msft samsp-msft changed the title X:Forwarded for should replace rather than append values, strip values if x-forwarded is disabled. x-Forwarded-for should replace rather than append values, strip values if x-forwarded is disabled. Jun 9, 2021
@karelz karelz added this to the YARP 1.0.0 milestone Jun 10, 2021
@alnikola alnikola self-assigned this Jun 15, 2021
@alnikola alnikola changed the title x-Forwarded-for should replace rather than append values, strip values if x-forwarded is disabled. All X-Forwarded-* transforms should replace rather than append values, strip values if x-forwarded is disabled. Jun 15, 2021
@alnikola alnikola mentioned this issue Jun 18, 2021
Merged
alnikola added a commit that referenced this issue Jun 23, 2021
@alnikola
Default X-Forwarded-* transform action is to set the header value (#1099
9202998 
)

Enables to specify the action for each of X-Forwarded-* transforms and Forwarded one in the configuration. Additionally, sets the default action for these transforms to "set or replace the header".

Fixes #1070
@Tratcher
Copy link
Member

Re-opening to track an remaining question: Should Forwarded be removed when X-Forwarded is used and vice versa?

@Tratcher Tratcher reopened this Jun 23, 2021
@Tratcher
Copy link
Member

Tratcher commented Jun 24, 2021 •
edited
Loading

Design notes:

In the default scenario we should add X-Forwarded-* headers (Set/Replace) like we do today, but also Remove the Forwarded header.

If someone manually configures X-Forwarded, we should still Remove the Forwarded header.

If someone manually configures Forwarded, we should Remove the X-Forwarded headers.

If someone in the future wants both then we'll need more input on how they expect them to interact. One future mitigation might be adding a flag to each of these transforms like RemoveAlternatives that defaults to true (the behavior described above), but would allow them to opt-out.

@alnikola alnikola mentioned this issue Sep 8, 2021
Merged
alnikola added a commit that referenced this issue Sep 9, 2021
@alnikola
Remove Forwarded header when X-Forwarded transforms added and vice ve…
52b3110 
…rsa (#1234)

If X-Forwarded-* transforms are enabled by any way (default or manually), a transform removing Forwarded header is added automatically. On the opposite, if Forwarded transform is added, the transforms removing all X-Forwarded-* headers are added automatically.

Fixes #1070
@adityamandaleeka adityamandaleeka mentioned this issue Feb 6, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alnikola alnikola

Labels
Type: Feedback This issue is general feedback and doesn't represent actionable work.
Projects
None yet
Milestone
YARP 1.0.0
4 participants
@Tratcher @karelz @samsp-msft @alnikola