Skip to content

Commit 76000ab

Browse files
kenjiskenjis
authored
Merge pull request #6820 from kenjis/fix-Request-getIPAddress
fix: Request::getIPAddress()
2 parents bc38d85 + ead7757 commit 76000ab

File tree

4 files changed

+51
-34
lines changed

4 files changed

+51
-34
lines changed

‎system/HTTP/RequestTrait.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,13 @@ public function getIPAddress(): string
6767
$this->ipAddress = $this->getServer('REMOTE_ADDR');
6868

6969
if ($proxyIPs) {
70-
foreach (['HTTP_X_FORWARDED_FOR', 'HTTP_CLIENT_IP', 'HTTP_X_CLIENT_IP', 'HTTP_X_CLUSTER_CLIENT_IP'] as $header) {
71-
if (($spoof = $this->getServer($header)) !== null) {
70+
foreach (['x-forwarded-for', 'client-ip', 'x-client-ip', 'x-cluster-client-ip'] as $header) {
71+
$spoof = null;
72+
$headerObj = $this->header($header);
73+
74+
if ($headerObj !== null) {
75+
$spoof = $headerObj->getValue();
76+
7277
// Some proxies typically list the whole chain of IP
7378
// addresses through which the client has reached us.
7479
// e.g. client_ip, proxy_ip1, proxy_ip2, etc.

‎tests/system/HTTP/IncomingRequestTest.php

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -700,7 +700,10 @@ public function testGetIPAddressNormal()
700700
{
701701
$expected = '123.123.123.123';
702702
$_SERVER['REMOTE_ADDR'] = $expected;
703-
$this->request = new Request(new App());
703+
704+
$this->request = new Request(new App());
705+
$this->request->populateHeaders();
706+
704707
$this->assertSame($expected, $this->request->getIPAddress());
705708
// call a second time to exercise the initial conditional block in getIPAddress()
706709
$this->assertSame($expected, $this->request->getIPAddress());
@@ -709,66 +712,75 @@ public function testGetIPAddressNormal()
709712
public function testGetIPAddressThruProxy()
710713
{
711714
$expected = '123.123.123.123';
715+
$_SERVER['HTTP_X_FORWARDED_FOR'] = $expected;
712716
$_SERVER['REMOTE_ADDR'] = '10.0.1.200';
713717
$config = new App();
714718
$config->proxyIPs = '10.0.1.200,192.168.5.0/24';
715-
$_SERVER['HTTP_X_FORWARDED_FOR'] = $expected;
716-
$this->request = new Request($config);
719+
720+
$this->request = new Request($config);
721+
$this->request->populateHeaders();
717722

718723
// we should see the original forwarded address
719724
$this->assertSame($expected, $this->request->getIPAddress());
720725
}
721726

722727
public function testGetIPAddressThruProxyInvalid()
723728
{
724-
$expected = '123.456.23.123';
725-
$_SERVER['REMOTE_ADDR'] = '10.0.1.200';
729+
$_SERVER['HTTP_X_FORWARDED_FOR'] = '123.456.23.123';
730+
$expected = '10.0.1.200';
731+
$_SERVER['REMOTE_ADDR'] = $expected;
726732
$config = new App();
727733
$config->proxyIPs = '10.0.1.200,192.168.5.0/24';
728-
$_SERVER['HTTP_X_FORWARDED_FOR'] = $expected;
729-
$this->request = new Request($config);
734+
735+
$this->request = new Request($config);
736+
$this->request->populateHeaders();
730737

731738
// spoofed address invalid
732-
$this->assertSame('10.0.1.200', $this->request->getIPAddress());
739+
$this->assertSame($expected, $this->request->getIPAddress());
733740
}
734741

735742
public function testGetIPAddressThruProxyNotWhitelisted()
736743
{
737-
$expected = '123.456.23.123';
738-
$_SERVER['REMOTE_ADDR'] = '10.10.1.200';
744+
$expected = '10.10.1.200';
745+
$_SERVER['REMOTE_ADDR'] = $expected;
746+
$_SERVER['HTTP_X_FORWARDED_FOR'] = '123.456.23.123';
739747
$config = new App();
740748
$config->proxyIPs = '10.0.1.200,192.168.5.0/24';
741-
$_SERVER['HTTP_X_FORWARDED_FOR'] = $expected;
742-
$this->request = new Request($config);
749+
750+
$this->request = new Request($config);
751+
$this->request->populateHeaders();
743752

744753
// spoofed address invalid
745-
$this->assertSame('10.10.1.200', $this->request->getIPAddress());
754+
$this->assertSame($expected, $this->request->getIPAddress());
746755
}
747756

748757
public function testGetIPAddressThruProxySubnet()
749758
{
750759
$expected = '123.123.123.123';
760+
$_SERVER['HTTP_X_FORWARDED_FOR'] = $expected;
751761
$_SERVER['REMOTE_ADDR'] = '192.168.5.21';
752762
$config = new App();
753763
$config->proxyIPs = ['192.168.5.0/24'];
754-
$_SERVER['HTTP_X_FORWARDED_FOR'] = $expected;
755-
$this->request = new Request($config);
764+
765+
$this->request = new Request($config);
766+
$this->request->populateHeaders();
756767

757768
// we should see the original forwarded address
758769
$this->assertSame($expected, $this->request->getIPAddress());
759770
}
760771

761772
public function testGetIPAddressThruProxyOutofSubnet()
762773
{
763-
$expected = '123.123.123.123';
764-
$_SERVER['REMOTE_ADDR'] = '192.168.5.21';
774+
$expected = '192.168.5.21';
775+
$_SERVER['REMOTE_ADDR'] = $expected;
765776
$config = new App();
766777
$config->proxyIPs = ['192.168.5.0/28'];
767-
$_SERVER['HTTP_X_FORWARDED_FOR'] = $expected;
778+
$_SERVER['HTTP_X_FORWARDED_FOR'] = '123.123.123.123';
768779
$this->request = new Request($config);
780+
$this->request->populateHeaders();
769781

770782
// we should see the original forwarded address
771-
$this->assertSame('192.168.5.21', $this->request->getIPAddress());
783+
$this->assertSame($expected, $this->request->getIPAddress());
772784
}
773785

774786
// @TODO getIPAddress should have more testing, to 100% code coverage

‎tests/system/HTTP/MessageTest.php

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,6 @@ protected function setUp(): void
3030
$this->message = new Message();
3131
}
3232

33-
protected function tearDown(): void
34-
{
35-
$this->message = null;
36-
unset($this->message);
37-
}
38-
3933
// We can only test the headers retrieved from $_SERVER
4034
// This test might fail under apache.
4135
public function testHeadersRetrievesHeaders()
@@ -246,23 +240,24 @@ public function testSetHeaderWithExistingArrayValuesAppendNullValue()
246240

247241
public function testPopulateHeadersWithoutContentType()
248242
{
249-
// fail path, if the CONTENT_TYPE doesn't exist
250243
$original = $_SERVER;
251-
$_SERVER = ['HTTP_ACCEPT_LANGUAGE' => 'en-us,en;q=0.50'];
252244
$originalEnv = getenv('CONTENT_TYPE');
245+
246+
// fail path, if the CONTENT_TYPE doesn't exist
247+
$_SERVER = ['HTTP_ACCEPT_LANGUAGE' => 'en-us,en;q=0.50'];
253248
putenv('CONTENT_TYPE');
254249

255250
$this->message->populateHeaders();
256251

257252
$this->assertNull($this->message->header('content-type'));
253+
258254
putenv("CONTENT_TYPE={$originalEnv}");
259-
$this->message->removeHeader('accept-language');
260255
$_SERVER = $original; // restore so code coverage doesn't break
261256
}
262257

263258
public function testPopulateHeadersWithoutHTTP()
264259
{
265-
// fail path, if arguement does't have the HTTP_*
260+
// fail path, if argument doesn't have the HTTP_*
266261
$original = $_SERVER;
267262
$_SERVER = [
268263
'USER_AGENT' => 'Mozilla/5.0 (iPad; U; CPU OS 3_2_1 like Mac OS X; en-us) AppleWebKit/531.21.10 (KHTML, like Gecko) Mobile/7B405',
@@ -273,6 +268,7 @@ public function testPopulateHeadersWithoutHTTP()
273268

274269
$this->assertNull($this->message->header('user-agent'));
275270
$this->assertNull($this->message->header('request-method'));
271+
276272
$_SERVER = $original; // restore so code coverage doesn't break
277273
}
278274

@@ -288,7 +284,7 @@ public function testPopulateHeadersKeyNotExists()
288284
$this->message->populateHeaders();
289285

290286
$this->assertSame('', $this->message->header('accept-charset')->getValue());
291-
$this->message->removeHeader('accept-charset');
287+
292288
$_SERVER = $original; // restore so code coverage doesn't break
293289
}
294290

@@ -305,8 +301,7 @@ public function testPopulateHeaders()
305301

306302
$this->assertSame('text/html; charset=utf-8', $this->message->header('content-type')->getValue());
307303
$this->assertSame('en-us,en;q=0.50', $this->message->header('accept-language')->getValue());
308-
$this->message->removeHeader('content-type');
309-
$this->message->removeHeader('accept-language');
304+
310305
$_SERVER = $original; // restore so code coverage doesn't break
311306
}
312307
}

‎tests/system/HTTP/RequestTest.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -613,6 +613,7 @@ public function testGetIPAddressThruProxy()
613613
$config->proxyIPs = '10.0.1.200,192.168.5.0/24';
614614
$_SERVER['HTTP_X_FORWARDED_FOR'] = $expected;
615615
$this->request = new Request($config);
616+
$this->request->populateHeaders();
616617

617618
// we should see the original forwarded address
618619
$this->assertSame($expected, $this->request->getIPAddress());
@@ -626,6 +627,7 @@ public function testGetIPAddressThruProxyInvalid()
626627
$config->proxyIPs = '10.0.1.200,192.168.5.0/24';
627628
$_SERVER['HTTP_X_FORWARDED_FOR'] = $expected;
628629
$this->request = new Request($config);
630+
$this->request->populateHeaders();
629631

630632
// spoofed address invalid
631633
$this->assertSame('10.0.1.200', $this->request->getIPAddress());
@@ -639,6 +641,7 @@ public function testGetIPAddressThruProxyNotWhitelisted()
639641
$config->proxyIPs = '10.0.1.200,192.168.5.0/24';
640642
$_SERVER['HTTP_X_FORWARDED_FOR'] = $expected;
641643
$this->request = new Request($config);
644+
$this->request->populateHeaders();
642645

643646
// spoofed address invalid
644647
$this->assertSame('10.10.1.200', $this->request->getIPAddress());
@@ -652,6 +655,7 @@ public function testGetIPAddressThruProxySubnet()
652655
$config->proxyIPs = ['192.168.5.0/24'];
653656
$_SERVER['HTTP_X_FORWARDED_FOR'] = $expected;
654657
$this->request = new Request($config);
658+
$this->request->populateHeaders();
655659

656660
// we should see the original forwarded address
657661
$this->assertSame($expected, $this->request->getIPAddress());
@@ -665,6 +669,7 @@ public function testGetIPAddressThruProxyOutofSubnet()
665669
$config->proxyIPs = ['192.168.5.0/28'];
666670
$_SERVER['HTTP_X_FORWARDED_FOR'] = $expected;
667671
$this->request = new Request($config);
672+
$this->request->populateHeaders();
668673

669674
// we should see the original forwarded address
670675
$this->assertSame('192.168.5.21', $this->request->getIPAddress());

0 commit comments

Comments
 (0)