Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: drop support for Config\App::$proxyIPs = '' #7621

Merged
merged 6 commits into from
Jun 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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