Skip to content

Commit 8e8f09e

Browse files
nicolas-grekasnicolas-grekas
committed
[HttpKernel] fix forwarding trusted headers as server parameters
1 parent 0332f86 commit 8e8f09e

File tree

4 files changed

+23
-7
lines changed

4 files changed

+23
-7
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1991,7 +1991,7 @@ private function normalizeAndFilterClientIps(array $clientIps, $ip)
19911991
if ($i) {
19921992
$clientIps[$key] = $clientIp = substr($clientIp, 0, $i);
19931993
}
1994-
} elseif ('[' == $clientIp[0]) {
1994+
} elseif (0 === strpos($clientIp, '[')) {
19951995
// Strip brackets and :port from IPv6 addresses.
19961996
$i = strpos($clientIp, ']', 1);
19971997
$clientIps[$key] = $clientIp = substr($clientIp, 1, $i - 1);

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -868,7 +868,7 @@ public function getClientIpsForwardedProvider()
868868

869869
public function getClientIpsProvider()
870870
{
871-
// $expected $remoteAddr $httpForwardedFor $trustedProxies
871+
// $expected $remoteAddr $httpForwardedFor $trustedProxies
872872
return array(
873873
// simple IPv4
874874
array(array('88.88.88.88'), '88.88.88.88', null, null),
@@ -882,8 +882,8 @@ public function getClientIpsProvider()
882882

883883
// forwarded for with remote IPv4 addr not trusted
884884
array(array('127.0.0.1'), '127.0.0.1', '88.88.88.88', null),
885-
// forwarded for with remote IPv4 addr trusted
886-
array(array('88.88.88.88'), '127.0.0.1', '88.88.88.88', array('127.0.0.1')),
885+
// forwarded for with remote IPv4 addr trusted + comma
886+
array(array('88.88.88.88'), '127.0.0.1', '88.88.88.88,', array('127.0.0.1')),
887887
// forwarded for with remote IPv4 and all FF addrs trusted
888888
array(array('88.88.88.88'), '127.0.0.1', '88.88.88.88', array('127.0.0.1', '88.88.88.88')),
889889
// forwarded for with remote IPv4 range trusted

‎src/Symfony/Component/HttpKernel/HttpCache/SubRequestHandler.php

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ public static function handle(HttpKernelInterface $kernel, Request $request, $ty
4343
if (!IpUtils::checkIp($remoteAddr, $trustedProxies)) {
4444
foreach (array_filter($trustedHeaders) as $name) {
4545
$request->headers->remove($name);
46+
$request->server->remove('HTTP_'.strtoupper(str_replace('-', '_', $name)));
4647
}
4748
}
4849

@@ -61,13 +62,16 @@ public static function handle(HttpKernelInterface $kernel, Request $request, $ty
6162
// set trusted values, reusing as much as possible the global trusted settings
6263
if ($name = $trustedHeaders[Request::HEADER_FORWARDED]) {
6364
$trustedValues[0] .= sprintf(';host="%s";proto=%s', $request->getHttpHost(), $request->getScheme());
64-
$request->headers->set($name, implode(', ', $trustedValues));
65+
$request->headers->set($name, $v = implode(', ', $trustedValues));
66+
$request->server->set('HTTP_'.strtoupper(str_replace('-', '_', $name)), $v);
6567
}
6668
if ($name = $trustedHeaders[Request::HEADER_CLIENT_IP]) {
67-
$request->headers->set($name, implode(', ', $trustedIps));
69+
$request->headers->set($name, $v = implode(', ', $trustedIps));
70+
$request->server->set('HTTP_'.strtoupper(str_replace('-', '_', $name)), $v);
6871
}
6972
if (!$name && !$trustedHeaders[Request::HEADER_FORWARDED]) {
70-
$request->headers->set('X-Forwarded-For', implode(', ', $trustedIps));
73+
$request->headers->set('X-Forwarded-For', $v = implode(', ', $trustedIps));
74+
$request->server->set('HTTP_X_FORWARDED_FOR', $v);
7175
Request::setTrustedHeaderName(Request::HEADER_CLIENT_IP, 'X_FORWARDED_FOR');
7276
}
7377

‎src/Symfony/Component/HttpKernel/Tests/Fragment/InlineFragmentRendererTest.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ public function testRenderWithObjectsAsAttributes()
6060
$subRequest->attributes->replace(array('object' => $object, '_format' => 'html', '_controller' => 'main_controller', '_locale' => 'en'));
6161
$subRequest->headers->set('x-forwarded-for', array('127.0.0.1'));
6262
$subRequest->headers->set('forwarded', array('for="127.0.0.1";host="localhost";proto=http'));
63+
$subRequest->server->set('HTTP_X_FORWARDED_FOR', '127.0.0.1');
64+
$subRequest->server->set('HTTP_FORWARDED', 'for="127.0.0.1";host="localhost";proto=http');
6365

6466
$strategy = new InlineFragmentRenderer($this->getKernelExpectingRequest($subRequest));
6567

@@ -91,6 +93,8 @@ public function testRenderWithTrustedHeaderDisabled()
9193

9294
$expectedSubRequest = Request::create('/');
9395
$expectedSubRequest->headers->set('x-forwarded-for', array('127.0.0.1'));
96+
$expectedSubRequest->server->set('HTTP_X_FORWARDED_FOR', '127.0.0.1');
97+
9498

9599
$strategy = new InlineFragmentRenderer($this->getKernelExpectingRequest($expectedSubRequest));
96100
$this->assertSame('foo', $strategy->render('/', Request::create('/'))->getContent());
@@ -178,8 +182,10 @@ public function testESIHeaderIsKeptInSubrequest()
178182
$expectedSubRequest->headers->set('Surrogate-Capability', 'abc="ESI/1.0"');
179183
if (Request::getTrustedHeaderName(Request::HEADER_CLIENT_IP)) {
180184
$expectedSubRequest->headers->set('x-forwarded-for', array('127.0.0.1'));
185+
$expectedSubRequest->server->set('HTTP_X_FORWARDED_FOR', '127.0.0.1');
181186
}
182187
$expectedSubRequest->headers->set('forwarded', array('for="127.0.0.1";host="localhost";proto=http'));
188+
$expectedSubRequest->server->set('HTTP_FORWARDED', 'for="127.0.0.1";host="localhost";proto=http');
183189

184190
$strategy = new InlineFragmentRenderer($this->getKernelExpectingRequest($expectedSubRequest));
185191

@@ -203,6 +209,8 @@ public function testHeadersPossiblyResultingIn304AreNotAssignedToSubrequest()
203209
$expectedSubRequest = Request::create('/');
204210
$expectedSubRequest->headers->set('x-forwarded-for', array('127.0.0.1'));
205211
$expectedSubRequest->headers->set('forwarded', array('for="127.0.0.1";host="localhost";proto=http'));
212+
$expectedSubRequest->server->set('HTTP_X_FORWARDED_FOR', '127.0.0.1');
213+
$expectedSubRequest->server->set('HTTP_FORWARDED', 'for="127.0.0.1";host="localhost";proto=http');
206214

207215
$strategy = new InlineFragmentRenderer($this->getKernelExpectingRequest($expectedSubRequest));
208216
$request = Request::create('/', 'GET', array(), array(), array(), array('HTTP_IF_MODIFIED_SINCE' => 'Fri, 01 Jan 2016 00:00:00 GMT', 'HTTP_IF_NONE_MATCH' => '*'));
@@ -216,6 +224,8 @@ public function testFirstTrustedProxyIsSetAsRemote()
216224
$expectedSubRequest->server->set('REMOTE_ADDR', '127.0.0.1');
217225
$expectedSubRequest->headers->set('x-forwarded-for', array('127.0.0.1'));
218226
$expectedSubRequest->headers->set('forwarded', array('for="127.0.0.1";host="localhost";proto=http'));
227+
$expectedSubRequest->server->set('HTTP_X_FORWARDED_FOR', '127.0.0.1');
228+
$expectedSubRequest->server->set('HTTP_FORWARDED', 'for="127.0.0.1";host="localhost";proto=http');
219229

220230
Request::setTrustedProxies(array('1.1.1.1'));
221231

@@ -235,6 +245,8 @@ public function testIpAddressOfRangedTrustedProxyIsSetAsRemote()
235245
$expectedSubRequest->server->set('REMOTE_ADDR', '127.0.0.1');
236246
$expectedSubRequest->headers->set('x-forwarded-for', array('127.0.0.1'));
237247
$expectedSubRequest->headers->set('forwarded', array('for="127.0.0.1";host="localhost";proto=http'));
248+
$expectedSubRequest->server->set('HTTP_X_FORWARDED_FOR', '127.0.0.1');
249+
$expectedSubRequest->server->set('HTTP_FORWARDED', 'for="127.0.0.1";host="localhost";proto=http');
238250

239251
Request::setTrustedProxies(array('1.1.1.1/24'));
240252

0 commit comments

Comments
 (0)