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

Lenient handling of unencoded URIs in ForwardedHeaderTransformer #30137

Closed
wants to merge 1 commit into from

Conversation

joakimmohn
Copy link
Contributor

@joakimmohn joakimmohn commented Mar 18, 2023

Currently ForwardedHeaderTransformer assumes the url is url encoded which is not necessarily the case.

I experienced an issue where the a query parameter as part of the OpenID Connect flow contains a "=" character which fails the verifyUriComponent check in HierarchicalUriComponents.

@pivotal-cla
Copy link

@joakimmohn Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 18, 2023
@pivotal-cla
Copy link

@joakimmohn Thank you for signing the Contributor License Agreement!

Copy link
Contributor

@rstoyanchev rstoyanchev left a comment

Choose a reason for hiding this comment

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

The URI should be encoded, or otherwise it cannot be parsed reliably. I don't think we should have to make such checks, and this wouldn't be the only place that would require it.

I don't where the root cause lies. It could be that OpenID Connect sends it that way, and it should be fixed there. We also made some recent improvements to how the URI is initialized for Reactor Netty in 6.0.6 and 5.3.26. It's worth trying with those versions if you haven't.

@rstoyanchev rstoyanchev added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Mar 27, 2023
@rstoyanchev rstoyanchev self-assigned this Mar 27, 2023
@rstoyanchev rstoyanchev added the status: waiting-for-feedback We need additional information before we can continue label Mar 28, 2023
@rstoyanchev rstoyanchev added type: enhancement A general enhancement and removed status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged or decided on labels Sep 20, 2023
@rstoyanchev rstoyanchev added this to the 6.1.0-RC1 milestone Sep 20, 2023
@rstoyanchev rstoyanchev changed the title Handle unencoded URIs gracefully in ForwardedHeaderTransformer Lenient handling of unencoded URIs in ForwardedHeaderTransformer Sep 20, 2023
@rstoyanchev
Copy link
Contributor

I've solved this in a slightly different way, essentially still avoiding encoding but also not causing a strict check to be performed.

bclozel added a commit that referenced this pull request Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants