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

URL normalization #2956

Closed
Lookyan opened this issue Mar 31, 2018 · 3 comments · Fixed by #3160
Closed

URL normalization #2956

Lookyan opened this issue Mar 31, 2018 · 3 comments · Fixed by #3160
Labels
beginner Good starter issues! enhancement Feature requests. Not bugs or questions. help wanted Needs help!

Comments

@Lookyan
Copy link

Lookyan commented Mar 31, 2018

URL normalization

Description:
Can't find an URL normalization option (https://en.wikipedia.org/wiki/URL_normalization).

Now we have an option prefix rewrite for vhosts. For example we can replace /service-hello with / and proxy to root address of service. But in case of /service-hello -> / replacement if we request to this address: http://envoy:8888/service-hello/_healthz then we will get a request to http://helloservice//_healthz
It would be nice if we had this feature of URL normalization if we don't have one.
@mattklein123 mattklein123 added question Questions that are neither investigations, bugs, nor enhancements enhancement Feature requests. Not bugs or questions. help wanted Needs help! beginner Good starter issues! and removed question Questions that are neither investigations, bugs, nor enhancements labels Apr 3, 2018
@dio
Copy link
Member

dio commented Apr 20, 2018

But in case of /service-hello -> / replacement if we request to this address: http://envoy:8888/service-hello/_healthz then we will get a request to http://helloservice//_healthz

@Lookyan could you elaborate more on this? Thank you!

@Lookyan
Copy link
Author

Lookyan commented Apr 21, 2018

@dio at least I want to have a feature of multiple slash replacement with one slash during proxying to upstream.
So, we can have URL like this: http://smth//a/b///c
And we should make a request to this URL in this case: http://smth/a/b/c

@dio
Copy link
Member

dio commented Apr 21, 2018 •
edited
Loading

@Lookyan got it. Sorry I didn't see that additional slashes when I read this issue previously.

@dio dio mentioned this issue Apr 22, 2018
Merged
zuercher pushed a commit that referenced this issue Apr 30, 2018
@dio @zuercher
docs: clarify the use of "/" as a prefix_rewrite (#3160)
d20c12d 
docs: clarify the use of "/" as a prefix_rewrite

This patch clarifies the use of "/" as a prefix_rewrite in route and redirect prefix rewriting. And also a note on the use of trailing slashes as match value.

Risk Level: Low
Testing: add more input samples to RedirectPrefixRewrite test.
Docs Changes: Update route.proto doc regarding path_rewrite both for redirect and route.
Release Notes: N/A

Fixes #2956

Signed-off-by: Dhi Aurrahman <dio@rockybars.com>
ramaraochavali pushed a commit to ramaraochavali/envoy that referenced this issue May 3, 2018
@dio @ramaraochavali
docs: clarify the use of "/" as a prefix_rewrite (envoyproxy#3160)
8aee121 
docs: clarify the use of "/" as a prefix_rewrite

This patch clarifies the use of "/" as a prefix_rewrite in route and redirect prefix rewriting. And also a note on the use of trailing slashes as match value.

Risk Level: Low
Testing: add more input samples to RedirectPrefixRewrite test.
Docs Changes: Update route.proto doc regarding path_rewrite both for redirect and route.
Release Notes: N/A

Fixes envoyproxy#2956

Signed-off-by: Dhi Aurrahman <dio@rockybars.com>

Signed-off-by: Rama <rama.rao@salesforce.com>
@PiotrSikora PiotrSikora mentioned this issue Apr 19, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
beginner Good starter issues! enhancement Feature requests. Not bugs or questions. help wanted Needs help!
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

docs: clarify the use of "/" as a prefix_rewrite
3 participants
@dio @Lookyan @mattklein123