Skip to content

Commit

Permalink
Merge pull request #7621 from kenjis/remove-Request-proxyIPs
Browse files Browse the repository at this point in the history
refactor: drop support for `Config\App::$proxyIPs = ''`
  • Loading branch information
kenjis authored Jun 28, 2023
2 parents d19d034 + e9d9026 commit 40409b3
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 29 deletions.
2 changes: 2 additions & 0 deletions rector.php
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@
__DIR__ . '/system/Debug/Exceptions.php',
// @TODO remove if deprecated $httpVerb is removed
__DIR__ . '/system/Router/AutoRouterImproved.php',
// @TODO remove if deprecated $config is removed
__DIR__ . '/system/HTTP/Request.php',
],

// check on constant compare
Expand Down
17 changes: 6 additions & 11 deletions system/HTTP/Request.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class Request extends OutgoingRequest implements RequestInterface
*
* @var array<string, string>
*
* @deprecated Check the App config directly
* @deprecated 4.0.5 No longer used. Check the App config directly
*/
protected $proxyIPs;

Expand All @@ -35,15 +35,10 @@ class Request extends OutgoingRequest implements RequestInterface
*
* @param App $config
*
* @deprecated The $config is no longer needed and will be removed in a future version
* @deprecated 4.0.5 The $config is no longer needed and will be removed in a future version
*/
public function __construct($config = null)
public function __construct($config = null) // @phpstan-ignore-line
{
/**
* @deprecated $this->proxyIps property will be removed in the future
*/
$this->proxyIPs = $config->proxyIPs;

if (empty($this->method)) {
$this->method = $this->getServer('REQUEST_METHOD') ?? 'GET';
}
Expand All @@ -59,7 +54,7 @@ public function __construct($config = null)
* @param string $ip IP Address
* @param string $which IP protocol: 'ipv4' or 'ipv6'
*
* @deprecated Use Validation instead
* @deprecated 4.0.5 Use Validation instead
*
* @codeCoverageIgnore
*/
Expand All @@ -73,7 +68,7 @@ public function isValidIP(?string $ip = null, ?string $which = null): bool
*
* @param bool $upper Whether to return in upper or lower case.
*
* @deprecated The $upper functionality will be removed and this will revert to its PSR-7 equivalent
* @deprecated 4.0.5 The $upper functionality will be removed and this will revert to its PSR-7 equivalent
*
* @codeCoverageIgnore
*/
Expand All @@ -87,7 +82,7 @@ public function getMethod(bool $upper = false): string
*
* @return $this
*
* @deprecated Use withMethod() instead for immutability
* @deprecated 4.0.5 Use withMethod() instead for immutability
*
* @codeCoverageIgnore
*/
Expand Down
13 changes: 2 additions & 11 deletions system/HTTP/RequestTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,17 +60,8 @@ public function getIPAddress(): string
'valid_ip',
];

/**
* @deprecated $this->proxyIPs property will be removed in the future
*/
// @phpstan-ignore-next-line
$proxyIPs = $this->proxyIPs ?? config(App::class)->proxyIPs;
// @phpstan-ignore-next-line

// Workaround for old Config\App file. App::$proxyIPs may be empty string.
if ($proxyIPs === '') {
$proxyIPs = [];
}
$proxyIPs = config(App::class)->proxyIPs;

if (! empty($proxyIPs) && (! is_array($proxyIPs) || is_int(array_key_first($proxyIPs)))) {
throw new ConfigException(
'You must set an array with Proxy IP address key and HTTP header name value in Config\App::$proxyIPs.'
Expand Down
16 changes: 11 additions & 5 deletions tests/system/HTTP/IncomingRequestTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace CodeIgniter\HTTP;

use CodeIgniter\Config\Factories;
use CodeIgniter\Exceptions\ConfigException;
use CodeIgniter\HTTP\Exceptions\HTTPException;
use CodeIgniter\HTTP\Files\UploadedFile;
Expand Down Expand Up @@ -973,7 +974,8 @@ public function testGetIPAddressThruProxy()
'10.0.1.200' => 'X-Forwarded-For',
'192.168.5.0/24' => 'X-Forwarded-For',
];
$this->request = new Request($config);
Factories::injectMock('config', App::class, $config);
$this->request = new Request();
$this->request->populateHeaders();

// we should see the original forwarded address
Expand All @@ -990,7 +992,8 @@ public function testGetIPAddressThruProxyIPv6()
$config->proxyIPs = [
'2001:db8::2:1' => 'X-Forwarded-For',
];
$this->request = new Request($config);
Factories::injectMock('config', App::class, $config);
$this->request = new Request();
$this->request->populateHeaders();

// we should see the original forwarded address
Expand Down Expand Up @@ -1075,7 +1078,8 @@ public function testGetIPAddressThruProxySubnet()

$config = new App();
$config->proxyIPs = ['192.168.5.0/24' => 'X-Forwarded-For'];
$this->request = new Request($config);
Factories::injectMock('config', App::class, $config);
$this->request = new Request();
$this->request->populateHeaders();

// we should see the original forwarded address
Expand All @@ -1090,7 +1094,8 @@ public function testGetIPAddressThruProxySubnetIPv6()

$config = new App();
$config->proxyIPs = ['2001:db8:1234::/48' => 'X-Forwarded-For'];
$this->request = new Request($config);
Factories::injectMock('config', App::class, $config);
$this->request = new Request();
$this->request->populateHeaders();

// we should see the original forwarded address
Expand Down Expand Up @@ -1166,7 +1171,8 @@ public function testGetIPAddressThruProxyInvalidConfigArray()

$config = new App();
$config->proxyIPs = ['192.168.5.0/28'];
$this->request = new Request($config);
Factories::injectMock('config', App::class, $config);
$this->request = new Request();
$this->request->populateHeaders();

$this->request->getIPAddress();
Expand Down
7 changes: 5 additions & 2 deletions tests/system/HTTP/RequestTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace CodeIgniter\HTTP;

use CodeIgniter\Config\Factories;
use CodeIgniter\Test\CIUnitTestCase;
use Config\App;

Expand Down Expand Up @@ -616,7 +617,8 @@ public function testGetIPAddressThruProxy()
'10.0.1.200' => 'X-Forwarded-For',
'192.168.5.0/24' => 'X-Forwarded-For',
];
$this->request = new Request($config);
Factories::injectMock('config', App::class, $config);
$this->request = new Request();
$this->request->populateHeaders();

// we should see the original forwarded address
Expand Down Expand Up @@ -667,7 +669,8 @@ public function testGetIPAddressThruProxySubnet()

$config = new App();
$config->proxyIPs = ['192.168.5.0/24' => 'X-Forwarded-For'];
$this->request = new Request($config);
Factories::injectMock('config', App::class, $config);
$this->request = new Request();
$this->request->populateHeaders();

// we should see the original forwarded address
Expand Down
6 changes: 6 additions & 0 deletions user_guide_src/source/installation/upgrade_440.rst
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,12 @@ Mandatory File Changes
Config Files
============

app/Config/App.php
------------------

- The property ``$proxyIPs`` must be an array. If you don't use proxy servers,
it must be ``public array $proxyIPs = [];``.

.. _upgrade-440-config-routing:

app/Config/Routing.php
Expand Down

0 comments on commit 40409b3

Please sign in to comment.