Skip to content

Implemented RFC 7239 - "Forwarded HTTP Extension" #94

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

Merged
dg merged 8 commits into from
Jun 17, 2016
Merged

Implemented RFC 7239 - "Forwarded HTTP Extension" #94

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ec85d5d
Implemented RFC 7239 - " Forwarded HTTP Extension" handling in Reques…
patrickkusebauch May 21, 2016
61e9275
Implemented RFC 7239 - " Forwarded HTTP Extension" handling in Reques…
patrickkusebauch May 21, 2016
a6e205c
Deleted echo statement
patrickkusebauch May 21, 2016
44660bd
case- insensitive handling of tokens
patrickkusebauch May 22, 2016
11461e6
Proper handle of quoted strings.
patrickkusebauch May 22, 2016
17d3397
Tests refactoring:
patrickkusebauch May 22, 2016
a2c7a79
Code simplifications
patrickkusebauch May 22, 2016
2383979
Fixed coding standards for tests
patrickkusebauch May 22, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 43 additions & 19 deletions src/Http/RequestFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,30 +194,54 @@ public function createHttpRequest()
$usingTrustedProxy = $remoteAddr && array_filter($this->proxies, function ($proxy) use ($remoteAddr) {
return Helpers::ipMatch($remoteAddr, $proxy);
});

