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

Correctly handle URL escaped paths #118

Merged
merged 1 commit into from Dec 6, 2021
Merged

Conversation

alek-sys
Copy link
Contributor

@alek-sys alek-sys commented Dec 6, 2021

Consider proxy configuration *,/test/(.*),https://dest/$1.

When reproxy accepts a request with URL encoded path, i.e.
'/test/a%205%25%20b/' which the encoded form of '/test/a 5% b',
it is using request.URL.Path which is already URL decoded by Golang.

This causes an error in proxy.go while it is trying to validate the
destination with url.Parse(match.Destination) as, strictly speaking,
destination URL is not a valid URL anymore, it is https://target-dest/a 5% b.

With this fix, the original escaped URL stays as is, correctly passes
the validation and then it is up to destination server to URL decode and
correctly handle the URL.

Consider proxy configuration `*,/test/(.*),https://dest/$1`.

When reproxy accepts a request with URL encoded path, i.e.
'/test/a%205%25%20b/' which the encoded form of '/test/a 5% b',
it is using request.URL.Path which is already URL decoded by Golang.

This causes an error in proxy.go while it is trying to validate the
destination with `url.Parse(match.Destination)` as, strictly speaking,
destination URL is not a valid URL anymore, it is `https://target-dest/a 5% b`.

With this fix, the original escaped URL stays as is, correctly passes
the validation and then it is up to destination server to URL decode and
correctly handle the URL.
Copy link
Owner

@umputun umputun left a comment

Choose a reason for hiding this comment

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

LGTM

@umputun umputun merged commit 2e8733b into umputun:master Dec 6, 2021
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.

None yet

2 participants