merged branch fabpot/request (PR #7734)
This PR was merged into the master branch. Discussion ---------- Refactored tests of Request::getTrustedProxies() | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a This PR reorganizes the tests for the request trusted proxies as they were unreadable. In the process, I've also removed obsolete stuff and changed the order returned by `Request::getClientIps()` (not a BC break as this method was added in 2.3 -- see #7612). Commits -------75db8eb
[HttpFoundation] changed the order of IP addresses returned by Request::getClientIps()deccb76
[HttpFoundation] refactored trusted proxies tests to make them easier to understand and change1af9e5e
[Request] removed obsolete proxy setting in tests168b8cb
[HttpFoundation] removed obsolete request property
This commit is contained in:
commit
79cadc437d
@ -35,8 +35,6 @@ class Request
|
||||
const HEADER_CLIENT_PROTO = 'client_proto';
|
||||
const HEADER_CLIENT_PORT = 'client_port';
|
||||
|
||||
protected static $trustProxy = false;
|
||||
|
||||
protected static $trustedProxies = array();
|
||||
|
||||
/**
|
||||
@ -476,7 +474,6 @@ class Request
|
||||
public static function setTrustedProxies(array $proxies)
|
||||
{
|
||||
self::$trustedProxies = $proxies;
|
||||
self::$trustProxy = $proxies ? true : false;
|
||||
}
|
||||
|
||||
/**
|
||||
@ -664,6 +661,12 @@ class Request
|
||||
/**
|
||||
* Returns the client IP addresses.
|
||||
*
|
||||
* The most trusted IP address is first, and the less trusted one last.
|
||||
* The "real" client IP address is the last one, but this is also the
|
||||
* less trusted one.
|
||||
*
|
||||
* Use this method carefully; you should use getClientIp() instead.
|
||||
*
|
||||
* @return array The client IP addresses
|
||||
*
|
||||
* @see getClientIp()
|
||||
@ -672,7 +675,7 @@ class Request
|
||||
{
|
||||
$ip = $this->server->get('REMOTE_ADDR');
|
||||
|
||||
if (!self::$trustProxy) {
|
||||
if (!self::$trustedProxies) {
|
||||
return array($ip);
|
||||
}
|
||||
|
||||
@ -683,11 +686,11 @@ class Request
|
||||
$clientIps = array_map('trim', explode(',', $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_IP])));
|
||||
$clientIps[] = $ip;
|
||||
|
||||
$trustedProxies = self::$trustProxy && !self::$trustedProxies ? array($ip) : self::$trustedProxies;
|
||||
$trustedProxies = !self::$trustedProxies ? array($ip) : self::$trustedProxies;
|
||||
$ip = $clientIps[0];
|
||||
$clientIps = array_diff($clientIps, $trustedProxies);
|
||||
$clientIps = array_values(array_diff($clientIps, $trustedProxies));
|
||||
|
||||
return $clientIps ? $clientIps : array($ip);
|
||||
return $clientIps ? array_reverse($clientIps) : array($ip);
|
||||
}
|
||||
|
||||
/**
|
||||
@ -714,7 +717,7 @@ class Request
|
||||
{
|
||||
$ipAddresses = $this->getClientIps();
|
||||
|
||||
return array_pop($ipAddresses);
|
||||
return $ipAddresses[0];
|
||||
}
|
||||
|
||||
/**
|
||||
@ -827,7 +830,7 @@ class Request
|
||||
*/
|
||||
public function getPort()
|
||||
{
|
||||
if (self::$trustProxy && self::$trustedHeaders[self::HEADER_CLIENT_PORT] && $port = $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_PORT])) {
|
||||
if (self::$trustedProxies && self::$trustedHeaders[self::HEADER_CLIENT_PORT] && $port = $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_PORT])) {
|
||||
return $port;
|
||||
}
|
||||
|
||||
@ -988,7 +991,7 @@ class Request
|
||||
*/
|
||||
public function isSecure()
|
||||
{
|
||||
if (self::$trustProxy && self::$trustedHeaders[self::HEADER_CLIENT_PROTO] && $proto = $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_PROTO])) {
|
||||
if (self::$trustedProxies && self::$trustedHeaders[self::HEADER_CLIENT_PROTO] && $proto = $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_PROTO])) {
|
||||
return in_array(strtolower($proto), array('https', 'on', '1'));
|
||||
}
|
||||
|
||||
@ -1014,7 +1017,7 @@ class Request
|
||||
*/
|
||||
public function getHost()
|
||||
{
|
||||
if (self::$trustProxy && self::$trustedHeaders[self::HEADER_CLIENT_HOST] && $host = $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_HOST])) {
|
||||
if (self::$trustedProxies && self::$trustedHeaders[self::HEADER_CLIENT_HOST] && $host = $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_HOST])) {
|
||||
$elements = explode(',', $host);
|
||||
|
||||
$host = $elements[count($elements) - 1];
|
||||
|
@ -666,7 +666,6 @@ class RequestTest extends \PHPUnit_Framework_TestCase
|
||||
|
||||
$request->initialize(array(), array(), array(), array(), array(), array('SERVER_NAME' => 'www.exemple.com', 'HTTP_HOST' => 'www.host.com'));
|
||||
$this->assertEquals('www.host.com', $request->getHost(), '->getHost() value from Host header has priority over SERVER_NAME ');
|
||||
$this->stopTrustingProxyData();
|
||||
}
|
||||
|
||||
/**
|
||||
@ -739,41 +738,23 @@ class RequestTest extends \PHPUnit_Framework_TestCase
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider testGetClientIpProvider
|
||||
* @dataProvider testGetClientIpsProvider
|
||||
*/
|
||||
public function testGetClientIp($expected, $proxy, $remoteAddr, $httpForwardedFor, $trustedProxies)
|
||||
public function testGetClientIp($expected, $remoteAddr, $httpForwardedFor, $trustedProxies)
|
||||
{
|
||||
$request = $this->getRequestInstanceForClientIpTests($proxy, $remoteAddr, $httpForwardedFor, $trustedProxies);
|
||||
$request = $this->getRequestInstanceForClientIpTests($remoteAddr, $httpForwardedFor, $trustedProxies);
|
||||
|
||||
$this->assertEquals($expected, $request->getClientIp());
|
||||
$this->assertEquals($expected[0], $request->getClientIp());
|
||||
|
||||
Request::setTrustedProxies(array());
|
||||
}
|
||||
|
||||
public function testGetClientIpProvider()
|
||||
{
|
||||
return array(
|
||||
array('88.88.88.88', false, '88.88.88.88', null, null),
|
||||
array('88.88.88.88', true, '88.88.88.88', null, null),
|
||||
array('127.0.0.1', false, '127.0.0.1', null, null),
|
||||
array('::1', false, '::1', null, null),
|
||||
array('127.0.0.1', false, '127.0.0.1', '88.88.88.88', null),
|
||||
array('88.88.88.88', true, '127.0.0.1', '88.88.88.88', null),
|
||||
array('2620:0:1cfe:face:b00c::3', true, '::1', '2620:0:1cfe:face:b00c::3', null),
|
||||
array('88.88.88.88', true, '123.45.67.89', '127.0.0.1, 87.65.43.21, 88.88.88.88', null),
|
||||
array('87.65.43.21', true, '123.45.67.89', '127.0.0.1, 87.65.43.21, 88.88.88.88', array('123.45.67.89', '88.88.88.88')),
|
||||
array('87.65.43.21', false, '123.45.67.89', '127.0.0.1, 87.65.43.21, 88.88.88.88', array('123.45.67.89', '88.88.88.88')),
|
||||
array('88.88.88.88', true, '123.45.67.89', '88.88.88.88', array('123.45.67.89', '88.88.88.88')),
|
||||
array('88.88.88.88', false, '123.45.67.89', '88.88.88.88', array('123.45.67.89', '88.88.88.88')),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider testGetClientIpsProvider
|
||||
*/
|
||||
public function testGetClientIps($expected, $proxy, $remoteAddr, $httpForwardedFor, $trustedProxies)
|
||||
public function testGetClientIps($expected, $remoteAddr, $httpForwardedFor, $trustedProxies)
|
||||
{
|
||||
$request = $this->getRequestInstanceForClientIpTests($proxy, $remoteAddr, $httpForwardedFor, $trustedProxies);
|
||||
$request = $this->getRequestInstanceForClientIpTests($remoteAddr, $httpForwardedFor, $trustedProxies);
|
||||
|
||||
$this->assertEquals($expected, $request->getClientIps());
|
||||
|
||||
@ -782,16 +763,45 @@ class RequestTest extends \PHPUnit_Framework_TestCase
|
||||
|
||||
public function testGetClientIpsProvider()
|
||||
{
|
||||
// $expected $remoteAddr $httpForwardedFor $trustedProxies
|
||||
return array(
|
||||
array(array('88.88.88.88'), false, '88.88.88.88', null, null),
|
||||
array(array('127.0.0.1'), false, '127.0.0.1', null, null),
|
||||
array(array('::1'), false, '::1', null, null),
|
||||
array(array('127.0.0.1'), false, '127.0.0.1', '88.88.88.88', null),
|
||||
array(array('88.88.88.88'), true, '127.0.0.1', '88.88.88.88', null),
|
||||
array(array('2620:0:1cfe:face:b00c::3'), true, '::1', '2620:0:1cfe:face:b00c::3', null),
|
||||
array(array('127.0.0.1', '87.65.43.21', '88.88.88.88'), true, '123.45.67.89', '127.0.0.1, 87.65.43.21, 88.88.88.88', null),
|
||||
array(array('127.0.0.1', '87.65.43.21'), true, '123.45.67.89', '127.0.0.1, 87.65.43.21, 88.88.88.88', array('123.45.67.89', '88.88.88.88')),
|
||||
array(array('127.0.0.1', '87.65.43.21'), false, '123.45.67.89', '127.0.0.1, 87.65.43.21, 88.88.88.88', array('123.45.67.89', '88.88.88.88')),
|
||||
// simple IPv4
|
||||
array(array('88.88.88.88'), '88.88.88.88', null, null),
|
||||
// trust the IPv4 remote addr
|
||||
array(array('88.88.88.88'), '88.88.88.88', null, array('88.88.88.88')),
|
||||
|
||||
// simple IPv6
|
||||
array(array('::1'), '::1', null, null),
|
||||
// trust the IPv6 remote addr
|
||||
array(array('::1'), '::1', null, array('::1')),
|
||||
|
||||
// forwarded for with remote IPv4 addr not trusted
|
||||
array(array('127.0.0.1'), '127.0.0.1', '88.88.88.88', null),
|
||||
// forwarded for with remote IPv4 addr trusted
|
||||
array(array('88.88.88.88'), '127.0.0.1', '88.88.88.88', array('127.0.0.1')),
|
||||
// forwarded for with remote IPv4 and all FF addrs trusted
|
||||
array(array('88.88.88.88'), '127.0.0.1', '88.88.88.88', array('127.0.0.1', '88.88.88.88')),
|
||||
|
||||
// forwarded for with remote IPv6 addr not trusted
|
||||
array(array('1620:0:1cfe:face:b00c::3'), '1620:0:1cfe:face:b00c::3', '2620:0:1cfe:face:b00c::3', null),
|
||||
// forwarded for with remote IPv6 addr trusted
|
||||
array(array('2620:0:1cfe:face:b00c::3'), '1620:0:1cfe:face:b00c::3', '2620:0:1cfe:face:b00c::3', array('1620:0:1cfe:face:b00c::3')),
|
||||
|
||||
// multiple forwarded for with remote IPv4 addr trusted
|
||||
array(array('88.88.88.88', '87.65.43.21', '127.0.0.1'), '123.45.67.89', '127.0.0.1, 87.65.43.21, 88.88.88.88', array('123.45.67.89')),
|
||||
// multiple forwarded for with remote IPv4 addr and some reverse proxies trusted
|
||||
array(array('87.65.43.21', '127.0.0.1'), '123.45.67.89', '127.0.0.1, 87.65.43.21, 88.88.88.88', array('123.45.67.89', '88.88.88.88')),
|
||||
// multiple forwarded for with remote IPv4 addr and some reverse proxies trusted but in the middle
|
||||
array(array('88.88.88.88', '127.0.0.1'), '123.45.67.89', '127.0.0.1, 87.65.43.21, 88.88.88.88', array('123.45.67.89', '87.65.43.21')),
|
||||
// multiple forwarded for with remote IPv4 addr and all reverse proxies trusted
|
||||
array(array('127.0.0.1'), '123.45.67.89', '127.0.0.1, 87.65.43.21, 88.88.88.88', array('123.45.67.89', '87.65.43.21', '88.88.88.88', '127.0.0.1')),
|
||||
|
||||
// multiple forwarded for with remote IPv6 addr trusted
|
||||
array(array('2620:0:1cfe:face:b00c::3', '3620:0:1cfe:face:b00c::3'), '1620:0:1cfe:face:b00c::3', '3620:0:1cfe:face:b00c::3,2620:0:1cfe:face:b00c::3', array('1620:0:1cfe:face:b00c::3')),
|
||||
// multiple forwarded for with remote IPv6 addr and some reverse proxies trusted
|
||||
array(array('3620:0:1cfe:face:b00c::3'), '1620:0:1cfe:face:b00c::3', '3620:0:1cfe:face:b00c::3,2620:0:1cfe:face:b00c::3', array('1620:0:1cfe:face:b00c::3', '2620:0:1cfe:face:b00c::3')),
|
||||
// multiple forwarded for with remote IPv4 addr and some reverse proxies trusted but in the middle
|
||||
array(array('2620:0:1cfe:face:b00c::3', '4620:0:1cfe:face:b00c::3'), '1620:0:1cfe:face:b00c::3', '4620:0:1cfe:face:b00c::3,3620:0:1cfe:face:b00c::3,2620:0:1cfe:face:b00c::3', array('1620:0:1cfe:face:b00c::3', '3620:0:1cfe:face:b00c::3')),
|
||||
);
|
||||
}
|
||||
|
||||
@ -1277,14 +1287,6 @@ class RequestTest extends \PHPUnit_Framework_TestCase
|
||||
);
|
||||
}
|
||||
|
||||
private function stopTrustingProxyData()
|
||||
{
|
||||
$class = new \ReflectionClass('Symfony\\Component\\HttpFoundation\\Request');
|
||||
$property = $class->getProperty('trustProxy');
|
||||
$property->setAccessible(true);
|
||||
$property->setValue(false);
|
||||
}
|
||||
|
||||
private function disableHttpMethodParameterOverride()
|
||||
{
|
||||
$class = new \ReflectionClass('Symfony\\Component\\HttpFoundation\\Request');
|
||||
@ -1293,7 +1295,7 @@ class RequestTest extends \PHPUnit_Framework_TestCase
|
||||
$property->setValue(false);
|
||||
}
|
||||
|
||||
private function getRequestInstanceForClientIpTests($proxy, $remoteAddr, $httpForwardedFor, $trustedProxies)
|
||||
private function getRequestInstanceForClientIpTests($remoteAddr, $httpForwardedFor, $trustedProxies)
|
||||
{
|
||||
$request = new Request();
|
||||
|
||||
@ -1302,8 +1304,8 @@ class RequestTest extends \PHPUnit_Framework_TestCase
|
||||
$server['HTTP_X_FORWARDED_FOR'] = $httpForwardedFor;
|
||||
}
|
||||
|
||||
if ($proxy || $trustedProxies) {
|
||||
Request::setTrustedProxies(null === $trustedProxies ? array($remoteAddr) : $trustedProxies);
|
||||
if ($trustedProxies) {
|
||||
Request::setTrustedProxies($trustedProxies);
|
||||
}
|
||||
|
||||
$request->initialize(array(), array(), array(), array(), array(), $server);
|
||||
|
Reference in New Issue
Block a user