if ($usingTrustedProxy) {
if (!empty($_SERVER['HTTP_X_FORWARDED_PROTO'])) {
$url->setScheme(strcasecmp($_SERVER['HTTP_X_FORWARDED_PROTO'], 'https') === 0 ? 'https' : 'http');
}
if(!empty($_SERVER['HTTP_FORWARDED'])) {
$forwardParams = preg_split('/[,]|[;]/', $_SERVER['HTTP_FORWARDED']);
Copy link
Copy Markdown
Contributor

@Majkl578 Majkl578 May 22, 2016

Choose a reason for hiding this comment

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

Semicolin is used as a forward-pair delimiter whereas comma is used as a field separator. I'm afraid current implementation won't be able to distinguish it...

Copy link
Copy Markdown
Contributor Author

@patrickkusebauch patrickkusebauch May 22, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

It is not. However it should not be a problem, as I believe there is no need for the distinction.
These two headers would produce the same result array:
for=192.0.2.43, for=198.51.100.17
for=192.0.2.43; for=198.51.100.17

I just need to split the "key-value" pairs and put them in the proper sub-array.

Copy link
Copy Markdown
Contributor

@JanTvrdik JanTvrdik May 22, 2016

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 preg_split('#[,;]#')

Copy link
Copy Markdown
Contributor Author

@patrickkusebauch patrickkusebauch May 22, 2016

Choose a reason for hiding this comment

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

Integrated in commit a2c7a79

foreach ($forwardParams as $forwardParam) {
$param = explode("=", $forwardParam);
$proxyParams[trim($param[0])][] = trim($param[1]); //e.g. array['for'][0] = 192.168.0.1
Copy link
Copy Markdown
Contributor

@Majkl578 Majkl578 May 22, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Parameter names must be treated in case-insensitive manner.

Copy link
Copy Markdown
Contributor

@Majkl578 Majkl578 May 22, 2016

Choose a reason for hiding this comment

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

Also this doesn't properly handle quoted values.

Copy link
Copy Markdown
Contributor Author

@patrickkusebauch patrickkusebauch May 22, 2016

Choose a reason for hiding this comment

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

Parameter names must be treated in case-insensitive manner.
solved by commit 44660bd

Copy link
Copy Markdown
Contributor Author

@patrickkusebauch patrickkusebauch May 22, 2016

Choose a reason for hiding this comment

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

Quoted values in commit 11461e6

}
Copy link
Copy Markdown
Contributor

@JanTvrdik JanTvrdik May 22, 2016

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

@patrickkusebauch patrickkusebauch May 22, 2016

Choose a reason for hiding this comment

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

Integrated in commit a2c7a79


if (!empty($_SERVER['HTTP_X_FORWARDED_PORT'])) {
$url->setPort((int) $_SERVER['HTTP_X_FORWARDED_PORT']);
}
if (isset($proxyParams['for'])) {
$remoteAddr = explode(':', $proxyParams['for'][0])[0];
Copy link
Copy Markdown
Contributor

@Majkl578 Majkl578 May 22, 2016

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

@patrickkusebauch patrickkusebauch May 22, 2016

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

}

if (isset($proxyParams['host']) && count($proxyParams['host']) === 1) {
$remoteHostArr = explode(':', $proxyParams['host'][0]);
if(count($remoteHostArr) === 2) {
$remoteHost = $remoteHostArr[0];
$url->setPort((int) $remoteHostArr[1]);
} else {
$remoteHost = $proxyParams['host'][0];
}
}

$scheme = (isset($proxyParams['scheme']) && count($proxyParams['scheme']) === 1) ? $proxyParams['scheme'] : 'http';
$url->setScheme(strcasecmp($scheme, 'https') === 0 ? 'https' : 'http');
} else {
if (!empty($_SERVER['HTTP_X_FORWARDED_PROTO'])) {
$url->setScheme(strcasecmp($_SERVER['HTTP_X_FORWARDED_PROTO'], 'https') === 0 ? 'https' : 'http');
}

if (!empty($_SERVER['HTTP_X_FORWARDED_PORT'])) {
$url->setPort((int) $_SERVER['HTTP_X_FORWARDED_PORT']);
}

if (!empty($_SERVER['HTTP_X_FORWARDED_FOR'])) {
$xForwardedForWithoutProxies = array_filter(explode(',', $_SERVER['HTTP_X_FORWARDED_FOR']), function ($ip) {
return !array_filter($this->proxies, function ($proxy) use ($ip) {
return Helpers::ipMatch(trim($ip), $proxy);
if (!empty($_SERVER['HTTP_X_FORWARDED_FOR'])) {
$xForwardedForWithoutProxies = array_filter(explode(',', $_SERVER['HTTP_X_FORWARDED_FOR']), function ($ip) {
return !array_filter($this->proxies, function ($proxy) use ($ip) {
return Helpers::ipMatch(trim($ip), $proxy);
});
});
});
$remoteAddr = trim(end($xForwardedForWithoutProxies));
$xForwardedForRealIpKey = key($xForwardedForWithoutProxies);
}
$remoteAddr = trim(end($xForwardedForWithoutProxies));
$xForwardedForRealIpKey = key($xForwardedForWithoutProxies);
}

if (isset($xForwardedForRealIpKey) && !empty($_SERVER['HTTP_X_FORWARDED_HOST'])) {
$xForwardedHost = explode(',', $_SERVER['HTTP_X_FORWARDED_HOST']);
if (isset($xForwardedHost[$xForwardedForRealIpKey])) {
$remoteHost = trim($xForwardedHost[$xForwardedForRealIpKey]);
if (isset($xForwardedForRealIpKey) && !empty($_SERVER['HTTP_X_FORWARDED_HOST'])) {
$xForwardedHost = explode(',', $_SERVER['HTTP_X_FORWARDED_HOST']);
if (isset($xForwardedHost[$xForwardedForRealIpKey])) {
$remoteHost = trim($xForwardedHost[$xForwardedForRealIpKey]);
}
}
}
}
Expand Down
17 changes: 17 additions & 0 deletions tests/Http/RequestFactory.proxy.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,20 @@ test(function () {
Assert::same('172.16.0.1', $factory->createHttpRequest()->getRemoteAddress());
Assert::same('real', $factory->createHttpRequest()->getRemoteHost());
});

test(function () {
$_SERVER = [
'REMOTE_ADDR' => '127.0.0.3',
'REMOTE_HOST' => 'localhost',
'HTTP_FORWARDED' => 'for=23.75.45.200:85;host=otherhost',
];

$factory = new RequestFactory;
$factory->setProxy('127.0.0.1');
Assert::same('127.0.0.3', $factory->createHttpRequest()->getRemoteAddress());
Assert::same('localhost', $factory->createHttpRequest()->getRemoteHost());

$factory->setProxy('127.0.0.1/8');
Assert::same('23.75.45.200', $factory->createHttpRequest()->getRemoteAddress());
Assert::same('otherhost', $factory->createHttpRequest()->getRemoteHost());
});