8000 [HttpFoundation] Add $trustedHeader arg to Request::setTrustedProxies… · symfony/symfony@306cc67 · GitHub
[go: up one dir, main page]
Skip to content

Commit 306cc67

Browse files
nicolas-grekasnicolas-grekas
committed
[HttpFoundation] Add $trustedHeader arg to Request::setTrustedProxies() - deprecate not setting it
1 parent aaa1437 commit 306cc67

File tree

5 files changed

+139
-31
lines changed

5 files changed

+139
-31
lines changed

‎UPGRADE-3.3.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,13 @@ FrameworkBundle
166166
class has been deprecated and will be removed in 4.0. Use the
167167
`Symfony\Component\Routing\DependencyInjection\RoutingResolverPass` class instead.
168168

169+
HttpFoundation
170+
--------------
171+
172+
* The `Request::setTrustedProxies()` method takes a new `$trustedHeader` argument - not setting it is deprecated.
173+
Set it to `Request::HEADER_FORWARDED` if your reverse-proxy uses the RFC7239 `Forwarded` header,
174+
or to `Request::HEADER_CLIENT_IP` if it is using `X-Forwarded-For`.
175+
169176
HttpKernel
170177
-----------
171178

‎UPGRADE-4.0.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,13 @@ FrameworkBundle
274274
HttpFoundation
275275
---------------
276276

277+
HttpFoundation
278+
--------------
279+
280+
* The `Request::setTrustedProxies()` method takes a new `$trustedHeader` mandatory argument.
281+
Set it to `Request::HEADER_FORWARDED` if your reverse-proxy uses the RFC7239 `Forwarded` header,
282+
or to `Request::HEADER_CLIENT_IP` if it is using `X-Forwarded-For` or similar.
283+
277284
* Extending the following methods of `Response`
278285
is no longer possible (these methods are now `final`):
279286

‎src/Symfony/Component/HttpFoundation/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ CHANGELOG
44
3.3.0
55
-----
66

7+
* added `$trustedHeader` argument to `Request::setTrustedProxies()` - deprecate not setting it,
78
* added `File\Stream`, to be passed to `BinaryFileResponse` when the size of the served file is unknown,
89
disabling `Range` and `Content-Length` handling, switching to chunked encoding instead
910
* added the `Cookie::fromString()` method that allows to create a cookie from a

‎src/Symfony/Component/HttpFoundation/Request.php

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,14 @@ class Request
210210
private $isHostValid = true;
211211
private $isClientIpsValid = true;
212212

213+
private static $trustedHeaderNames = array(
214+
self::HEADER_FORWARDED => 'FORWARDED',
215+
self::HEADER_CLIENT_IP => 'X_FORWARDED_FOR',
216+
self::HEADER_CLIENT_HOST => 'X_FORWARDED_HOST',
217+
self::HEADER_CLIENT_PROTO => 'X_FORWARDED_PROTO',
218+
self::HEADER_CLIENT_PORT => 'X_FORWARDED_PORT',
219+
);
220+
213221
/**
214222
* Constructor.
215223
*
@@ -548,11 +556,37 @@ public function overrideGlobals()
548556
*
549557
* You should only list the reverse proxies that you manage directly.
550558
*
551-
* @param array $proxies A list of trusted proxies
559+
* @param array $proxies A list of trusted proxies
560+
* @param string $trustedHeader Either Request::HEADER_FORWARDED or Request::HEADER_CLIENT_IP to set which header to trust from your proxies
561+
*
562+
* @throws \InvalidArgumentException When $trustedHeader is invalid
552563
*/
553-
public static function setTrustedProxies(array $proxies)
564+
public static function setTrustedProxies(array $proxies/*, string $trustedHeader*/)
554565
{
555566
self::$trustedProxies = $proxies;
567+
568+
if (2 > func_num_args()) {
569+
@trigger_error(sprintf('The %s() method expects Request::HEADER_FORWARDED or Request::HEADER_CLIENT_IP as second argument. Not passing it is deprecated since version 3.3 and will be required in 4.0.', __METHOD__), E_USER_DEPRECATED);
570+
571+
return;
572+
}
573+
$trustedHeader = func_get_arg(1);
574+
575+
if (self::HEADER_FORWARDED === $trustedHeader) {
576+
self::$trustedHeaders[self::HEADER_FORWARDED] = self::$trustedHeaderNames[self::HEADER_FORWARDED];
577+
self::$trustedHeaders[self::HEADER_CLIENT_IP] = null;
578+
self::$trustedHeaders[self::HEADER_CLIENT_HOST] = null;
579+
self::$trustedHeaders[self::HEADER_CLIENT_PORT] = null;
580+
self::$trustedHeaders[self::HEADER_CLIENT_PROTO] = null;
581+
} elseif (self::HEADER_CLIENT_IP === $trustedHeader) {
582+
self::$trustedHeaders[self::HEADER_FORWARDED] = null;
583+
self::$trustedHeaders[self::HEADER_CLIENT_IP] = self::$trustedHeaderNames[self::HEADER_CLIENT_IP];
584+
self::$trustedHeaders[self::HEADER_CLIENT_HOST] = self::$trustedHeaderNames[self::HEADER_CLIENT_HOST];
585+
self::$trustedHeaders[self::HEADER_CLIENT_PORT] = self::$trustedHeaderNames[self::HEADER_CLIENT_PORT];
586+
self::$trustedHeaders[self::HEADER_CLIENT_PROTO] = self::$trustedHeaderNames[self::HEADER_CLIENT_PROTO];
587+
} else {
588+
throw new \InvalidArgumentException(sprintf('Invalid trusted header argument: Request::HEADER_FORWARDED or Request::HEADER_CLIENT_IP was expected, "%s" given.', $trustedHead 10000 er));
589+
}
556590
}
557591

