-
-
Notifications
You must be signed in to change notification settings - Fork 148
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
Validate proxy requests in absolute-form #169
Conversation
4808542
to
708b143
Compare
Updated PR to remove unrelated changes so this now has a clear focus on requests in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just a minor nitpick
tests/ServerTest.php
Outdated
$this->assertSame('http://example.com/test', $requestAssertion->getRequestTarget()); | ||
$this->assertEquals('http://example.com/test', $requestAssertion->getUri()); | ||
$this->assertSame('/test', $requestAssertion->getUri()->getPath()); | ||
//$this->assertSame(array(), $requestAssertion->getQueryParams()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason this is commented out? To me commented out code shouldn't be committed tbh
tests/ServerTest.php
Outdated
$this->assertSame('http://example.com:8080/test', $requestAssertion->getRequestTarget()); | ||
$this->assertEquals('http://example.com:8080/test', $requestAssertion->getUri()); | ||
$this->assertSame('/test', $requestAssertion->getUri()->getPath()); | ||
//$this->assertSame(array(), $requestAssertion->getQueryParams()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as line 289
Thanks for spotting! Updated and also rebased now that #170 is in, which did not preserve original request-target |
👍 |
The
absolute-form
request-target
was already supported in earlier version, but was neither tested nor documented. This PR makes sure thatgetUri()
behaves consistent across different forms of request-target and adds matching documentation and an example for a plain HTTP proxy that relies on requests inabsolute-form
.Example request as sent to a plain HTTP proxy:
Builds on top of #167, #158 and #157.