From d3c960493ce4f2cc957744d2fbd8c0b3d8df642b Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sun, 12 Feb 2017 12:31:06 +0100 Subject: [PATCH] [HttpFoundation] Add $trustedHeaderSet arg to Request::setTrustedProxies() - deprecate not setting it --- UPGRADE-3.3.md | 19 ++- UPGRADE-4.0.md | 11 ++ .../Tests/Processor/WebProcessorTest.php | 2 +- src/Symfony/Bridge/Monolog/composer.json | 3 + .../Bundle/FrameworkBundle/CHANGELOG.md | 1 + .../DependencyInjection/Configuration.php | 9 ++ .../FrameworkBundle/FrameworkBundle.php | 4 +- .../DependencyInjection/ConfigurationTest.php | 3 + .../DependencyInjection/Fixtures/php/full.php | 1 - .../DependencyInjection/Fixtures/xml/full.xml | 2 +- .../DependencyInjection/Fixtures/yml/full.yml | 1 - .../FrameworkExtensionTest.php | 7 - .../Component/HttpFoundation/CHANGELOG.md | 2 + .../Component/HttpFoundation/Request.php | 76 +++++++++- .../HttpFoundation/Tests/RequestTest.php | 140 ++++++++++++++---- .../Fragment/InlineFragmentRenderer.php | 2 +- .../HttpKernel/HttpCache/HttpCache.php | 2 +- .../ValidateRequestListenerTest.php | 2 +- .../Fragment/InlineFragmentRendererTest.php | 25 +--- .../Tests/HttpCache/HttpCacheTest.php | 2 +- .../HttpKernel/Tests/HttpKernelTest.php | 2 +- 21 files changed, 243 insertions(+), 73 deletions(-) diff --git a/UPGRADE-3.3.md b/UPGRADE-3.3.md index beeeea4c7ffab..a34f920857f13 100644 --- a/UPGRADE-3.3.md +++ b/UPGRADE-3.3.md @@ -126,6 +126,9 @@ FrameworkBundle * The `cache:clear` command should always be called with the `--no-warmup` option. Warmup should be done via the `cache:warmup` command. + * The "framework.trusted_proxies" configuration option and the corresponding "kernel.trusted_proxies" parameter have been deprecated and will be removed in 4.0. Use the Request::setTrustedProxies() method in your front controller instead. + + * The `Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\AddConsoleCommandPass` has been deprecated. Use `Symfony\Component\Console\DependencyInjection\AddConsoleCommandPass` instead. * The `Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\SerializerPass` class has been @@ -175,14 +178,24 @@ FrameworkBundle class has been deprecated and will be removed in 4.0. Use the `Symfony\Component\Routing\DependencyInjection\RoutingResolverPass` class instead. - * The `server:run`, `server:start`, `server:stop` and - `server:status` console commands have been moved to a dedicated bundle. - Require `symfony/web-server-bundle` in your composer.json and register + * The `server:run`, `server:start`, `server:stop` and + `server:status` console commands have been moved to a dedicated bundle. + Require `symfony/web-server-bundle` in your composer.json and register `Symfony\Bundle\WebServerBundle\WebServerBundle` in your AppKernel to use them. * The `Symfony\Bundle\FrameworkBundle\Translation\Translator` constructor now takes the default locale as 3rd argument. Not passing it will trigger an error in 4.0. +HttpFoundation +-------------- + + * The `Request::setTrustedProxies()` method takes a new `$trustedHeaderSet` argument - not setting it is deprecated. + Set it to `Request::HEADER_FORWARDED` if your reverse-proxy uses the RFC7239 `Forwarded` header, + or to `Request::HEADER_X_FORWARDED_ALL` if it is using `X-Forwarded-*` headers instead. + + * The `Request::setTrustedHeaderName()` and `Request::getTrustedHeaderName()` methods are deprecated, + use the RFC7239 `Forwarded` header, or the `X-Forwarded-*` headers instead. + HttpKernel ----------- diff --git a/UPGRADE-4.0.md b/UPGRADE-4.0.md index 7c9d0a29ff5eb..65b0ae75d6278 100644 --- a/UPGRADE-4.0.md +++ b/UPGRADE-4.0.md @@ -190,6 +190,8 @@ FrameworkBundle * The `cache:clear` command does not warmup the cache anymore. Warmup should be done via the `cache:warmup` command. + * The "framework.trusted_proxies" configuration option and the corresponding "kernel.trusted_proxies" parameter have been removed. Use the `Request::setTrustedProxies()` method in your front controller instead. + * Support for absolute template paths has been removed. * The following form types registered as services have been removed; use their @@ -280,6 +282,15 @@ FrameworkBundle HttpFoundation --------------- +HttpFoundation +-------------- + + * The `Request::setTrustedProxies()` method takes a new `$trustedHeaderSet` argument. + Set it to `Request::HEADER_FORWARDED` if your reverse-proxy uses the RFC7239 `Forwarded` header, + or to `Request::HEADER_X_FORWARDED_ALL` if it is using `X-Forwarded-*` headers instead. + + * The `Request::setTrustedHeaderName()` and `Request::getTrustedHeaderName()` methods have been removed. + * Extending the following methods of `Response` is no longer possible (these methods are now `final`): diff --git a/src/Symfony/Bridge/Monolog/Tests/Processor/WebProcessorTest.php b/src/Symfony/Bridge/Monolog/Tests/Processor/WebProcessorTest.php index 51bddd1d9828c..e73002e0292fa 100644 --- a/src/Symfony/Bridge/Monolog/Tests/Processor/WebProcessorTest.php +++ b/src/Symfony/Bridge/Monolog/Tests/Processor/WebProcessorTest.php @@ -36,7 +36,7 @@ public function testUsesRequestServerData() public function testUseRequestClientIp() { - Request::setTrustedProxies(array('192.168.0.1')); + Request::setTrustedProxies(array('192.168.0.1'), Request::HEADER_X_FORWARDED_ALL); list($event, $server) = $this->createRequestEvent(array('X_FORWARDED_FOR' => '192.168.0.2')); $processor = new WebProcessor(); diff --git a/src/Symfony/Bridge/Monolog/composer.json b/src/Symfony/Bridge/Monolog/composer.json index acb09396a51ab..3c3cc8873459a 100644 --- a/src/Symfony/Bridge/Monolog/composer.json +++ b/src/Symfony/Bridge/Monolog/composer.json @@ -25,6 +25,9 @@ "symfony/event-dispatcher": "~2.8|~3.0", "symfony/var-dumper": "~3.3" }, + "conflict": { + "symfony/http-foundation": "<3.3" + }, "suggest": { "symfony/http-kernel": "For using the debugging handlers together with the response life cycle of the HTTP kernel.", "symfony/console": "For the possibility to show log messages in console commands depending on verbosity settings. You need version ~2.3 of the console for it.", diff --git a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md index 568ee6120dbf4..2c73aad5ddb9f 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md @@ -5,6 +5,7 @@ CHANGELOG ----- * Deprecated `cache:clear` with warmup (always call it with `--no-warmup`) + * Deprecated the "framework.trusted_proxies" configuration option and the corresponding "kernel.trusted_proxies" parameter * Changed default configuration for assets/forms/validation/translation/serialization/csrf from `canBeEnabled()` to `canBeDisabled()` when Flex is used diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index 9ccbb4ed29a83..8f63bd12cfa7d 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -18,6 +18,7 @@ use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; use Symfony\Component\Form\Form; +use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Serializer\Serializer; use Symfony\Component\Translation\Translator; use Symfony\Component\Validator\Validation; @@ -58,6 +59,14 @@ public function getConfigTreeBuilder() return $v; }) ->end() + ->beforeNormalization() + ->ifTrue(function ($v) { return isset($v['trusted_proxies']); }) + ->then(function ($v) { + @trigger_error('The "framework.trusted_proxies" configuration key is deprecated since version 3.3 and will be removed in 4.0. Use the Request::setTrustedProxies() method in your front controller instead.', E_USER_DEPRECATED); + + return $v; + }) + ->end() ->children() ->scalarNode('secret')->end() ->scalarNode('http_method_override') diff --git a/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php b/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php index fa4951e2da58b..fde898503b507 100644 --- a/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php +++ b/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php @@ -60,7 +60,9 @@ public function boot() ErrorHandler::register(null, false)->throwAt($this->container->getParameter('debug.error_handler.throw_at'), true); if ($trustedProxies = $this->container->getParameter('kernel.trusted_proxies')) { - Request::setTrustedProxies($trustedProxies); + @trigger_error('The "kernel.trusted_proxies" parameter is deprecated since version 3.3 and will be removed in 4.0. Use the Request::setTrustedProxies() method in your front controller instead.', E_USER_DEPRECATED); + + Request::setTrustedProxies($trustedProxies, Request::getTrustedHeaderSet()); } if ($this->container->getParameter('kernel.http_method_override')) { diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php index 3d101d030a9a5..6c7a1fd757052 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php @@ -43,6 +43,7 @@ public function testDoNoDuplicateDefaultFormResources() } /** + * @group legacy * @dataProvider getTestValidTrustedProxiesData */ public function testValidTrustedProxies($trustedProxies, $processedProxies) @@ -73,6 +74,7 @@ public function getTestValidTrustedProxiesData() } /** + * @group legacy * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException */ public function testInvalidTypeTrustedProxies() @@ -88,6 +90,7 @@ public function testInvalidTypeTrustedProxies() } /** + * @group legacy * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException */ public function testInvalidValueTrustedProxies() diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/full.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/full.php index 9173dd5615bda..58a4340e6979f 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/full.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/full.php @@ -10,7 +10,6 @@ ), ), 'http_method_override' => false, - 'trusted_proxies' => array('127.0.0.1', '10.0.0.1'), 'esi' => array( 'enabled' => true, ), diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/full.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/full.xml index 7c6b11ac44aa6..5c6a221c18489 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/full.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/full.xml @@ -6,7 +6,7 @@ xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd http://symfony.com/schema/dic/symfony http://symfony.com/schema/dic/symfony/symfony-1.0.xsd"> - + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/full.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/full.yml index 237654880c984..f471372097c15 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/full.yml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/full.yml @@ -6,7 +6,6 @@ framework: csrf_protection: field_name: _csrf http_method_override: false - trusted_proxies: ['127.0.0.1', '10.0.0.1'] esi: enabled: true profiler: diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index a55abfd30a4b8..53ff5e54bea16 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -119,13 +119,6 @@ public function testCsrfProtectionForFormsEnablesCsrfProtectionAutomatically() $this->assertTrue($container->hasDefinition('security.csrf.token_manager')); } - public function testProxies() - { - $container = $this->createContainerFromFile('full'); - - $this->assertEquals(array('127.0.0.1', '10.0.0.1'), $container->getParameter('kernel.trusted_proxies')); - } - public function testHttpMethodOverride() { $container = $this->createContainerFromFile('full'); diff --git a/src/Symfony/Component/HttpFoundation/CHANGELOG.md b/src/Symfony/Component/HttpFoundation/CHANGELOG.md index 9acffbbe7d342..38c9a153dc823 100644 --- a/src/Symfony/Component/HttpFoundation/CHANGELOG.md +++ b/src/Symfony/Component/HttpFoundation/CHANGELOG.md @@ -4,6 +4,8 @@ CHANGELOG 3.3.0 ----- + * added `$trustedHeaderSet` argument to `Request::setTrustedProxies()` - deprecate not setting it, + * deprecated the `Request::setTrustedHeaderName()` and `Request::getTrustedHeaderName()` methods, * added `File\Stream`, to be passed to `BinaryFileResponse` when the size of the served file is unknown, disabling `Range` and `Content-Length` handling, switching to chunked encoding instead * added the `Cookie::fromString()` method that allows to create a cookie from a diff --git a/src/Symfony/Component/HttpFoundation/Request.php b/src/Symfony/Component/HttpFoundation/Request.php index 5ddadf4d0d2e3..6d43de0430971 100644 --- a/src/Symfony/Component/HttpFoundation/Request.php +++ b/src/Symfony/Component/HttpFoundation/Request.php @@ -30,11 +30,21 @@ */ class Request { - const HEADER_FORWARDED = 'forwarded'; - const HEADER_CLIENT_IP = 'client_ip'; - const HEADER_CLIENT_HOST = 'client_host'; - const HEADER_CLIENT_PROTO = 'client_proto'; - const HEADER_CLIENT_PORT = 'client_port'; + const HEADER_FORWARDED = 0b00001; + const HEADER_X_FORWARDED_ALL = 0b11110; + const HEADER_X_FORWARDED_FOR = 2; + const HEADER_X_FORWARDED_HOST = 4; + const HEADER_X_FORWARDED_PROTO = 8; + const HEADER_X_FORWARDED_PORT = 16; + + /** @deprecated since version 3.3, to be removed in 4.0 */ + const HEADER_CLIENT_IP = self::HEADER_X_FORWARDED_FOR; + /** @deprecated since version 3.3, to be removed in 4.0 */ + const HEADER_CLIENT_HOST = self::HEADER_X_FORWARDED_HOST; + /** @deprecated since version 3.3, to be removed in 4.0 */ + const HEADER_CLIENT_PROTO = self::HEADER_X_FORWARDED_PROTO; + /** @deprecated since version 3.3, to be removed in 4.0 */ + const HEADER_CLIENT_PORT = self::HEADER_X_FORWARDED_PORT; const METHOD_HEAD = 'HEAD'; const METHOD_GET = 'GET'; @@ -70,6 +80,8 @@ class Request * * The other headers are non-standard, but widely used * by popular reverse proxies (like Apache mod_proxy or Amazon EC2). + * + * @deprecated since version 3.3, to be removed in 4.0 */ protected static $trustedHeaders = array( self::HEADER_FORWARDED => 'FORWARDED', @@ -210,6 +222,17 @@ class Request private $isHostValid = true; private $isClientIpsValid = true; + private static $trustedHeaderSet = -1; + + /** @deprecated since version 3.3, to be removed in 4.0 */ + private static $trustedHeaderNames = array( + self::HEADER_FORWARDED => 'FORWARDED', + self::HEADER_CLIENT_IP => 'X_FORWARDED_FOR', + self::HEADER_CLIENT_HOST => 'X_FORWARDED_HOST', + self::HEADER_CLIENT_PROTO => 'X_FORWARDED_PROTO', + self::HEADER_CLIENT_PORT => 'X_FORWARDED_PORT', + ); + /** * Constructor. * @@ -548,11 +571,26 @@ public function overrideGlobals() * * You should only list the reverse proxies that you manage directly. * - * @param array $proxies A list of trusted proxies + * @param array $proxies A list of trusted proxies + * @param int $trustedHeaderSet A bit field of Request::HEADER_*, usually either Request::HEADER_FORWARDED or Request::HEADER_X_FORWARDED_ALL, to set which headers to trust from your proxies + * + * @throws \InvalidArgumentException When $trustedHeaderSet is invalid */ - public static function setTrustedProxies(array $proxies) + public static function setTrustedProxies(array $proxies/*, int $trustedHeaderSet*/) { self::$trustedProxies = $proxies; + + if (2 > func_num_args()) { + @trigger_error(sprintf('The %s() method expects a bit field of Request::HEADER_* as second argument. Not defining it is deprecated since version 3.3 and will be required in 4.0.', __METHOD__), E_USER_DEPRECATED); + + return; + } + $trustedHeaderSet = func_get_arg(1); + + foreach (self::$trustedHeaderNames as $header => $name) { + self::$trustedHeaders[$header] = $header & $trustedHeaderSet ? $name : null; + } + self::$trustedHeaderSet = $trustedHeaderSet; } /** @@ -565,6 +603,16 @@ public static function getTrustedProxies() return self::$trustedProxies; } + /** + * Gets the set of trusted headers from trusted proxies. + * + * @return int A bit field of Request::HEADER_* that defines which headers are trusted from your proxies + */ + public static function getTrustedHeaderSet() + { + return self::$trustedHeaderSet; + } + /** * Sets a list of trusted host patterns. * @@ -608,14 +656,22 @@ public static function getTrustedHosts() * @param string $value The header name * * @throws \InvalidArgumentException + * + * @deprecated since version 3.3, to be removed in 4.0. Use "X-Forwarded-*" headers or the "Forwarded" header defined in RFC7239, and the $trustedHeaderSet argument of the Request::setTrustedProxies() method instead. */ public static function setTrustedHeaderName($key, $value) { + @trigger_error(sprintf('The "%s()" method is deprecated since version 3.3 and will be removed in 4.0. Use "X-Forwarded-*" headers or the "Forwarded" header defined in RFC7239, and the $trustedHeaderSet argument of the Request::setTrustedProxies() method instead.', __METHOD__), E_USER_DEPRECATED); + if (!array_key_exists($key, self::$trustedHeaders)) { throw new \InvalidArgumentException(sprintf('Unable to set the trusted header name for key "%s".', $key)); } self::$trustedHeaders[$key] = $value; + + if (null !== $value) { + self::$trustedHeaderNames[$key] = $value; + } } /** @@ -626,9 +682,15 @@ public static function setTrustedHeaderName($key, $value) * @return string The header name * * @throws \InvalidArgumentException + * + * @deprecated since version 3.3, to be removed in 4.0. Use the Request::getTrustedHeaderSet() method instead. */ public static function getTrustedHeaderName($key) { + if (2 > func_num_args() || func_get_arg(1)) { + @trigger_error(sprintf('The "%s()" method is deprecated since version 3.3 and will be removed in 4.0. Use the Request::getTrustedHeaderSet() method instead.', __METHOD__), E_USER_DEPRECATED); + } + if (!array_key_exists($key, self::$trustedHeaders)) { throw new \InvalidArgumentException(sprintf('Unable to get the trusted header name for key "%s".', $key)); } diff --git a/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php b/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php index 0eb2f7b510632..7321ca4cdeb07 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php +++ b/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php @@ -19,6 +19,12 @@ class RequestTest extends TestCase { + protected function tearDown() + { + // reset + Request::setTrustedProxies(array(), -1); + } + public function testInitialize() { $request = new Request(); @@ -727,7 +733,7 @@ public function testGetPort() $this->assertEquals(80, $port, 'Without trusted proxies FORWARDED_PROTO and FORWARDED_PORT are ignored.'); - Request::setTrustedProxies(array('1.1.1.1')); + Request::setTrustedProxies(array('1.1.1.1'), Request::HEADER_X_FORWARDED_ALL); $request = Request::create('http://example.com', 'GET', array(), array(), array(), array( 'HTTP_X_FORWARDED_PROTO' => 'https', 'HTTP_X_FORWARDED_PORT' => '8443', @@ -769,8 +775,6 @@ public function testGetPort() )); $port = $request->getPort(); $this->assertEquals(80, $port, 'With only PROTO set and value is not recognized, getPort() defaults to 80.'); - - Request::setTrustedProxies(array()); } /** @@ -846,8 +850,6 @@ public function testGetClientIp($expected, $remoteAddr, $httpForwardedFor, $trus $request = $this->getRequestInstanceForClientIpTests($remoteAddr, $httpForwardedFor, $trustedProxies); $this->assertEquals($expected[0], $request->getClientIp()); - - Request::setTrustedProxies(array()); } /** @@ -858,8 +860,6 @@ public function testGetClientIps($expected, $remoteAddr, $httpForwardedFor, $tru $request = $this->getRequestInstanceForClientIpTests($remoteAddr, $httpForwardedFor, $trustedProxies); $this->assertEquals($expected, $request->getClientIps()); - - Request::setTrustedProxies(array()); } /** @@ -870,8 +870,6 @@ public function testGetClientIpsForwarded($expected, $remoteAddr, $httpForwarded $request = $this->getRequestInstanceForClientIpsForwardedTests($remoteAddr, $httpForwarded, $trustedProxies); $this->assertEquals($expected, $request->getClientIps()); - - Request::setTrustedProxies(array()); } public function testGetClientIpsForwardedProvider() @@ -956,7 +954,7 @@ public function testGetClientIpsWithConflictingHeaders($httpForwarded, $httpXFor 'HTTP_X_FORWARDED_FOR' => $httpXForwardedFor, ); - Request::setTrustedProxies(array('88.88.88.88')); + Request::setTrustedProxies(array('88.88.88.88'), Request::HEADER_X_FORWARDED_ALL | Request::HEADER_FORWARDED); $request->initialize(array(), array(), array(), array(), array(), $server); @@ -988,13 +986,11 @@ public function testGetClientIpsWithAgreeingHeaders($httpForwarded, $httpXForwar 'HTTP_X_FORWARDED_FOR' => $httpXForwardedFor, ); - Request::setTrustedProxies(array('88.88.88.88')); + Request::setTrustedProxies(array('88.88.88.88'), Request::HEADER_X_FORWARDED_ALL); $request->initialize(array(), array(), array(), array(), array(), $server); $request->getClientIps(); - - Request::setTrustedProxies(array()); } public function testGetClientIpsWithAgreeingHeadersProvider() @@ -1177,11 +1173,10 @@ public function testOverrideGlobals() $request->headers->set('X_FORWARDED_PROTO', 'https'); - Request::setTrustedProxies(array('1.1.1.1')); + Request::setTrustedProxies(array('1.1.1.1'), Request::HEADER_X_FORWARDED_ALL); $this->assertFalse($request->isSecure()); $request->server->set('REMOTE_ADDR', '1.1.1.1'); $this->assertTrue($request->isSecure()); - Request::setTrustedProxies(array()); $request->overrideGlobals(); @@ -1644,7 +1639,7 @@ private function getRequestInstanceForClientIpTests($remoteAddr, $httpForwardedF } if ($trustedProxies) { - Request::setTrustedProxies($trustedProxies); + Request::setTrustedProxies($trustedProxies, Request::HEADER_X_FORWARDED_ALL); } $request->initialize(array(), array(), array(), array(), array(), $server); @@ -1663,7 +1658,7 @@ private function getRequestInstanceForClientIpsForwardedTests($remoteAddr, $http } if ($trustedProxies) { - Request::setTrustedProxies($trustedProxies); + Request::setTrustedProxies($trustedProxies, Request::HEADER_FORWARDED); } $request->initialize(array(), array(), array(), array(), array(), $server); @@ -1679,10 +1674,6 @@ public function testTrustedProxies() $request->headers->set('X_FORWARDED_HOST', 'foo.example.com, real.example.com:8080'); $request->headers->set('X_FORWARDED_PROTO', 'https'); $request->headers->set('X_FORWARDED_PORT', 443); - $request->headers->set('X_MY_FOR', '3.3.3.3, 4.4.4.4'); - $request->headers->set('X_MY_HOST', 'my.example.com'); - $request->headers->set('X_MY_PROTO', 'http'); - $request->headers->set('X_MY_PORT', 81); // no trusted proxies $this->assertEquals('3.3.3.3', $request->getClientIp()); @@ -1691,40 +1682,60 @@ public function testTrustedProxies() $this->assertFalse($request->isSecure()); // disabling proxy trusting - Request::setTrustedProxies(array()); + Request::setTrustedProxies(array(), Request::HEADER_X_FORWARDED_ALL); $this->assertEquals('3.3.3.3', $request->getClientIp()); $this->assertEquals('example.com', $request->getHost()); $this->assertEquals(80, $request->getPort()); $this->assertFalse($request->isSecure()); // request is forwarded by a non-trusted proxy - Request::setTrustedProxies(array('2.2.2.2')); + Request::setTrustedProxies(array('2.2.2.2'), Request::HEADER_X_FORWARDED_ALL); $this->assertEquals('3.3.3.3', $request->getClientIp()); $this->assertEquals('example.com', $request->getHost()); $this->assertEquals(80, $request->getPort()); $this->assertFalse($request->isSecure()); // trusted proxy via setTrustedProxies() - Request::setTrustedProxies(array('3.3.3.3', '2.2.2.2')); + Request::setTrustedProxies(array('3.3.3.3', '2.2.2.2'), Request::HEADER_X_FORWARDED_ALL); $this->assertEquals('1.1.1.1', $request->getClientIp()); $this->assertEquals('real.example.com', $request->getHost()); $this->assertEquals(443, $request->getPort()); $this->assertTrue($request->isSecure()); // trusted proxy via setTrustedProxies() - Request::setTrustedProxies(array('3.3.3.4', '2.2.2.2')); + Request::setTrustedProxies(array('3.3.3.4', '2.2.2.2'), Request::HEADER_X_FORWARDED_ALL); $this->assertEquals('3.3.3.3', $request->getClientIp()); $this->assertEquals('example.com', $request->getHost()); $this->assertEquals(80, $request->getPort()); $this->assertFalse($request->isSecure()); // check various X_FORWARDED_PROTO header values - Request::setTrustedProxies(array('3.3.3.3', '2.2.2.2')); + Request::setTrustedProxies(array('3.3.3.3', '2.2.2.2'), Request::HEADER_X_FORWARDED_ALL); $request->headers->set('X_FORWARDED_PROTO', 'ssl'); $this->assertTrue($request->isSecure()); $request->headers->set('X_FORWARDED_PROTO', 'https, http'); $this->assertTrue($request->isSecure()); + } + + /** + * @group legacy + * @expectedDeprecation The "Symfony\Component\HttpFoundation\Request::setTrustedHeaderName()" method is deprecated since version 3.3 and will be removed in 4.0. Use "X-Forwarded-*" headers or the "Forwarded" header defined in RFC7239, and the $trustedHeaderSet argument of the Request::setTrustedProxies() method instead. + */ + public function testLegacyTrustedProxies() + { + $request = Request::create('http://example.com/'); + $request->server->set('REMOTE_ADDR', '3.3.3.3'); + $request->headers->set('X_FORWARDED_FOR', '1.1.1.1, 2.2.2.2'); + $request->headers->set('X_FORWARDED_HOST', 'foo.example.com, real.example.com:8080'); + $request->headers->set('X_FORWARDED_PROTO', 'https'); + $request->headers->set('X_FORWARDED_PORT', 443); + $request->headers->set('X_MY_FOR', '3.3.3.3, 4.4.4.4'); + $request->headers->set('X_MY_HOST', 'my.example.com'); + $request->headers->set('X_MY_PROTO', 'http'); + $request->headers->set('X_MY_PORT', 81); + + Request::setTrustedProxies(array('3.3.3.3', '2.2.2.2'), Request::HEADER_X_FORWARDED_ALL); // custom header names Request::setTrustedHeaderName(Request::HEADER_CLIENT_IP, 'X_MY_FOR'); @@ -1746,8 +1757,8 @@ public function testTrustedProxies() $this->assertEquals(80, $request->getPort()); $this->assertFalse($request->isSecure()); - // reset - Request::setTrustedProxies(array()); + //reset + Request::setTrustedHeaderName(Request::HEADER_FORWARDED, 'FORWARDED'); Request::setTrustedHeaderName(Request::HEADER_CLIENT_IP, 'X_FORWARDED_FOR'); Request::setTrustedHeaderName(Request::HEADER_CLIENT_HOST, 'X_FORWARDED_HOST'); Request::setTrustedHeaderName(Request::HEADER_CLIENT_PORT, 'X_FORWARDED_PORT'); @@ -1755,6 +1766,7 @@ public function testTrustedProxies() } /** + * @group legacy * @expectedException \InvalidArgumentException */ public function testSetTrustedProxiesInvalidHeaderName() @@ -1764,6 +1776,7 @@ public function testSetTrustedProxiesInvalidHeaderName() } /** + * @group legacy * @expectedException \InvalidArgumentException */ public function testGetTrustedProxiesInvalidHeaderName() @@ -2062,6 +2075,77 @@ public function methodCacheableProvider() array('CONNECT', false), ); } + + /** + * @group legacy + * @expectedDeprecation The Symfony\Component\HttpFoundation\Request::setTrustedProxies() method expects a bit field of Request::HEADER_* as second argument. Not defining it is deprecated since version 3.3 and will be required in 4.0. + * @expectedDeprecation The "Symfony\Component\HttpFoundation\Request::getTrustedHeaderName()" method is deprecated since version 3.3 and will be removed in 4.0. Use the Request::getTrustedHeaderSet() method instead. + */ + public function testSetTrustedProxiesNoSecondArg() + { + Request::setTrustedProxies(array('8.8.8.8')); + + $this->assertSame('FORWARDED', Request::getTrustedHeaderName(Request::HEADER_FORWARDED)); + $this->assertSame('X_FORWARDED_FOR', Request::getTrustedHeaderName(Request::HEADER_CLIENT_IP)); + $this->assertSame('X_FORWARDED_HOST', Request::getTrustedHeaderName(Request::HEADER_CLIENT_HOST)); + $this->assertSame('X_FORWARDED_PORT', Request::getTrustedHeaderName(Request::HEADER_CLIENT_PORT)); + $this->assertSame('X_FORWARDED_PROTO', Request::getTrustedHeaderName(Request::HEADER_CLIENT_PROTO)); + } + + /** + * @group legacy + */ + public function testGetTrustedHeaderName() + { + Request::setTrustedProxies(array('8.8.8.8'), Request::HEADER_X_FORWARDED_ALL); + + $this->assertNull(Request::getTrustedHeaderName(Request::HEADER_FORWARDED)); + $this->assertSame('X_FORWARDED_FOR', Request::getTrustedHeaderName(Request::HEADER_CLIENT_IP)); + $this->assertSame('X_FORWARDED_HOST', Request::getTrustedHeaderName(Request::HEADER_CLIENT_HOST)); + $this->assertSame('X_FORWARDED_PORT', Request::getTrustedHeaderName(Request::HEADER_CLIENT_PORT)); + $this->assertSame('X_FORWARDED_PROTO', Request::getTrustedHeaderName(Request::HEADER_CLIENT_PROTO)); + + Request::setTrustedProxies(array('8.8.8.8'), Request::HEADER_FORWARDED); + + $this->assertSame('FORWARDED', Request::getTrustedHeaderName(Request::HEADER_FORWARDED)); + $this->assertNull(Request::getTrustedHeaderName(Request::HEADER_CLIENT_IP)); + $this->assertNull(Request::getTrustedHeaderName(Request::HEADER_CLIENT_HOST)); + $this->assertNull(Request::getTrustedHeaderName(Request::HEADER_CLIENT_PORT)); + $this->assertNull(Request::getTrustedHeaderName(Request::HEADER_CLIENT_PROTO)); + + Request::setTrustedHeaderName(Request::HEADER_FORWARDED, 'A'); + Request::setTrustedHeaderName(Request::HEADER_CLIENT_IP, 'B'); + Request::setTrustedHeaderName(Request::HEADER_CLIENT_HOST, 'C'); + Request::setTrustedHeaderName(Request::HEADER_CLIENT_PORT, 'D'); + Request::setTrustedHeaderName(Request::HEADER_CLIENT_PROTO, 'E'); + + Request::setTrustedProxies(array('8.8.8.8'), Request::HEADER_FORWARDED); + + $this->assertSame('A', Request::getTrustedHeaderName(Request::HEADER_FORWARDED)); + $this->assertNull(Request::getTrustedHeaderName(Request::HEADER_CLIENT_IP)); + $this->assertNull(Request::getTrustedHeaderName(Request::HEADER_CLIENT_HOST)); + $this->assertNull(Request::getTrustedHeaderName(Request::HEADER_CLIENT_PORT)); + $this->assertNull(Request::getTrustedHeaderName(Request::HEADER_CLIENT_PROTO)); + + Request::setTrustedProxies(array('8.8.8.8'), Request::HEADER_X_FORWARDED_ALL); + + $this->assertNull(Request::getTrustedHeaderName(Request::HEADER_FORWARDED)); + $this->assertSame('B', Request::getTrustedHeaderName(Request::HEADER_CLIENT_IP)); + $this->assertSame('C', Request::getTrustedHeaderName(Request::HEADER_CLIENT_HOST)); + $this->assertSame('D', Request::getTrustedHeaderName(Request::HEADER_CLIENT_PORT)); + $this->assertSame('E', Request::getTrustedHeaderName(Request::HEADER_CLIENT_PROTO)); + + Request::setTrustedProxies(array('8.8.8.8'), Request::HEADER_FORWARDED); + + $this->assertSame('A', Request::getTrustedHeaderName(Request::HEADER_FORWARDED)); + + //reset + Request::setTrustedHeaderName(Request::HEADER_FORWARDED, 'FORWARDED'); + Request::setTrustedHeaderName(Request::HEADER_CLIENT_IP, 'X_FORWARDED_FOR'); + Request::setTrustedHeaderName(Request::HEADER_CLIENT_HOST, 'X_FORWARDED_HOST'); + Request::setTrustedHeaderName(Request::HEADER_CLIENT_PORT, 'X_FORWARDED_PORT'); + Request::setTrustedHeaderName(Request::HEADER_CLIENT_PROTO, 'X_FORWARDED_PROTO'); + } } class RequestContentProxy extends Request diff --git a/src/Symfony/Component/HttpKernel/Fragment/InlineFragmentRenderer.php b/src/Symfony/Component/HttpKernel/Fragment/InlineFragmentRenderer.php index a61b239c158b9..09ce50df4d260 100644 --- a/src/Symfony/Component/HttpKernel/Fragment/InlineFragmentRenderer.php +++ b/src/Symfony/Component/HttpKernel/Fragment/InlineFragmentRenderer.php @@ -119,7 +119,7 @@ protected function createSubRequest($uri, Request $request) // Sub-request object will point to localhost as client ip and real client ip // will be included into trusted header for client ip try { - if ($trustedHeaderName = Request::getTrustedHeaderName(Request::HEADER_CLIENT_IP)) { + if ($trustedHeaderName = Request::getTrustedHeaderName(Request::HEADER_CLIENT_IP, false)) { $currentXForwardedFor = $request->headers->get($trustedHeaderName, ''); $server['HTTP_'.$trustedHeaderName] = ($currentXForwardedFor ? $currentXForwardedFor.', ' : '').$request->getClientIp(); diff --git a/src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php b/src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php index 686f0b852c202..aa7dc0a2bc3cd 100644 --- a/src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php +++ b/src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php @@ -464,7 +464,7 @@ protected function forward(Request $request, $catch = false, Response $entry = n // make sure HttpCache is a trusted proxy if (!in_array('127.0.0.1', $trustedProxies = Request::getTrustedProxies())) { $trustedProxies[] = '127.0.0.1'; - Request::setTrustedProxies($trustedProxies); + Request::setTrustedProxies($trustedProxies, Request::HEADER_X_FORWARDED_ALL); } // always a "master" request (as the real master request can be in cache) diff --git a/src/Symfony/Component/HttpKernel/Tests/EventListener/ValidateRequestListenerTest.php b/src/Symfony/Component/HttpKernel/Tests/EventListener/ValidateRequestListenerTest.php index 8311a76e35a03..9d2cf3ee89d73 100644 --- a/src/Symfony/Component/HttpKernel/Tests/EventListener/ValidateRequestListenerTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/EventListener/ValidateRequestListenerTest.php @@ -30,7 +30,7 @@ public function testListenerThrowsWhenMasterRequestHasInconsistentClientIps() $kernel = $this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(); $request = new Request(); - $request->setTrustedProxies(array('1.1.1.1')); + $request->setTrustedProxies(array('1.1.1.1'), Request::HEADER_X_FORWARDED_FOR | Request::HEADER_FORWARDED); $request->server->set('REMOTE_ADDR', '1.1.1.1'); $request->headers->set('FORWARDED', '2.2.2.2'); $request->headers->set('X_FORWARDED_FOR', '3.3.3.3'); diff --git a/src/Symfony/Component/HttpKernel/Tests/Fragment/InlineFragmentRendererTest.php b/src/Symfony/Component/HttpKernel/Tests/Fragment/InlineFragmentRendererTest.php index 4a665adadc60d..18e55a5be0df2 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Fragment/InlineFragmentRendererTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/Fragment/InlineFragmentRendererTest.php @@ -25,18 +25,6 @@ class InlineFragmentRendererTest extends TestCase { - private $originalTrustedHeaderName; - - protected function setUp() - { - $this->originalTrustedHeaderName = Request::getTrustedHeaderName(Request::HEADER_CLIENT_IP); - } - - protected function tearDown() - { - Request::setTrustedHeaderName(Request::HEADER_CLIENT_IP, $this->originalTrustedHeaderName); - } - public function testRender() { $strategy = new InlineFragmentRenderer($this->getKernel($this->returnValue(new Response('foo')))); @@ -109,10 +97,12 @@ public function testRenderWithObjectsAsAttributesPassedAsObjectsInTheController( public function testRenderWithTrustedHeaderDisabled() { - Request::setTrustedHeaderName(Request::HEADER_CLIENT_IP, ''); + Request::setTrustedProxies(array(), 0); $strategy = new InlineFragmentRenderer($this->getKernelExpectingRequest(Request::create('/'))); $this->assertSame('foo', $strategy->render('/', Request::create('/'))->getContent()); + + Request::setTrustedProxies(array(), -1); } /** @@ -198,7 +188,7 @@ public function testESIHeaderIsKeptInSubrequest() $expectedSubRequest = Request::create('/'); $expectedSubRequest->headers->set('Surrogate-Capability', 'abc="ESI/1.0"'); - if (Request::getTrustedHeaderName(Request::HEADER_CLIENT_IP)) { + 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'); } @@ -212,18 +202,17 @@ public function testESIHeaderIsKeptInSubrequest() public function testESIHeaderIsKeptInSubrequestWithTrustedHeaderDisabled() { - $trustedHeaderName = Request::getTrustedHeaderName(Request::HEADER_CLIENT_IP); - Request::setTrustedHeaderName(Request::HEADER_CLIENT_IP, ''); + Request::setTrustedProxies(array(), 0); $this->testESIHeaderIsKeptInSubrequest(); - Request::setTrustedHeaderName(Request::HEADER_CLIENT_IP, $trustedHeaderName); + Request::setTrustedProxies(array(), -1); } public function testHeadersPossiblyResultingIn304AreNotAssignedToSubrequest() { $expectedSubRequest = Request::create('/'); - if (Request::getTrustedHeaderName(Request::HEADER_CLIENT_IP)) { + 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'); } diff --git a/src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTest.php b/src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTest.php index d5cf8ca7ae70d..1016cbc737c10 100644 --- a/src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTest.php @@ -1184,7 +1184,7 @@ public function testClientIpIsAlwaysLocalhostForForwardedRequests() */ public function testHttpCacheIsSetAsATrustedProxy(array $existing, array $expected) { - Request::setTrustedProxies($existing); + Request::setTrustedProxies($existing, Request::HEADER_X_FORWARDED_ALL); $this->setNextResponse(); $this->request('GET', '/', array('REMOTE_ADDR' => '10.0.0.1')); diff --git a/src/Symfony/Component/HttpKernel/Tests/HttpKernelTest.php b/src/Symfony/Component/HttpKernel/Tests/HttpKernelTest.php index 637924d44e17d..db6e4a33e1c31 100644 --- a/src/Symfony/Component/HttpKernel/Tests/HttpKernelTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/HttpKernelTest.php @@ -337,7 +337,7 @@ public function testVerifyRequestStackPushPopDuringHandle() public function testInconsistentClientIpsOnMasterRequests() { $request = new Request(); - $request->setTrustedProxies(array('1.1.1.1')); + $request->setTrustedProxies(array('1.1.1.1'), Request::HEADER_X_FORWARDED_FOR | Request::HEADER_FORWARDED); $request->server->set('REMOTE_ADDR', '1.1.1.1'); $request->headers->set('FORWARDED', '2.2.2.2'); $request->headers->set('X_FORWARDED_FOR', '3.3.3.3');