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.

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

Quarkus proxy ForwardedParser doesn't support a defacto forwarded header fallback #25077

Closed
luneo7 opened this issue Apr 21, 2022 · 4 comments · Fixed by #25117
Closed

Quarkus proxy ForwardedParser doesn't support a defacto forwarded header fallback #25077

luneo7 opened this issue Apr 21, 2022 · 4 comments · Fixed by #25117
Labels
area/vertx kind/bug Something isn't working
Milestone

Comments

@luneo7
Copy link
Contributor

luneo7 commented Apr 21, 2022

Describe the bug

According to Quarkus docs https://quarkus.io/guides/http-reference#reverse-proxy we should be able to combine defacto proxy forwarding headers Forwarded with the non defacto X-Fowarded, and the standard headers should have precedence

Both configurations related to standard and non-standard headers can be combined, although the standard headers configuration will have precedence.

But when setting the configs, you either set to use Forwarded or X-Forwarded and there is no way to use them both.

I've caught that when using Keycloak (keycloak/keycloak#11580)

Expected behavior

Check for Forwarded header, if doesn't exists uses X-Forwarded header

Actual behavior

Uses Forwarded header or X-Forwarded header

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

It would be nice to have a way of also adding a custom handler, so we could also build a custom Vert.x handler and add it to Quarkus configuration and it would be picked up during build time.

@luneo7 luneo7 added the kind/bug Something isn't working label Apr 21, 2022
@geoand
Copy link
Contributor

geoand commented Apr 22, 2022

The implementation seems to only consider FORWARDED if quarkus.http.allow-forwarded=true while the various X-FORWARDED-* headers are only considered if ``quarkus.http.allow-forwarded=false`.

I don't know if this is the expected behavior, so I'll leave this to @cescoffier and @stuartwdouglas

@sberyozkin
Copy link
Member

@ejba did a nice work on the original code, @ejba, would you like to look at this issue ?

Given @geoand's analysis, I propose to check both Forwarded-* and X-Forwarded* if both quarkus.http.allow-forwarded=true and a new quarkus.http.allow-x-forwarded=true are set. quarkus.http.allow-x-forwarded should probably have no default value set at the injection time but defaulted to true if quarkus.http.allow-forwarded=false to retain the current behavior...

@ejba
Copy link
Contributor

ejba commented Apr 23, 2022

@sberyozkin it will be a pleasure to be helpful again!

@sberyozkin
Copy link
Member

@ejba Thanks a million :-)

@quarkus-bot quarkus-bot bot added this to the 2.10 - main milestone Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/vertx kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants