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

ensure absolute path after strip to maintain rfc complaince #645

Merged
merged 2 commits into from
Apr 23, 2019

Conversation

aaronhurt
Copy link
Member

It's easy to inadvertently remove the leading slash from the request path when stripping which results in sending a malformed GET with a relative path to the backend server.

Example below ...

Origin URL:

http://foo.com/apples/oranges/steve.html

Route:

route add strippy /apples/ http://1.2.3.4:80/ opts "strip=/apples/"

Results in sending GET oranges/steve.html to the 1.2.3.4 backend. Proper GET request should always be an absolute path or absolute URI according to the RFC (https://tools.ietf.org/html/rfc2616#section-5.1.2).

@aaronhurt
Copy link
Member Author

@nathanejohnson This should resolve your issue.

@aaronhurt aaronhurt force-pushed the ensure-abs-path-after-strip branch from 93fd858 to cb89bf9 Compare April 18, 2019 23:17
Copy link
Member

@pschultz pschultz left a comment

Choose a reason for hiding this comment

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

RFC2616 has been superseeded by 7230-7235 and the comment should reference https://tools.ietf.org/html/rfc7230#section-5.3.

Otherwise looks good to me.

@aaronhurt
Copy link
Member Author

Got it, I'll update the comment. Thanks.

@aaronhurt aaronhurt merged commit dad14b9 into fabiolb:master Apr 23, 2019
@aaronhurt aaronhurt deleted the ensure-abs-path-after-strip branch April 23, 2019 14:46
@magiconair magiconair added this to the 1.5.12 milestone Oct 11, 2019
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.

3 participants