Skip to content

Commit ed22067

Browse files
fabpotfabpot
committed
merged branch gnutix/issue-7349-fix (PR #7612)
This PR was merged into the master branch. Discussion ---------- [HttpFoundation] Split getClientIp into two methods for better flexibility | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | Locally: NO (HttpFoundation tests passes, but all Symfony tests fails because of my local setup). Travis : YES | Fixed tickets | #7349 | License | MIT | Doc PR | none (yet?) Split the ``Request::getClientIp`` method in two to allow better overriding flexibility. See #7349 for more information. Commits ------- 2678dd5 [HttpFoundation] Move comments from one method to the other [ci skip]. 7529664 [HttpFoundation] Removed the @api annotation on getClientIps method. f0c4ab6 [HttpFoundation] Split the tests into two methods / data providers. 80030fb [HttpFoundation] Use @see annotation for better documentation generation. 3e703a2 [HttpFoundation] Fixing tests. 63cbbb5 [HttpFoundation] Add tests for the newly created Request::getClientIps() method. 58347fb [HttpFoundation] Split getClientIp into two methods for better flexibility.
2 parents 39a9c16 + 5cf6a73 commit ed22067

File tree

2 files changed

+77
-27
lines changed

2 files changed

+77
-27
lines changed

‎Request.php

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -661,6 +661,34 @@ public function setSession(SessionInterface $session)
661661
$this->session = $session;
662662
}
663663

664+
/**
665+
* Returns the client IP addresses.
666+
*
667+
* @return array The client IP addresses
668+
*
669+
* @see getClientIp()
670+
*/
671+
public function getClientIps()
672+
{
673+
$ip = $this->server->get('REMOTE_ADDR');
674+
675+
if (!self::$trustProxy) {
676+
return array($ip);
677+
}
678+
679+
if (!self::$trustedHeaders[self::HEADER_CLIENT_IP] || !$this->headers->has(self::$trustedHeaders[self::HEADER_CLIENT_IP])) {
680+
return array($ip);
681+
}
682+
683+
$clientIps = array_map('trim', explode(',', $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_IP])));
684+
$clientIps[] = $ip;
685+
686+
$trustedProxies = self::$trustProxy && !self::$trustedProxies ? array($ip) : self::$trustedProxies;
687+
$clientIps = array_diff($clientIps, $trustedProxies);
688+
689+
return $clientIps;
690+
}
691+
664692
/**
665693
* Returns the client IP address.
666694
*
@@ -676,29 +704,16 @@ public function setSession(SessionInterface $session)
676704
*
677705
* @return string The client IP address
678706
*
707+
* @see getClientIps()
679708
* @see http://en.wikipedia.org/wiki/X-Forwarded-For
680709
*
681710
* @api
682711
*/
683712
public function getClientIp()
684713
{
685-
$ip = $this->server->get('REMOTE_ADDR');
686-
687-
if (!self::$trustProxy) {
688-
return $ip;
689-
}
690-
691-
if (!self::$trustedHeaders[self::HEADER_CLIENT_IP] || !$this->headers->has(self::$trustedHeaders[self::HEADER_CLIENT_IP])) {
692-
return $ip;
693-
}
694-
695-
$clientIps = array_map('trim', explode(',', $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_IP])));
696-
$clientIps[] = $ip;
697-
698-
$trustedProxies = self::$trustProxy && !self::$trustedProxies ? array($ip) : self::$trustedProxies;
699-
$clientIps = array_diff($clientIps, $trustedProxies);
714+
$ipAddresses = $this->getClientIps();
700715

701-
return array_pop($clientIps);
716+
return array_pop($ipAddresses);
702717
}
703718

704719
/**

‎Tests/RequestTest.php

Lines changed: 46 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -743,18 +743,8 @@ public function testGetSetMethod()
743743
*/
744744
public function testGetClientIp($expected, $proxy, $remoteAddr, $httpForwardedFor, $trustedProxies)
745745
{
746-
$request = new Request();
747-
748-
$server = array('REMOTE_ADDR' => $remoteAddr);
749-
if (null !== $httpForwardedFor) {
750-
$server['HTTP_X_FORWARDED_FOR'] = $httpForwardedFor;
751-
}
752-
753-
if ($proxy || $trustedProxies) {
754-
Request::setTrustedProxies(null === $trustedProxies ? array($remoteAddr) : $trustedProxies);
755-
}
746+
$request = $this->getRequestInstanceForClientIpTests($proxy, $remoteAddr, $httpForwardedFor, $trustedProxies);
756747

757-
$request->initialize(array(), array(), array(), array(), array(), $server);
758748
$this->assertEquals($expected, $request->getClientIp());
759749

760750
Request::setTrustedProxies(array());
@@ -775,6 +765,33 @@ public function testGetClientIpProvider()
775765
);
776766
}
777767

768+
/**
769+
* @dataProvider testGetClientIpsProvider
770+
*/
771+
public function testGetClientIps($expected, $proxy, $remoteAddr, $httpForwardedFor, $trustedProxies)
772+
{
773+
$request = $this->getRequestInstanceForClientIpTests($proxy, $remoteAddr, $httpForwardedFor, $trustedProxies);
774+
775+
$this->assertEquals($expected, $request->getClientIps());
776+
777+
Request::setTrustedProxies(array());
778+
}
779+
780+
public function testGetClientIpsProvider()
781+
{
782+
return array(
783+
array(array('88.88.88.88'), false, '88.88.88.88', null, null),
784+
array(array('127.0.0.1'), false, '127.0.0.1', null, null),
785+
array(array('::1'), false, '::1', null, null),
786+
array(array('127.0.0.1'), false, '127.0.0.1', '88.88.88.88', null),
787+
array(array('88.88.88.88'), true, '127.0.0.1', '88.88.88.88', null),
788+
array(array('2620:0:1cfe:face:b00c::3'), true, '::1', '2620:0:1cfe:face:b00c::3', null),
789+
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),
790+
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')),
791+
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')),
792+
);
793+
}
794+
778795
public function testGetContentWorksTwiceInDefaultMode()
779796
{
780797
$req = new Request;
@@ -1273,6 +1290,24 @@ private function disableHttpMethodParameterOverride()
12731290
$property->setValue(false);
12741291
}
12751292

1293+
private function getRequestInstanceForClientIpTests($proxy, $remoteAddr, $httpForwardedFor, $trustedProxies)
1294+
{
1295+
$request = new Request();
1296+
1297+
$server = array('REMOTE_ADDR' => $remoteAddr);
1298+
if (null !== $httpForwardedFor) {
1299+
$server['HTTP_X_FORWARDED_FOR'] = $httpForwardedFor;
1300+
}
1301+
1302+
if ($proxy || $trustedProxies) {
1303+
Request::setTrustedProxies(null === $trustedProxies ? array($remoteAddr) : $trustedProxies);
1304+
}
1305+
1306+
$request->initialize(array(), array(), array(), array(), array(), $server);
1307+
1308+
return $request;
1309+
}
1310+
12761311
public function testTrustedProxies()
12771312
{
12781313
$request = Request::create('http://example.com/');

0 commit comments

Comments
 (0)