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

Added option to disable path normalization #649

Merged
merged 1 commit into from
Sep 18, 2019
Merged

Added option to disable path normalization #649

merged 1 commit into from
Sep 18, 2019

Conversation

dfirsht
Copy link
Contributor

@dfirsht dfirsht commented Sep 9, 2019

No description provided.

@erikdubbelboer
Copy link
Collaborator

Can you explain why you need this/why this is useful? Also can you add some tests that show this works?

@dfirsht
Copy link
Contributor Author

dfirsht commented Sep 10, 2019

Added a test to show path normalization being disabled.

This is helpful for proxy servers that wish to preserve the original path when forwarding requests. With this option, the proxy server will not make any assumptions on how the destination server expects paths to be formatted and instead, can let that server do its own normalization.

erikdubbelboer
erikdubbelboer requested changes Sep 13, 2019
View reviewed changes
http.go Outdated Show resolved Hide resolved
uri.go Outdated Show resolved Hide resolved
erikdubbelboer
erikdubbelboer requested changes Sep 17, 2019
View reviewed changes
http.go Outdated Show resolved Hide resolved
@dfirsht
Copy link
Contributor Author

dfirsht commented Sep 17, 2019

To more closely align with options like DisableHeaderNamesNormalizing, I made the DisablePathNormalizing bool an exported member. If you prefer though, I can add back the method instead.

kirillDanshin
kirillDanshin approved these changes Sep 18, 2019
View reviewed changes
Copy link
Collaborator

@kirillDanshin kirillDanshin left a comment

Choose a reason for hiding this comment

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

LGTM

@erikdubbelboer erikdubbelboer merged commit ee84500 into valyala:master Sep 18, 2019
@erikdubbelboer
Copy link
Collaborator

Thanks!

@froodian froodian mentioned this pull request Oct 10, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erikdubbelboer erikdubbelboer erikdubbelboer requested changes

@kirillDanshin kirillDanshin kirillDanshin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dfirsht @erikdubbelboer @kirillDanshin