558592
/**
@@ -616,6 +650,10 @@ public static function setTrustedHeaderName($key, $value)
616650
}
617651

618652
self::$trustedHeaders[$key] = $value;
653+
654+
if (null !== $value) {
655+
self::$trustedHeaderNames[$key] = $value;
656+
}
619657
}
620658

621659
/**

‎src/Symfony/Component/HttpFoundation/Tests/RequestTest.php

Lines changed: 84 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,17 @@
1919

2020
class RequestTest extends TestCase
2121
{
22+
protected function tearDown()
23+
{
24+
// reset
25+
Request::setTrustedProxies(array(), Request::HEADER_CLIENT_IP);
26+
Request::setTrustedHeaderName(Request::HEADER_FORWARDED, 'FORWARDED');
27+
Request::setTrustedHeaderName(Request::HEADER_CLIENT_IP, 'X_FORWARDED_FOR');
28+
Request::setTrustedHeaderName(Request::HEADER_CLIENT_HOST, 'X_FORWARDED_HOST');
29+
Request::setTrustedHeaderName(Request::HEADER_CLIENT_PORT, 'X_FORWARDED_PORT');
30+
Request::setTrustedHeaderName(Request::HEADER_CLIENT_PROTO, 'X_FORWARDED_PROTO');
31+
}
32+
2233
public function testInitialize()
2334
{
2435
$request = new Request();
@@ -727,7 +738,7 @@ public function testGetPort()
727738

728739
$this->assertEquals(80, $port, 'Without trusted proxies FORWARDED_PROTO and FORWARDED_PORT are ignored.');
729740

730-
Request::setTrustedProxies(array('1.1.1.1'));
741+
Request::setTrustedProxies(array('1.1.1.1'), Request::HEADER_CLIENT_IP);
731742
$request = Request::create('http://example.com', 'GET', array(), array(), array(), array(
732743
'HTTP_X_FORWARDED_PROTO' => 'https',
733744
'HTTP_X_FORWARDED_PORT' => '8443',
@@ -769,8 +780,6 @@ public function testGetPort()
769780
));
770781
$port = $request->getPort();
771782
$this->assertEquals(80, $port, 'With only PROTO set and value is not recognized, getPort() defaults to 80.');
772-
773-
Request::setTrustedProxies(array());
774783
}
775784

776785
/**
@@ -846,8 +855,6 @@ public function testGetClientIp($expected, $remoteAddr, $httpForwardedFor, $trus
846855
$request = $this->getRequestInstanceForClientIpTests($remoteAddr, $httpForwardedFor, $trustedProxies);
847856

848857
$this->assertEquals($expected[0], $request->getClientIp());
849-
850-
Request::setTrustedProxies(array());
851858
}
852859

853860
/**
@@ -858,8 +865,6 @@ public function testGetClientIps($expected, $remoteAddr, $httpForwardedFor, $tru
858865
$request = $this->getRequestInstanceForClientIpTests($remoteAddr, $httpForwardedFor, $trustedProxies);
859866

860867
$this->assertEquals($expected, $request->getClientIps());
861-
862-
Request::setTrustedProxies(array());
863868
}
864869

865870
/**
@@ -870,8 +875,6 @@ public function testGetClientIpsForwarded($expected, $remoteAddr, $httpForwarded
870875
$request = $this->getRequestInstanceForClientIpsForwardedTests($remoteAddr, $httpForwarded, $trustedProxies);
871876

872877
$this->assertEquals($expected, $request->getClientIps());
873-
874-
Request::setTrustedProxies(array());
875878
}
876879

877880
public function testGetClientIpsForwardedProvider()
@@ -956,7 +959,9 @@ public function testGetClientIpsWithConflictingHeaders($httpForwarded, $httpXFor
956959
'HTTP_X_FORWARDED_FOR' => $httpXForwardedFor,
957960
);
958961

959-
Request::setTrustedProxies(array('88.88.88.88'));
962+
Request::setTrustedProxies(array('88.88.88.88'), Request::HEADER_CLIENT_IP);
963+
Request::setTrustedHeaderName(Request::HEADER_FORWARDED, 'FORWARDED');
964+
Request::setTrustedHeaderName(Request::HEADER_CLIENT_IP, 'X_FORWARDED_FOR');
960965

961966
$request->initialize(array(), array(), array(), array(), array(), $server);
962967

@@ -988,13 +993,11 @@ public function testGetClientIpsWithAgreeingHeaders($httpForwarded, $httpXForwar
988993
'HTTP_X_FORWARDED_FOR' => $httpXForwardedFor,
989994
);
990995

991-
Request::setTrustedProxies(array('88.88.88.88'));
996+
Request::setTrustedProxies(array('88.88.88.88'), Request::HEADER_CLIENT_IP);
992997

993998
$request->initialize(array(), array(), array(), array(), array(), $server);
994999

9951000
$request->getClientIps();
996-
997-
Request::setTrustedProxies(array());
9981001
}
9991002

10001003
public function testGetClientIpsWithAgreeingHeadersProvider()
@@ -1177,11 +1180,10 @@ public function testOverrideGlobals()
11771180

11781181
$request->headers->set('X_FORWARDED_PROTO', 'https');
11791182

1180-
Request::setTrustedProxies(array('1.1.1.1'));
1183+
Request::setTrustedProxies(array('1.1.1.1'), Request::HEADER_CLIENT_IP);
11811184
$this->assertFalse($request->isSecure());
11821185
$request->server->set('REMOTE_ADDR', '1.1.1.1');
11831186
$this->assertTrue($request->isSecure());
1184-
Request::setTrustedProxies(array());
11851187

11861188
$request->overrideGlobals();
11871189

@@ -1644,7 +1646,7 @@ private function getRequestInstanceForClientIpTests($remoteAddr, $httpForwardedF
16441646
}
16451647

16461648
if ($trustedProxies) {
1647-
Request::setTrustedProxies($trustedProxies);
1649+
Request::setTrustedProxies($trustedProxies, Request::HEADER_CLIENT_IP);
16481650
}
16491651

16501652
$request->initialize(array(), array(), array(), array(), array(), $server);
@@ -1663,7 +1665,7 @@ private function getRequestInstanceForClientIpsForwardedTests($remoteAddr, $http
16631665
}
16641666

16651667
if ($trustedProxies) {
1666-
Request::setTrustedProxies($trustedProxies);
1668+
Request::setTrustedProxies($trustedProxies, Request::HEADER_FORWARDED);
16671669
}
16681670

16691671
$request->initialize(array(), array(), array(), array(), array(), $server);
@@ -1691,35 +1693,35 @@ public function testTrustedProxies()
16911693
$this->assertFalse($request->isSecure());
16921694

16931695
// disabling proxy trusting
1694-
Request::setTrustedProxies(array());
1696+
Request::setTrustedProxies(array(), Request::HEADER_CLIENT_IP);
16951697
$this->assertEquals('3.3.3.3', $request->getClientIp());
16961698
$this->assertEquals('example.com', $request->getHost());
16971699
$this->assertEquals(80, $request->getPort());
16981700
$this->assertFalse($request->isSecure());
16991701

17001702
// request is forwarded by a non-trusted proxy
1701-
Request::setTrustedProxies(array('2.2.2.2'));
1703+
Request::setTrustedProxies(array('2.2.2.2'), Request::HEADER_CLIENT_IP);
17021704
$this->assertEquals('3.3.3.3', $request->getClientIp());
17031705
$this->assertEquals('example.com', $request->getHost());
17041706
$this->assertEquals(80, $request->getPort());
17051707
$this->assertFalse($request->isSecure());
17061708

17071709
// trusted proxy via setTrustedProxies()
1708-
Request::setTrustedProxies(array('3.3.3.3', '2.2.2.2'));
1710+
Request::setTrustedProxies(array('3.3.3.3', '2.2.2.2'), Request::HEADER_CLIENT_IP);
17091711
$this->assertEquals('1.1.1.1', $request->getClientIp());
17101712
$this->assertEquals('real.example.com', $request->getHost());
17111713
$this->assertEquals(443, $request->getPort());
17121714
$this->assertTrue($request->isSecure());
17131715

17141716
// trusted proxy via setTrustedProxies()
1715-
Request::setTrustedProxies(array('3.3.3.4', '2.2.2.2'));
1717+
Request::setTrustedProxies(array('3.3.3.4', '2.2.2.2'), Request::HEADER_CLIENT_IP);
17161718
$this->assertEquals('3.3.3.3', $request->getClientIp());
17171719
$this->assertEquals('example.com', $request->getHost());
17181720
$this->assertEquals(80, $request->getPort());
17191721
$this->assertFalse($request->isSecure());
17201722

17211723
// check various X_FORWARDED_PROTO header values
1722-
Request::setTrustedProxies(array('3.3.3.3', '2.2.2.2'));
1724+
Request::setTrustedProxies(array('3.3.3.3', '2.2.2.2'), Request::HEADER_CLIENT_IP);
17231725
$request->headers->set('X_FORWARDED_PROTO', 'ssl');
17241726
$this->assertTrue($request->isSecure());
17251727

@@ -1745,13 +1747,6 @@ public function testTrustedProxies()
17451747
$this->assertEquals('example.com', $request->getHost());
17461748
$this->assertEquals(80, $request->getPort());
17471749
$this->assertFalse($request->isSecure());
1748-
1749-
// reset
1750-
Request::setTrustedProxies(array());
1751-
Request::setTrustedHeaderName(Request::HEADER_CLIENT_IP, 'X_FORWARDED_FOR');
1752-
Request::setTrustedHeaderName(Request::HEADER_CLIENT_HOST, 'X_FORWARDED_HOST');
1753-
Request::setTrustedHeaderName(Request::HEADER_CLIENT_PORT, 'X_FORWARDED_PORT');
1754-
Request::setTrustedHeaderName(Request::HEADER_CLIENT_PROTO, 'X_FORWARDED_PROTO');
17551750
}
17561751

17571752
/**
@@ -2062,6 +2057,66 @@ public function methodCacheableProvider()
20622057
array('CONNECT', false),
20632058
);
20642059
}
2060+
2061+
/**
2062+
* @group legacy
2063+
* @expectedDeprecation The Symfony\Component\HttpFoundation\Request::setTrustedProxies() method expects Request::HEADER_FORWARDED or Request::HEADER_CLIENT_IP as second argument. Not passing it is deprecated since version 3.3 and will be required in 4.0.
2064+
*/
2065+
public function testLegacySetTrustedProxies()
2066+
{
2067+
Request::setTrustedProxies(array('8.8.8.8'));
2068+
2069+
$this->assertSame('FORWARDED', Request::getTrustedHeaderName(Request::HEADER_FORWARDED));
2070+
$this->assertSame('X_FORWARDED_FOR', Request::getTrustedHeaderName(Request::HEADER_CLIENT_IP));
2071+
$this->assertSame('X_FORWARDED_HOST', Request::getTrustedHeaderName(Request::HEADER_CLIENT_HOST));
2072+
$this->assertSame('X_FORWARDED_PORT', Request::getTrustedHeaderName(Request::HEADER_CLIENT_PORT));
2073+
$this->assertSame('X_FORWARDED_PROTO', Request::getTrustedHeaderName(Request::HEADER_CLIENT_PROTO));
2074+
}
2075+
2076+
public function testSetTrustedProxies()
2077+
{
2078+
Request::setTrustedProxies(array('8.8.8.8'), Request::HEADER_CLIENT_IP);
2079+
2080+
$this->assertNull(Request::getTrustedHeaderName(Request::HEADER_FORWARDED));
2081+
$this->assertSame('X_FORWARDED_FOR', Request::getTrustedHeaderName(Request::HEADER_CLIENT_IP));
2082+
$this->assertSame('X_FORWARDED_HOST', Request::getTrustedHeaderName(Request::HEADER_CLIENT_HOST));
2083+
$this->assertSame('X_FORWARDED_PORT', Request::getTrustedHeaderName(Request::HEADER_CLIENT_PORT));
2084+
$this->assertSame('X_FORWARDED_PROTO', Request::getTrustedHeaderName(Request::HEADER_CLIENT_PROTO));
2085+
2086+
Request::setTrustedProxies(array('8.8.8.8'), Request::HEADER_FORWARDED);
2087+
2088+
$this->assertSame('FORWARDED', Request::getTrustedHeaderName(Request::HEADER_FORWARDED));
2089+
$this->assertNull(Request::getTrustedHeaderName(Request::HEADER_CLIENT_IP));
2090+
$this->assertNull(Request::getTrustedHeaderName(Request::HEADER_CLIENT_HOST));
2091+
$this->assertNull(Request::getTrustedHeaderName(Request::HEADER_CLIENT_PORT));
2092+
$this->assertNull(Request::getTrustedHeaderName(Request::HEADER_CLIENT_PROTO));
2093+
2094+
Request::setTrustedHeaderName(Request::HEADER_FORWARDED, 'A');
2095+
Request::setTrustedHeaderName(Request::HEADER_CLIENT_IP, 'B');
2096+
Request::setTrustedHeaderName(Request::HEADER_CLIENT_HOST, 'C');
2097+
Request::setTrustedHeaderName(Request::HEADER_CLIENT_PORT, 'D');
2098+
Request::setTrustedHeaderName(Request::HEADER_CLIENT_PROTO, 'E');
2099+
2100+
Request::setTrustedProxies(array('8.8.8.8'), Request::HEADER_FORWARDED);
2101+
2102+
$this->assertSame('A', Request::getTrustedHeaderName(Request::HEADER_FORWARDED));
2103+
$this->assertNull(Request::getTrustedHeaderName(Request::HEADER_CLIENT_IP));
2104+
$this->assertNull(Request::getTrustedHeaderName(Request::HEADER_CLIENT_HOST));
2105+
$this->assertNull(Request::getTrustedHeaderName(Request::HEADER_CLIENT_PORT));
2106+
$this->assertNull(Request::getTrustedHeaderName(Request::HEADER_CLIENT_PROTO));
2107+
2108+
Request::setTrustedProxies(array('8.8.8.8'), Request::HEADER_CLIENT_IP);
2109+
2110+
$this->assertNull(Request::getTrustedHeaderName(Request::HEADER_FORWARDED));
2111+
$this->assertSame('B', Request::getTrustedHeaderName(Request::HEADER_CLIENT_IP));
2112+
$this->assertSame('C', Request::getTrustedHeaderName(Request::HEADER_CLIENT_HOST));
2113+
$this->assertSame('D', Request::getTrustedHeaderName(Request::HEADER_CLIENT_PORT));
2114+
$this->assertSame('E', Request::getTrustedHeaderName(Request::HEADER_CLIENT_PROTO));
2115+
2116+
Request::setTrustedProxies(array('8.8.8.8'), Request::HEADER_FORWARDED);
2117+
2118+
$this->assertSame('A', Request::getTrustedHeaderName(Request::HEADER_FORWARDED));
2119+
}
20652120
}
20662121

20672122
class RequestContentProxy extends Request

0 commit comments

Comments
 (0)
0