Merge branch '4.1'

* 4.1: [HttpKernel] fix forwarding trusted headers as server parameters
symfony · Aug 24, 2018 · 4e4b216 · 4e4b216
2 parents c099d86 + 3ac90c1
commit 4e4b216
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 7 deletions.
diff --git a/src/Symfony/Component/HttpFoundation/Request.php b/src/Symfony/Component/HttpFoundation/Request.php
@@ -2010,7 +2010,7 @@ private function normalizeAndFilterClientIps(array $clientIps, $ip)
                 if ($i) {
                     $clientIps[$key] = $clientIp = substr($clientIp, 0, $i);
                 }
-            } elseif ('[' == $clientIp[0]) {
+            } elseif (0 === strpos($clientIp, '[')) {
                 // Strip brackets and :port from IPv6 addresses.
                 $i = strpos($clientIp, ']', 1);
                 $clientIps[$key] = $clientIp = substr($clientIp, 1, $i - 1);

diff --git a/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php b/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php
@@ -906,7 +906,7 @@ public function getClientIpsForwardedProvider()
 
     public function getClientIpsProvider()
     {
-        //        $expected                   $remoteAddr                $httpForwardedFor            $trustedProxies
+        //        $expected                          $remoteAddr                 $httpForwardedFor            $trustedProxies
         return array(
             // simple IPv4
             array(array('88.88.88.88'),              '88.88.88.88',              null,                        null),
@@ -920,8 +920,8 @@ public function getClientIpsProvider()
 
             // forwarded for with remote IPv4 addr not trusted
             array(array('127.0.0.1'),                '127.0.0.1',                '88.88.88.88',               null),
-            // forwarded for with remote IPv4 addr trusted
-            array(array('88.88.88.88'),              '127.0.0.1',                '88.88.88.88',               array('127.0.0.1')),
+            // forwarded for with remote IPv4 addr trusted + comma
+            array(array('88.88.88.88'),              '127.0.0.1',                '88.88.88.88,',              array('127.0.0.1')),
             // forwarded for with remote IPv4 and all FF addrs trusted
             array(array('88.88.88.88'),              '127.0.0.1',                '88.88.88.88',               array('127.0.0.1', '88.88.88.88')),
             // forwarded for with remote IPv4 range trusted

diff --git a/src/Symfony/Component/HttpKernel/HttpCache/SubRequestHandler.php b/src/Symfony/Component/HttpKernel/HttpCache/SubRequestHandler.php
@@ -41,6 +41,7 @@ public static function handle(HttpKernelInterface $kernel, Request $request, $ty
             );
             foreach (array_filter($trustedHeaders) as $name => $key) {
                 $request->headers->remove($name);
+                $request->server->remove('HTTP_'.$name);
             }
         }
 
@@ -59,13 +60,16 @@ public static function handle(HttpKernelInterface $kernel, Request $request, $ty
         // set trusted values, reusing as much as possible the global trusted settings
         if (Request::HEADER_FORWARDED & $trustedHeaderSet) {
             $trustedValues[0] .= sprintf(';host="%s";proto=%s', $request->getHttpHost(), $request->getScheme());
-            $request->headers->set('Forwarded', implode(', ', $trustedValues));
+            $request->headers->set('Forwarded', $v = implode(', ', $trustedValues));
+            $request->server->set('HTTP_FORWARDED', $v);
         }
         if (Request::HEADER_X_FORWARDED_FOR & $trustedHeaderSet) {
-            $request->headers->set('X-Forwarded-For', implode(', ', $trustedIps));
+            $request->headers->set('X-Forwarded-For', $v = implode(', ', $trustedIps));
+            $request->server->set('HTTP_X_FORWARDED_FOR', $v);
         } elseif (!(Request::HEADER_FORWARDED & $trustedHeaderSet)) {
             Request::setTrustedProxies($trustedProxies, $trustedHeaderSet | Request::HEADER_X_FORWARDED_FOR);
-            $request->headers->set('X-Forwarded-For', implode(', ', $trustedIps));
+            $request->headers->set('X-Forwarded-For', $v = implode(', ', $trustedIps));
+            $request->server->set('HTTP_X_FORWARDED_FOR', $v);
         }
 
         // fix the client IP address by setting it to 127.0.0.1,

diff --git a/src/Symfony/Component/HttpKernel/Tests/Fragment/InlineFragmentRendererTest.php b/src/Symfony/Component/HttpKernel/Tests/Fragment/InlineFragmentRendererTest.php
@@ -45,6 +45,8 @@ public function testRenderWithObjectsAsAttributes()
         $subRequest->attributes->replace(array('object' => $object, '_format' => 'html', '_controller' => 'main_controller', '_locale' => 'en'));
         $subRequest->headers->set('x-forwarded-for', array('127.0.0.1'));
         $subRequest->headers->set('forwarded', array('for="127.0.0.1";host="localhost";proto=http'));
+        $subRequest->server->set('HTTP_X_FORWARDED_FOR', '127.0.0.1');
+        $subRequest->server->set('HTTP_FORWARDED', 'for="127.0.0.1";host="localhost";proto=http');
 
         $strategy = new InlineFragmentRenderer($this->getKernelExpectingRequest($subRequest));
 
@@ -57,6 +59,7 @@ public function testRenderWithTrustedHeaderDisabled()
 
         $expectedSubRequest = Request::create('/');
         $expectedSubRequest->headers->set('x-forwarded-for', array('127.0.0.1'));
+        $expectedSubRequest->server->set('HTTP_X_FORWARDED_FOR', '127.0.0.1');
 
         $strategy = new InlineFragmentRenderer($this->getKernelExpectingRequest($expectedSubRequest));
         $this->assertSame('foo', $strategy->render('/', Request::create('/'))->getContent());
@@ -149,8 +152,10 @@ public function testESIHeaderIsKeptInSubrequest()
 
         if (Request::HEADER_X_FORWARDED_FOR & Request::getTrustedHeaderSet()) {
             $expectedSubRequest->headers->set('x-forwarded-for', array('127.0.0.1'));
+            $expectedSubRequest->server->set('HTTP_X_FORWARDED_FOR', '127.0.0.1');
         }
         $expectedSubRequest->headers->set('forwarded', array('for="127.0.0.1";host="localhost";proto=http'));
+        $expectedSubRequest->server->set('HTTP_FORWARDED', 'for="127.0.0.1";host="localhost";proto=http');
 
         $strategy = new InlineFragmentRenderer($this->getKernelExpectingRequest($expectedSubRequest));
 
@@ -173,6 +178,8 @@ public function testHeadersPossiblyResultingIn304AreNotAssignedToSubrequest()
         $expectedSubRequest = Request::create('/');
         $expectedSubRequest->headers->set('x-forwarded-for', array('127.0.0.1'));
         $expectedSubRequest->headers->set('forwarded', array('for="127.0.0.1";host="localhost";proto=http'));
+        $expectedSubRequest->server->set('HTTP_X_FORWARDED_FOR', '127.0.0.1');
+        $expectedSubRequest->server->set('HTTP_FORWARDED', 'for="127.0.0.1";host="localhost";proto=http');
 
         $strategy = new InlineFragmentRenderer($this->getKernelExpectingRequest($expectedSubRequest));
         $request = Request::create('/', 'GET', array(), array(), array(), array('HTTP_IF_MODIFIED_SINCE' => 'Fri, 01 Jan 2016 00:00:00 GMT', 'HTTP_IF_NONE_MATCH' => '*'));
@@ -188,6 +195,8 @@ public function testFirstTrustedProxyIsSetAsRemote()
         $expectedSubRequest->server->set('REMOTE_ADDR', '127.0.0.1');
         $expectedSubRequest->headers->set('x-forwarded-for', array('127.0.0.1'));
         $expectedSubRequest->headers->set('forwarded', array('for="127.0.0.1";host="localhost";proto=http'));
+        $expectedSubRequest->server->set('HTTP_X_FORWARDED_FOR', '127.0.0.1');
+        $expectedSubRequest->server->set('HTTP_FORWARDED', 'for="127.0.0.1";host="localhost";proto=http');
 
         $strategy = new InlineFragmentRenderer($this->getKernelExpectingRequest($expectedSubRequest));
 
@@ -205,6 +214,8 @@ public function testIpAddressOfRangedTrustedProxyIsSetAsRemote()
         $expectedSubRequest->server->set('REMOTE_ADDR', '127.0.0.1');
         $expectedSubRequest->headers->set('x-forwarded-for', array('127.0.0.1'));
         $expectedSubRequest->headers->set('forwarded', array('for="127.0.0.1";host="localhost";proto=http'));
+        $expectedSubRequest->server->set('HTTP_X_FORWARDED_FOR', '127.0.0.1');
+        $expectedSubRequest->server->set('HTTP_FORWARDED', 'for="127.0.0.1";host="localhost";proto=http');
 
         Request::setTrustedProxies(array('1.1.1.1/24'), -1);