Skip to content

Implemented RFC 7239 - "Forwarded HTTP Extension"#94

Merged
dg merged 8 commits into
nette:masterfrom
patrickkusebauch:master
Jun 17, 2016
Merged

Implemented RFC 7239 - "Forwarded HTTP Extension"#94
dg merged 8 commits into
nette:masterfrom
patrickkusebauch:master

Conversation

@patrickkusebauch

@patrickkusebauch patrickkusebauch commented May 21, 2016

Copy link
Copy Markdown
Contributor

RequestFactory can now take "RemoteAddress", "RemoteHost", "port" and "scheme" from the proxy "FORWARDED" header.

Comment thread src/Http/RequestFactory.php Outdated
$url->setPort((int) $_SERVER['HTTP_X_FORWARDED_PORT']);
}
if (isset($proxyParams['for'])) {
$remoteAddr = explode(':', $proxyParams['for'][0])[0];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This will not work with IPv6.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

IPv6 support in commit 11461e6

Will now work with IPv6.

Added tests for port and scheme of the URL
 - Split test into "x-forwarded" and "forwarded" files for proxy
 - Added tests for default scheme.
 - Added tests for every combination of IPv4/IPv6 with and without port for both "host" and "for" headers
@JanTvrdik

Copy link
Copy Markdown
Contributor

possible related #92

foreach ($forwardParams as $forwardParam) {
$param = explode("=", $forwardParam);
$proxyParams[strtolower(trim($param[0]))][] = trim($param[1], "\"\t\n\r\0\x0B"); //e.g. array['for'][0] = 192.168.0.1
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This can be simplified to

foreach ($forwardParams as $forwardParam) {
    list($key, $value) = explode('=', $forwardParam, 2) + [1 => NULL];
    $proxyParams[strtolower(trim($key))] = trim($value, " \t\"");
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Integrated in commit a2c7a79

@patrickkusebauch

Copy link
Copy Markdown
Contributor Author

I know that it keeps failing on php5.6 with composer update --no-interaction --prefer-dist --prefer-lowest --prefer-stable. However this cannot be redeemed as the tests have to change to keep up with the versions. Furthermore they are test unrelated to this pull request.

@patrickkusebauch

Copy link
Copy Markdown
Contributor Author

Any other problems to be fixed with this PR?
Missing tests?
Or can I safely squash it to be merged?

@dg

dg commented Jun 17, 2016

Copy link
Copy Markdown
Member

Is it ready for merge?

@patrickkusebauch

Copy link
Copy Markdown
Contributor Author

Since there were no comments to the PR for more than 3 weeks, I would assume nobody has any more problems with it.
So yes, it is ready for merge.

@dg dg merged commit e819eff into nette:master Jun 17, 2016
@dg

dg commented Jun 17, 2016

Copy link
Copy Markdown
Member

So thanks!

@patrickkusebauch

Copy link
Copy Markdown
Contributor Author

I thank you for letting me be a part of the development of Nette. :)

dg pushed a commit that referenced this pull request Jun 17, 2016
* Implemented RFC 7239 - " Forwarded HTTP Extension" handling in RequestFactory

* Implemented RFC 7239 - " Forwarded HTTP Extension" handling in RequestFactory

* Deleted echo statement

* case- insensitive handling of  tokens

* Proper handle of quoted strings.
Will now work with IPv6.

Added tests for port and scheme of the URL [Closes #94]

* Tests refactoring:

 - Split test into "x-forwarded" and "forwarded" files for proxy
 - Added tests for default scheme.
 - Added tests for every combination of IPv4/IPv6 with and without port for both "host" and "for" headers

* Code simplifications

* Fixed coding standards for tests
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.

4 participants