diff --git a/CHANGELOG.md b/CHANGELOG.md index 5776fd9..a41651c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## 1.0.3 under development +- Bug #59: Parse comma-separated IP chains in non-RFC forwarded headers (@samdark) - Chg #44: Change PHP constraint in `composer.json` to `8.0 - 8.4` (@vjik) - Enh #55: Explicitly import constants in "use" section (@vjik) diff --git a/README.md b/README.md index c6cb84c..307b251 100644 --- a/README.md +++ b/README.md @@ -148,6 +148,9 @@ one data unit (for example, IP). Headers with "X" prefix are quite common despit The header groups are processed in the order they are defined. If the header containing IP is present and is non-empty, this group will be selected and further ones will be ignored. +IP headers in array-based groups are parsed as comma-separated lists. Multiple values of the same header are combined +in their original order before the connection chain is resolved. + You can add support for custom headers and/or change priority: ```php diff --git a/src/TrustedHostsNetworkResolver.php b/src/TrustedHostsNetworkResolver.php index 2609304..847e0b3 100644 --- a/src/TrustedHostsNetworkResolver.php +++ b/src/TrustedHostsNetworkResolver.php @@ -547,9 +547,8 @@ private function getConnectionChainItems(string $remoteAddr, ServerRequestInterf break; } - /** @psalm-var list $forwardedHeaderValue */ - $forwardedHeaderValue = $request->getHeader($forwardedHeaderGroup['ip']); - if (empty($forwardedHeaderValue) || empty($request->getHeaderLine($forwardedHeaderGroup['ip']))) { + $forwardedHeaderValue = $request->getHeaderLine($forwardedHeaderGroup['ip']); + if ($forwardedHeaderValue === '') { continue; } @@ -557,7 +556,10 @@ private function getConnectionChainItems(string $remoteAddr, ServerRequestInterf /** * @psalm-var non-empty-list $requestIps It needs for PHP 8.0 only */ - $requestIps = array_merge([$remoteAddr], array_reverse($forwardedHeaderValue)); + $requestIps = array_merge( + [$remoteAddr], + array_reverse(array_map('\trim', explode(',', $forwardedHeaderValue))), + ); foreach ($requestIps as $requestIp) { $items[] = $this->getConnectionChainItem( ip: $requestIp, diff --git a/tests/TrustedHostsNetworkResolver/ProcessTest.php b/tests/TrustedHostsNetworkResolver/ProcessTest.php index 4673c51..202757c 100644 --- a/tests/TrustedHostsNetworkResolver/ProcessTest.php +++ b/tests/TrustedHostsNetworkResolver/ProcessTest.php @@ -444,6 +444,84 @@ public function dataProcess(): iterable ], ], ], + yield 'headers with "X" prefix, comma-separated IPs' => [ + $this + ->createMiddleware() + ->withTrustedIps(['8.8.8.8', '5.5.5.5', '2.2.2.2', '18.18.18.18']) + ->withConnectionChainItemsAttribute('connectionChainItems'), + $this->createRequest( + headers: ['X-Forwarded-For' => '9.9.9.9, 7.7.7.7, 5.5.5.5, 2.2.2.2'], + serverParams: ['REMOTE_ADDR' => '18.18.18.18'], + ), + [ + 'requestClientIp' => '7.7.7.7', + 'connectionChainItemsAttribute' => [ + 'connectionChainItems', + [ + [ + 'ip' => '18.18.18.18', + 'protocol' => null, + 'host' => null, + 'port' => null, + 'ipIdentifier' => null, + ], + [ + 'ip' => '2.2.2.2', + 'protocol' => null, + 'host' => null, + 'port' => null, + 'ipIdentifier' => null, + ], + [ + 'ip' => '5.5.5.5', + 'protocol' => null, + 'host' => null, + 'port' => null, + 'ipIdentifier' => null, + ], + ], + ], + ], + ], + yield 'headers with "X" prefix, multiple comma-separated header values' => [ + $this + ->createMiddleware() + ->withTrustedIps(['8.8.8.8', '5.5.5.5', '2.2.2.2', '18.18.18.18']) + ->withConnectionChainItemsAttribute('connectionChainItems'), + $this->createRequest( + headers: ['X-Forwarded-For' => ['9.9.9.9, 7.7.7.7', '5.5.5.5, 2.2.2.2']], + serverParams: ['REMOTE_ADDR' => '18.18.18.18'], + ), + [ + 'requestClientIp' => '7.7.7.7', + 'connectionChainItemsAttribute' => [ + 'connectionChainItems', + [ + [ + 'ip' => '18.18.18.18', + 'protocol' => null, + 'host' => null, + 'port' => null, + 'ipIdentifier' => null, + ], + [ + 'ip' => '2.2.2.2', + 'protocol' => null, + 'host' => null, + 'port' => null, + 'ipIdentifier' => null, + ], + [ + 'ip' => '5.5.5.5', + 'protocol' => null, + 'host' => null, + 'port' => null, + 'ipIdentifier' => null, + ], + ], + ], + ], + ], yield 'RFC header, contains IP from private network, IPv4' => [ $this ->createMiddleware() @@ -821,7 +899,7 @@ public function dataProcess(): iterable 'port' => 8080, ], ], - yield 'custom headers, highest priority, IP related data' => [ + yield 'custom headers, highest priority, comma-separated IP related data' => [ $this ->createMiddleware() ->withTrustedIps(['8.8.8.8', '2.2.2.2', '18.18.18.18']) @@ -847,7 +925,7 @@ public function dataProcess(): iterable 'X-Forwarded-Proto' => ['http'], 'X-Forwarded-Host' => ['example2.com'], 'X-Forwarded-Port' => ['8020'], - 'Y-Forwarded-For' => ['9.9.9.9', '5.5.5.5', '2.2.2.2'], + 'Y-Forwarded-For' => '9.9.9.9, 5.5.5.5, 2.2.2.2', 'Y-Forwarded-Proto' => ['https'], 'Y-Forwarded-Host' => ['example3.com'], 'Y-Forwarded-Port' => ['8030'], diff --git a/tests/TrustedHostsNetworkResolver/RuntimeExceptionTest.php b/tests/TrustedHostsNetworkResolver/RuntimeExceptionTest.php index a3a7138..3b06c45 100644 --- a/tests/TrustedHostsNetworkResolver/RuntimeExceptionTest.php +++ b/tests/TrustedHostsNetworkResolver/RuntimeExceptionTest.php @@ -191,6 +191,16 @@ public function dataInvalidConnectionChainItemException(): iterable ), '"invalid5.5.5.5" is not a valid IP.', ], + yield 'IP, empty item in comma-separated header with "X" prefix' => [ + $this->createMiddleware()->withTrustedIps(['8.8.8.8', '2.2.2.2', '18.18.18.18']), + $this->createRequest( + headers: [ + 'X-Forwarded-For' => '9.9.9.9, , 2.2.2.2', + ], + serverParams: ['REMOTE_ADDR' => '18.18.18.18'], + ), + '"" is not a valid IP.', + ], yield 'IP with port, headers with "X" prefix' => [ $this->createMiddleware()->withTrustedIps(['8.8.8.8', '2.2.2.2', '18.18.18.18']), $this->createRequest(