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

Allow redirect URI path to not be normalized. #1638

Conversation

skidder
Copy link
Contributor

@skidder skidder commented Oct 26, 2023

Fix #1637

Propagates the DisablePathNormalizing setting on a request URI to the redirect URI. This fixes an issue where redirect URIs were being normalized regardless of the setting on the request URI. This could effectively break the redirect URI, depending on the expectations of the server.

Preserves the current default behavior, which is for DisablePathNormalizing to be false.

@kirillDanshin
Copy link
Collaborator

I would consider this approach a breaking change though.
The issue is that users already got used to the existing meaning of this parameter, so I would rather this be done via a new option.
What do you think?
/cc @erikdubbelboer

@skidder
Copy link
Contributor Author

skidder commented Oct 26, 2023

I would consider this approach a breaking change though. The issue is that users already got used to the existing meaning of this parameter, so I would rather this be done via a new option. What do you think? /cc @erikdubbelboer

@kirillDanshin Ah, good point. It sounds like you're proposing we signal to not normalize redirect locations using a new field on the request, is that right?

http.go Outdated Show resolved Hide resolved
@erikdubbelboer erikdubbelboer merged commit 42bd7bb into valyala:master Oct 30, 2023
12 of 14 checks passed
@erikdubbelboer
Copy link
Collaborator

Thanks!

@skidder skidder deleted the fix/propagate-path-normalizing-when-redirected branch October 30, 2023 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Redirect location is always normalized
3 participants