Fix #17755: Fix a bug for web request with trustedHosts set to format ['10.0.0.1' => ['X-Forwarded-For']]

framework/CHANGELOG.md

@@ -7,6 +7,7 @@ Yii Framework 2 Change Log
 - Bug #17037: Fix uploaded file saving method in case generated by `MultipartFormDataParser` (sup-ham)
 - Bug #17803: Fixed ErrorHandler unregister and register only change global state when applicable (SamMousa)
 - Bug #17744: Fix a bug with setting incorrect `defaultValue` to AR column with `CURRENT_TIMESTAMP(x)` as default expression (MySQL >= 5.6.4) (bizley)
+- Bug #17755: Fix a bug for web request with `trustedHosts` set to format `['10.0.0.1' => ['X-Forwarded-For']]` (shushenghong)
 - Enh #17729: Path alias support was added to `yii\web\UploadFile::saveAs()` (sup-ham)
 - Bug #17749: Dispatcher fix if target crashed in PHP 7.0+ (kamarton)
 - Bug #17762: PHP 7.4: Remove special condition for converting PHP errors to exceptions if they occurred inside of `__toString()` call (rob006)

framework/web/Request.php

@@ -1866,8 +1866,15 @@ class Request extends \yii\base\Request
     protected function getSecureForwardedHeaderTrustedParts()
     {
         $validator = $this->getIpValidator();
+        $trustedHosts = [];
+        foreach ($this->trustedHosts as $trustedCidr => $trustedCidrOrHeaders) {
+            if (!is_array($trustedCidrOrHeaders)) {
+                $trustedCidr = $trustedCidrOrHeaders;
+            }
+            $trustedHosts[] = $trustedCidr;
+        }
+        $validator->setRanges($trustedHosts);
 
-        $validator->setRanges($this->trustedHosts);
         return array_filter($this->getSecureForwardedHeaderParts(), function ($headerPart) use ($validator) {
             return isset($headerPart['for']) ? !$validator->validate($headerPart['for']) : true;
         });

tests/framework/web/RequestTest.php

@@ -393,6 +393,23 @@ class RequestTest extends TestCase
             ],
         ]);
 
+
+        $this->assertEquals($expected[0], $request->getHostInfo());
+        $this->assertEquals($expected[1], $request->getHostName());
+
+        $request = new Request([
+            'trustedHosts' => [
+                '192.168.0.0/24' => ['X-Forwarded-Host', 'forwarded'],
+            ],
+            'secureHeaders' => [
+                'X-Forwarded-For',
+                'X-Forwarded-Host',
+                'X-Forwarded-Proto',
+                'forwarded',
+            ],
+        ]);
+
+
         $this->assertEquals($expected[0], $request->getHostInfo());
         $this->assertEquals($expected[1], $request->getHostName());
         $_SERVER = $original;
@@ -549,6 +566,8 @@ class RequestTest extends TestCase
     public function testGetIsSecureConnection($server, $expected)
     {
         $original = $_SERVER;
+        $_SERVER = $server;
+
         $request = new Request([
             'trustedHosts' => [
                 '192.168.0.0/24',
@@ -562,9 +581,62 @@ class RequestTest extends TestCase
                 'forwarded',
             ],
         ]);
+        $this->assertEquals($expected, $request->getIsSecureConnection());
+
+        $request = new Request([
+            'trustedHosts' => [
+                '192.168.0.0/24' => ['Front-End-Https', 'X-Forwarded-Proto', 'forwarded'],
+            ],
+            'secureHeaders' => [
+                'Front-End-Https',
+                'X-Rewrite-Url',
+                'X-Forwarded-For',
+                'X-Forwarded-Host',
+                'X-Forwarded-Proto',
+                'forwarded',
+            ],
+        ]);
+        $this->assertEquals($expected, $request->getIsSecureConnection());
+
+        $_SERVER = $original;
+    }
+
+    public function isSecureServerWithoutTrustedHostDataProvider()
+    {
+        return [
+            // RFC 7239 forwarded header is not enabled
+            [[
+                'HTTP_FORWARDED' => 'proto=https',
+                'REMOTE_ADDR' => '192.168.0.1',
+            ], false],
+        ];
+    }
+
+    /**
+     * @dataProvider isSecureServerWithoutTrustedHostDataProvider
+     * @param array $server
+     * @param bool $expected
+     */
+    public function testGetIsSecureConnectionWithoutTrustedHost($server, $expected)
+    {
+        $original = $_SERVER;
         $_SERVER = $server;
 
+        $request = new Request([
+            'trustedHosts' => [
+                '192.168.0.0/24' => ['Front-End-Https', 'X-Forwarded-Proto'],
+            ],
+            'secureHeaders' => [
+                'Front-End-Https',
+                'X-Rewrite-Url',
+                'X-Forwarded-For',
+                'X-Forwarded-Host',
+                'X-Forwarded-Proto',
+                'forwarded',
+            ],
+        ]);
         $this->assertEquals($expected, $request->getIsSecureConnection());
+
         $_SERVER = $original;
     }
 
@@ -726,11 +798,68 @@ class RequestTest extends TestCase
                 'forwarded',
             ],
         ]);
-
         $this->assertEquals($expected, $request->getUserIP());
+
+        $request = new Request([
+            'trustedHosts' => [
+                '192.168.0.0/24' => ['X-Forwarded-For', 'forwarded'],
+            ],
+            'secureHeaders' => [
+                'Front-End-Https',
+                'X-Rewrite-Url',
+                'X-Forwarded-For',
+                'X-Forwarded-Host',
+                'X-Forwarded-Proto',
+                'forwarded',
+            ],
+        ]);
+        $this->assertEquals($expected, $request->getUserIP());
+
         $_SERVER = $original;
     }
 
+    public function getUserIPWithoutTruestHostDataProvider()
+    {
+        return [
+            // RFC 7239 forwarded is not enabled
+            [
+                [
+                    'HTTP_FORWARDED' => 'for=123.123.123.123',
+                    'REMOTE_ADDR' => '192.168.0.1',
+                ],
+                '192.168.0.1',
+            ],
+        ];
+    }
+
+    /**
+    * @dataProvider getUserIPWithoutTruestHostDataProvider
+    * @param array $server
+    * @param string $expected
+    */
+   public function testGetUserIPWithoutTrustedHost($server, $expected)
+   {
+       $original = $_SERVER;
+       $_SERVER = $server;
+
+       $request = new Request([
+           'trustedHosts' => [
+               '192.168.0.0/24' => ['X-Forwarded-For'],
+           ],
+           'secureHeaders' => [
+               'Front-End-Https',
+               'X-Rewrite-Url',
+               'X-Forwarded-For',
+               'X-Forwarded-Host',
+               'X-Forwarded-Proto',
+               'forwarded',
+           ],
+       ]);
+       $this->assertEquals($expected, $request->getUserIP());
+
+       $_SERVER = $original;
+   }
+
     public function getMethodDataProvider()
     {
         return [