Skip to content

Commit ac7436d

Browse files
zakdmazakdma
committed
MAGETWO-85134: IP resolving issue
1 parent 4b49996 commit ac7436d

File tree

2 files changed

+209
-43
lines changed

2 files changed

+209
-43
lines changed

‎lib/internal/Magento/Framework/HTTP/PhpEnvironment/RemoteAddress.php

Lines changed: 95 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,22 @@
55
*/
66
namespace Magento\Framework\HTTP\PhpEnvironment;
77

8+
use Magento\Framework\App\RequestInterface;
9+
810
/**
9-
* Library for working with client ip address
11+
* Library for working with client ip address.
1012
*/
1113
class RemoteAddress
1214
{
1315
/**
14-
* Request object
16+
* Request object.
1517
*
16-
* @var \Magento\Framework\App\RequestInterface
18+
* @var RequestInterface
1719
*/
1820
protected $request;
1921

2022
/**
21-
* Remote address cache
23+
* Remote address cache.
2224
*
2325
* @var string
2426
*/
@@ -30,46 +32,114 @@ class RemoteAddress
3032
protected $alternativeHeaders;
3133

3234
/**
33-
* @param \Magento\Framework\App\RequestInterface $httpRequest
35+
* @var string[]|null
36+
*/
37+
private $trustedProxies;
38+
39+
/**
40+
* @param RequestInterface $httpRequest
3441
* @param array $alternativeHeaders
42+
* @param string[]|null $trustedProxies
3543
*/
36-
public function __construct(\Magento\Framework\App\RequestInterface $httpRequest, array $alternativeHeaders = [])
37-
{
44+
public function __construct(
45+
RequestInterface $httpRequest,
46+
array $alternativeHeaders = [],
47+
array $trustedProxies = null
48+
) {
3849
$this->request = $httpRequest;
3950
$this->alternativeHeaders = $alternativeHeaders;
51+
$this->trustedProxies = $trustedProxies;
4052
}
4153

4254
/**
43-
* Retrieve Client Remote Address
55+
* Read address based on settings.
4456
*
45-
* @param bool $ipToLong converting IP to long format
46-
* @return string IPv4|long
57+
* @return string|null
4758
*/
48-
public function getRemoteAddress($ipToLong = false)
59+
private function readAddress()
4960
{
50-
if ($this->remoteAddress === null) {
51-
foreach ($this->alternativeHeaders as $var) {
52-
if ($this->request->getServer($var, false)) {
53-
$this->remoteAddress = $this->request->getServer($var);
54-
break;
55-
}
61+
$remoteAddress = null;
62+
foreach ($this->alternativeHeaders as $var) {
63+
if ($this->request->getServer($var, false)) {
64+
$remoteAddress = $this->request->getServer($var);
65+
break;
5666
}
67+
}
5768

58-
if (!$this->remoteAddress) {
59-
$this->remoteAddress = $this->request->getServer('REMOTE_ADDR');
69+
if (!$remoteAddress) {
70+
$remoteAddress = $this->request->getServer('REMOTE_ADDR');
71+
}
72+
73+
return $remoteAddress;
74+
}
75+
76+
/**
77+
* Filter addresses by trusted proxies list.
78+
*
79+
* @param string $remoteAddress
80+
* @return string|null
81+
*/
82+
private function filterAddress(string $remoteAddress)
83+
{
84+
if (strpos($remoteAddress, ',') !== false) {
85+
$ipList = explode(',', $remoteAddress);
86+
} else {
87+
$ipList = [$remoteAddress];
88+
}
89+
$ipList = array_filter(
90+
$ipList,
91+
function (string $ip) {
92+
return filter_var(trim($ip), FILTER_VALIDATE_IP);
6093
}
94+
);
95+
if ($this->trustedProxies !== null) {
96+
$ipList = array_filter(
97+
$ipList,
98+
function (string $ip) {
99+
return !in_array(trim($ip), $this->trustedProxies, true);
100+
}
101+
);
102+
$remoteAddress = trim(array_pop($ipList));
103+
} else {
104+
$remoteAddress = trim(reset($ipList));
61105
}
62106

63-
if (!$this->remoteAddress) {
64-
return false;
107+
return $remoteAddress ?: null;
108+
}
109+
110+
/**
111+
* Retrieve Client Remote Address.
112+
* If alternative headers are used and said headers allow multiple IPs
113+
* it is suggested that trusted proxies is also used
114+
* for more accurate IP recognition.
115+
*
116+
* @param bool $ipToLong converting IP to long format
117+
*
118+
* @return string IPv4|long
119+
*/
120+
public function getRemoteAddress(bool $ipToLong = false)
121+
{
122+
if ($this->remoteAddress !== null) {
123+
return $this->remoteAddress;
65124
}
66125

67-
if (strpos($this->remoteAddress, ',') !== false) {
68-
$ipList = explode(',', $this->remoteAddress);
69-
$this->remoteAddress = trim(reset($ipList));
126+
$remoteAddress = $this->readAddress();
127+
if (!$remoteAddress) {
128+
$this->remoteAddress = false;
129+
130+
return false;
70131
}
132+
$remoteAddress = $this->filterAddress($remoteAddress);
71133

72-
return $ipToLong ? ip2long($this->remoteAddress) : $this->remoteAddress;
134+
if (!$remoteAddress) {
135+
$this->remoteAddress = false;
136+
137+
return false;
138+
} else {
139+
$this->remoteAddress = $remoteAddress;
140+
141+
return $ipToLong ? ip2long($this->remoteAddress) : $this->remoteAddress;
142+
}
73143
}
74144

75145
/**

‎lib/internal/Magento/Framework/HTTP/Test/Unit/PhpEnvironment/RemoteAddressTest.php

Lines changed: 114 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,44 +5,70 @@
55
*/
66
namespace Magento\Framework\HTTP\Test\Unit\PhpEnvironment;
77

8+
use Magento\Framework\HTTP\PhpEnvironment\RemoteAddress;
9+
use Magento\Framework\App\Request\Http as HttpRequest;
10+
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
11+
812
class RemoteAddressTest extends \PHPUnit\Framework\TestCase
913
{
1014
/**
11-
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\App\Request\Http
15+
* @var \PHPUnit_Framework_MockObject_MockObject|\HttpRequest
1216
*/
1317
protected $_request;
1418

1519
/**
16-
* @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager
20+
* @var ObjectManager
1721
*/
1822
protected $_objectManager;
1923

24+
/**
25+
* @inheritdoc
26+
*/
2027
protected function setUp()
2128
{
22-
$this->_request = $this->getMockBuilder(
23-
\Magento\Framework\App\Request\Http::class
24-
)->disableOriginalConstructor()->setMethods(
25-
['getServer']
26-
)->getMock();
29+
$this->_request = $this->getMockBuilder(HttpRequest::class)
30+
->disableOriginalConstructor()
31+
->setMethods(['getServer'])
32+
->getMock();
2733

28-
$this->_objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
34+
$this->_objectManager = new ObjectManager($this);
2935
}
3036

3137
/**
38+
* @param string[] $alternativeHeaders
39+
* @param array $serverValueMap
40+
* @param string|bool $expected
41+
* @param bool $ipToLong
42+
* @param string[]|null $trustedProxies
43+
* @return void
3244
* @dataProvider getRemoteAddressProvider
3345
*/
34-
public function testGetRemoteAddress($alternativeHeaders, $serverValueMap, $expected, $ipToLong)
35-
{
46+
public function testGetRemoteAddress(
47+
array $alternativeHeaders,
48+
array $serverValueMap,
49+
$expected,
50+
bool $ipToLong,
51+
array $trustedProxies = null
52+
): void {
3653
$remoteAddress = $this->_objectManager->getObject(
37-
\Magento\Framework\HTTP\PhpEnvironment\RemoteAddress::class,
38-
['httpRequest' => $this->_request, 'alternativeHeaders' => $alternativeHeaders]
54+
RemoteAddress::class,
55+
[
56+
'httpRequest' => $this->_request,
57+
'alternativeHeaders' => $alternativeHeaders,
58+
'trustedProxies' => $trustedProxies,
59+
]
3960
);
40-
$this->_request->expects($this->any())->method('getServer')->will($this->returnValueMap($serverValueMap));
61+
$this->_request->expects($this->any())
62+
->method('getServer')
63+
->will($this->returnValueMap($serverValueMap));
64+
4165
$this->assertEquals($expected, $remoteAddress->getRemoteAddress($ipToLong));
4266
}
4367

4468
/**
4569
* @return array
70+
*
71+
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
4672
*/
4773
public function getRemoteAddressProvider()
4874
{
@@ -52,18 +78,21 @@ public function getRemoteAddressProvider()
5278
'serverValueMap' => [['REMOTE_ADDR', null, null]],
5379
'expected' => false,
5480
'ipToLong' => false,
81+
'trustedProxies' => null,
5582
],
5683
[
5784
'alternativeHeaders' => [],
5885
'serverValueMap' => [['REMOTE_ADDR', null, '192.168.0.1']],
5986
'expected' => '192.168.0.1',
60-
'ipToLong' => false
87+
'ipToLong' => false,
88+
'trustedProxies' => null,
6189
],
6290
[
6391
'alternativeHeaders' => [],
6492
'serverValueMap' => [['REMOTE_ADDR', null, '192.168.1.1']],
6593
'expected' => ip2long('192.168.1.1'),
66-
'ipToLong' => true
94+
'ipToLong' => true,
95+
'trustedProxies' => null,
6796
],
6897
[
6998
'alternativeHeaders' => ['TEST_HEADER'],
@@ -73,7 +102,8 @@ public function getRemoteAddressProvider()
73102
['TEST_HEADER', false, '192.168.0.1'],
74103
],
75104
'expected' => '192.168.0.1',
76-
'ipToLong' => false
105+
'ipToLong' => false,
106+
'trustedProxies' => null,
77107
],
78108
[
79109
'alternativeHeaders' => ['TEST_HEADER'],
@@ -83,8 +113,74 @@ public function getRemoteAddressProvider()
83113
['TEST_HEADER', false, '192.168.0.1'],
84114
],
85115
'expected' => ip2long('192.168.0.1'),
86-
'ipToLong' => true
87-
]
116+
'ipToLong' => true,
117+
'trustedProxies' => null,
118+
],
119+
[
120+
'alternativeHeaders' => [],
121+
'serverValueMap' => [
122+
['REMOTE_ADDR', null, 'NotValidIp'],
123+
],
124+
'expected' => false,
125+
'ipToLong' => false,
126+
'trustedProxies' => ['127.0.0.1'],
127+
],
128+
[
129+
'alternativeHeaders' => ['TEST_HEADER'],
130+
'serverValueMap' => [
131+
['TEST_HEADER', null, 'NotValid, 192.168.0.1'],
132+
['TEST_HEADER', false, 'NotValid, 192.168.0.1'],
133+
],
134+
'expected' => '192.168.0.1',
135+
'ipToLong' => false,
136+
'trustedProxies' => ['127.0.0.1'],
137+
],
138+
[
139+
'alternativeHeaders' => ['TEST_HEADER'],
140+
'serverValueMap' => [
141+
['TEST_HEADER', null, '192.168.0.2, 192.168.0.1'],
142+
['TEST_HEADER', false, '192.168.0.2, 192.168.0.1'],
143+
],
144+
'expected' => '192.168.0.2',
145+
'ipToLong' => false,
146+
'trustedProxies' => null,
147+
],
148+
[
149+
'alternativeHeaders' => [],
150+
'serverValueMap' => [
151+
[
152+
'REMOTE_ADDR',
153+
null,
154+
'192.168.0.2, 192.168.0.1, 192.168.0.3',
155+
],
156+
[
157+
'REMOTE_ADDR',
158+
false,
159+
'192.168.0.2, 192.168.0.1, 192.168.0.3',
160+
],
161+
],
162+
'expected' => '192.168.0.1',
163+
'ipToLong' => false,
164+
'trustedProxies' => ['192.168.0.3'],
165+
],
166+
[
167+
'alternativeHeaders' => [],
168+
'serverValueMap' => [
169+
[
170+
'REMOTE_ADDR',
171+
null,
172+
'192.168.0.2, 192.168.0.1, 192.168.0.3',
173+
],
174+
[
175+
'REMOTE_ADDR',
176+
false,
177+
'192.168.0.2, 192.168.0.1, 192.168.0.3',
178+
],
179+
],
180+
'expected' => '192.168.0.3',
181+
'ipToLong' => false,
182+
'trustedProxies' => [],
183+
],
88184
];
89185
}
90186
}

0 commit comments

Comments
 (0)