Skip to content

Commit 0a7aa63

Browse files
fabpotfabpot
committed
Merge branch '2.7' into 2.8
* 2.7: [HttpFoundation] Warning when request has both Forwarded and X-Forwarded-For [Console] Decouple SymfonyStyle from TableCell
2 parents cdf7b08 + 2d37230 commit 0a7aa63

File tree

8 files changed

+278
-39
lines changed

8 files changed

+278
-39
lines changed

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/web.xml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,5 +46,9 @@
4646
<argument type="service" id="request_stack" />
4747
<tag name="kernel.event_subscriber" />
4848
</service>
49+
50+
<service id="validate_request_listener" class="Symfony\Component\HttpKernel\EventListener\ValidateRequestListener">
51+
<tag name="kernel.event_subscriber" />
52+
</service>
4953
</services>
5054
</container>

‎src/Symfony/Component/Console/Style/SymfonyStyle.php‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
use Symfony\Component\Console\Helper\ProgressBar;
1919
use Symfony\Component\Console\Helper\SymfonyQuestionHelper;
2020
use Symfony\Component\Console\Helper\Table;
21-
use Symfony\Component\Console\Helper\TableCell;
2221
use Symfony\Component\Console\Input\InputInterface;
2322
use Symfony\Component\Console\Output\BufferedOutput;
2423
use Symfony\Component\Console\Output\OutputInterface;
@@ -185,21 +184,13 @@ public function caution($message)
185184
*/
186185
public function table(array $headers, array $rows)
187186
{
188-
array_walk_recursive($headers, function (&$value) {
189-
if ($value instanceof TableCell) {
190-
$value = new TableCell(sprintf('<info>%s</>', $value), array(
191-
'colspan' => $value->getColspan(),
192-
'rowspan' => $value->getRowspan(),
193-
));
194-
} else {
195-
$value = sprintf('<info>%s</>', $value);
196-
}
197-
});
187+
$style = clone Table::getStyleDefinition('symfony-style-guide');
188+
$style->setCellHeaderFormat('<info>%s</info>');
198189

199190
$table = new Table($this);
200191
$table->setHeaders($headers);
201192
$table->setRows($rows);
202-
$table->setStyle('symfony-style-guide');
193+
$table->setStyle($style);
203194

204195
$table->render();
205196
$this->newLine();

‎src/Symfony/Component/HttpFoundation/Exception/ConflictingHeadersException.php‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpFoundation\Exception;
13+
14+
/**
15+
* The HTTP request contains headers with conflicting information.
16+
*
17+
* This exception should trigger an HTTP 400 response in your application code.
18+
*
19+
* @author Magnus Nordlander <magnus@fervo.se>
20+
*/
21+
class ConflictingHeadersException extends \RuntimeException
22+
{
23+
}

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

Lines changed: 51 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\HttpFoundation;
1313

14+
use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException;
1415
use Symfony\Component\HttpFoundation\Session\SessionInterface;
1516

1617
/**
@@ -811,41 +812,34 @@ public function getClientIps()
811812
return array($ip);
812813
}
813814

814-
if (self::$trustedHeaders[self::HEADER_FORWARDED] && $this->headers->has(self::$trustedHeaders[self::HEADER_FORWARDED])) {
815+
$hasTrustedForwardedHeader = self::$trustedHeaders[self::HEADER_FORWARDED] && $this->headers->has(self::$trustedHeaders[self::HEADER_FORWARDED]);
816+
$hasTrustedClientIpHeader = self::$trustedHeaders[self::HEADER_CLIENT_IP] && $this->headers->has(self::$trustedHeaders[self::HEADER_CLIENT_IP]);
817+
818+
if ($hasTrustedForwardedHeader) {
815819
$forwardedHeader = $this->headers->get(self::$trustedHeaders[self::HEADER_FORWARDED]);
816820
preg_match_all('{(for)=("?\[?)([a-z0-9\.:_\-/]*)}', $forwardedHeader, $matches);
817-
$clientIps = $matches[3];
818-
} elseif (self::$trustedHeaders[self::HEADER_CLIENT_IP] && $this->headers->has(self::$trustedHeaders[self::HEADER_CLIENT_IP])) {
819-
$clientIps = array_map('trim', explode(',', $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_IP])));
820-
}
821+
$forwardedClientIps = $matches[3];
821822

822-
$clientIps[] = $ip; // Complete the IP chain with the IP the request actually came from
823-
$firstTrustedIp = null;
824-
825-
foreach ($clientIps as $key => $clientIp) {
826-
// Remove port (unfortunately, it does happen)
827-
if (preg_match('{((?:\d+\.){3}\d+)\:\d+}', $clientIp, $match)) {
828-
$clientIps[$key] = $clientIp = $match[1];
829-
}
823+
$forwardedClientIps = $this->normalizeAndFilterClientIps($forwardedClientIps, $ip);
824+
$clientIps = $forwardedClientIps;
825+
}
830826

831-
if (!filter_var($clientIp, FILTER_VALIDATE_IP)) {
832-
unset($clientIps[$key]);
827+
if ($hasTrustedClientIpHeader) {
828+
$xForwardedForClientIps = array_map('trim', explode(',', $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_IP])));
833829

834-
continue;
835-
}
830+
$xForwardedForClientIps = $this->normalizeAndFilterClientIps($xForwardedForClientIps, $ip);
831+
$clientIps = $xForwardedForClientIps;
832+
}
836833

837-
if (IpUtils::checkIp($clientIp, self::$trustedProxies)) {
838-
unset($clientIps[$key]);
834+
if ($hasTrustedForwardedHeader && $hasTrustedClientIpHeader && $forwardedClientIps !== $xForwardedForClientIps) {
835+
throw new ConflictingHeadersException('The request has both a trusted Forwarded header and a trusted Client IP header, conflicting with each other with regards to the originating IP addresses of the request. This is the result of a misconfiguration. You should either configure your proxy only to send one of these headers, or configure Symfony to distrust one of them.');
836+
}
839837

840-
// Fallback to this when the client IP falls into the range of trusted proxies
841-
if (null === $firstTrustedIp) {
842-
$firstTrustedIp = $clientIp;
843-
}
844-
}
838+
if (!$hasTrustedForwardedHeader && !$hasTrustedClientIpHeader) {
839+
return $this->normalizeAndFilterClientIps(array(), $ip);
845840
}
846841

847-
// Now the IP chain contains only untrusted proxies and the client IP
848-
return $clientIps ? array_reverse($clientIps) : array($firstTrustedIp);
842+
return $clientIps;
849843
}
850844

851845
/**
@@ -1936,4 +1930,35 @@ private function isFromTrustedProxy()
19361930
{
19371931
return self::$trustedProxies && IpUtils::checkIp($this->server->get('REMOTE_ADDR'), self::$trustedProxies);
19381932
}
1933+
1934+
private function normalizeAndFilterClientIps(array $clientIps, $ip)
1935+
{
1936+
$clientIps[] = $ip; // Complete the IP chain with the IP the request actually came from
1937+
$firstTrustedIp = null;
1938+
1939+
foreach ($clientIps as $key => $clientIp) {
1940+
// Remove port (unfortunately, it does happen)
1941+
if (preg_match('{((?:\d+\.){3}\d+)\:\d+}', $clientIp, $match)) {
1942+
$clientIps[$key] = $clientIp = $match[1];
1943+
}
1944+
1945+
if (!filter_var($clientIp, FILTER_VALIDATE_IP)) {
1946+
unset($clientIps[$key]);
1947+
1948+
continue;
1949+
}
1950+
1951+
if (IpUtils::checkIp($clientIp, self::$trustedProxies)) {
1952+
unset($clientIps[$key]);
1953+
1954+
// Fallback to this when the client IP falls into the range of trusted proxies
1955+
if (null === $firstTrustedIp) {
1956+
$firstTrustedIp = $clientIp;
1957+
}
1958+
}
1959+
}
1960+
1961+
// Now the IP chain contains only untrusted proxies and the client IP
1962+
return $clientIps ? array_reverse($clientIps) : array($firstTrustedIp);
1963+
}
19391964
}

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

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -923,6 +923,74 @@ public function testGetClientIpsProvider()
923923
);
924924
}
925925

926+
/**
927+
* @expectedException \Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException
928+
* @dataProvider testGetClientIpsWithConflictingHeadersProvider
929+
*/
930+
public function testGetClientIpsWithConflictingHeaders($httpForwarded, $httpXForwardedFor)
931+
{
932+
$request = new Request();
933+
934+
$server = array(
935+
'REMOTE_ADDR' => '88.88.88.88',
936+
'HTTP_FORWARDED' => $httpForwarded,
937+
'HTTP_X_FORWARDED_FOR' => $httpXForwardedFor,
938+
);
939+
940+
Request::setTrustedProxies(array('88.88.88.88'));
941+
942+
$request->initialize(array(), array(), array(), array(), array(), $server);
943+
944+
$request->getClientIps();
945+
}
946+
947+
public function testGetClientIpsWithConflictingHeadersProvider()
948+
{
949+
// $httpForwarded $httpXForwardedFor
950+
return array(
951+
array('for=87.65.43.21', '192.0.2.60'),
952+
array('for=87.65.43.21, for=192.0.2.60', '192.0.2.60'),
953+
array('for=192.0.2.60', '192.0.2.60,87.65.43.21'),
954+
array('for="::face", for=192.0.2.60', '192.0.2.60,192.0.2.43'),
955+
array('for=87.65.43.21, for=192.0.2.60', '192.0.2.60,87.65.43.21'),
956+
);
957+
}
958+
959+
/**
960+
* @dataProvider testGetClientIpsWithAgreeingHeadersProvider
961+
*/
962+
public function testGetClientIpsWithAgreeingHeaders($httpForwarded, $httpXForwardedFor)
963+
{
964+
$request = new Request();
965+
966+
$server = array(
967+
'REMOTE_ADDR' => '88.88.88.88',
968+
'HTTP_FORWARDED' => $httpForwarded,
969+
'HTTP_X_FORWARDED_FOR' => $httpXForwardedFor,
970+
);
971+
972+
Request::setTrustedProxies(array('88.88.88.88'));
973+
974+
$request->initialize(array(), array(), array(), array(), array(), $server);
975+
976+
$request->getClientIps();
977+
978+
Request::setTrustedProxies(array());
979+
}
980+
981+
public function testGetClientIpsWithAgreeingHeadersProvider()
982+
{
983+
// $httpForwarded $httpXForwardedFor
984+
return array(
985+
array('for="192.0.2.60"', '192.0.2.60'),
986+
array('for=192.0.2.60, for=87.65.43.21', '192.0.2.60,87.65.43.21'),
987+
array('for="[::face]", for=192.0.2.60', '::face,192.0.2.60'),
988+
array('for="192.0.2.60:80"', '192.0.2.60'),
989+
array('for=192.0.2.60;proto=http;by=203.0.113.43', '192.0.2.60'),
990+
array('for="[2001:db8:cafe::17]:4711"', '2001:db8:cafe::17'),
991+
);
992+
}
993+
926994
public function testGetContentWorksTwiceInDefaultMode()
927995
{
928996
$req = new Request();

‎src/Symfony/Component/HttpKernel/EventListener/ValidateRequestListener.php‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpKernel\EventListener;
13+
14+
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
15+
use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException;
16+
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
17+
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
18+
use Symfony\Component\HttpKernel\KernelEvents;
19+
20+
/**
21+
* Validates that the headers and other information indicating the
22+
* client IP address of a request are consistent.
23+
*
24+
* @author Magnus Nordlander <magnus@fervo.se>
25+
*/
26+
class ValidateRequestListener implements EventSubscriberInterface
27+
{
28+
/**
29+
* Performs the validation.
30+
*
31+
* @param GetResponseEvent $event
32+
*/
33+
public function onKernelRequest(GetResponseEvent $event)
34+
{
35+
if ($event->isMasterRequest()) {
36+
try {
37+
// This will throw an exception if the headers are inconsistent.
38+
$event->getRequest()->getClientIps();
39+
} catch (ConflictingHeadersException $e) {
40+
throw new BadRequestHttpException('The request headers contain conflicting information regarding the origin of this request.', $e);
41+
}
42+
}
43+
}
44+
45+
/**
46+
* {@inheritdoc}
47+
*/
48+
public static function getSubscribedEvents()
49+
{
50+
return array(
51+
KernelEvents::REQUEST => array(
52+
array('onKernelRequest', 256),
53+
),
54+
);
55+
}
56+
}

‎src/Symfony/Component/HttpKernel/Profiler/Profiler.php‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\HttpKernel\Profiler;
1313

14+
use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException;
1415
use Symfony\Component\HttpFoundation\Request;
1516
use Symfony\Component\HttpFoundation\Response;
1617
use Symfony\Component\HttpKernel\DataCollector\DataCollectorInterface;
@@ -208,9 +209,13 @@ public function collect(Request $request, Response $response, \Exception $except
208209
$profile = new Profile(substr(hash('sha256', uniqid(mt_rand(), true)), 0, 6));
209210
$profile->setTime(time());
210211
$profile->setUrl($request->getUri());
211-
$profile->setIp($request->getClientIp());
212212
$profile->setMethod($request->getMethod());
213213
$profile->setStatusCode($response->getStatusCode());
214+
try {
215+
$profile->setIp($request->getClientIp());
216+
} catch (ConflictingHeadersException $e) {
217+
$profile->setIp('Unknown');
218+
}
214219

215220
$response->headers->set('X-Debug-Token', $profile->getToken());
216221

‎src/Symfony/Component/HttpKernel/Tests/EventListener/ValidateRequestListenerTest.php‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpKernel\Tests\EventListener;
13+
14+
use Symfony\Component\EventDispatcher\EventDispatcher;
15+
use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException;
16+
use Symfony\Component\HttpFoundation\Request;
17+
use Symfony\Component\HttpKernel\EventListener\ValidateRequestListener;
18+
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
19+
use Symfony\Component\HttpKernel\HttpKernelInterface;
20+
use Symfony\Component\HttpKernel\KernelEvents;
21+
22+
class ValidateRequestListenerTest extends \PHPUnit_Framework_TestCase
23+
{
24+
public function testListenerThrowsWhenMasterRequestHasInconsistentClientIps()
25+
{
26+
$dispatcher = new EventDispatcher();
27+
$kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
28+
$listener = new ValidateRequestListener();
29+
$request = $this->getMock('Symfony\Component\HttpFoundation\Request');
30+
$request->method('getClientIps')
31+
->will($this->throwException(new ConflictingHeadersException()));
32+
33+
$dispatcher->addListener(KernelEvents::REQUEST, array($listener, 'onKernelRequest'));
34+
$event = new GetResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST);
35+
36+
$this->setExpectedException('Symfony\Component\HttpKernel\Exception\BadRequestHttpException');
37+
$dispatcher->dispatch(KernelEvents::REQUEST, $event);
38+
}
39+
40+
public function testListenerDoesNothingOnValidRequests()
41+
{
42+
$dispatcher = new EventDispatcher();
43+
$kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
44+
$listener = new ValidateRequestListener();
45+
$request = $this->getMock('Symfony\Component\HttpFoundation\Request');
46+
$request->method('getClientIps')
47+
->willReturn(array('127.0.0.1'));
48+
49+
$dispatcher->addListener(KernelEvents::REQUEST, array($listener, 'onKernelRequest'));
50+
$event = new GetResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST);
51+
$dispatcher->dispatch(KernelEvents::REQUEST, $event);
52+
}
53+
54+
public function testListenerDoesNothingOnSubrequests()
55+
{
56+
$dispatcher = new EventDispatcher();
57+
$kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
58+
$listener = new ValidateRequestListener();
59+
$request = $this->getMock('Symfony\Component\HttpFoundation\Request');
60+
$request->method('getClientIps')
61+
->will($this->throwException(new ConflictingHeadersException()));
62+
63+
$dispatcher->addListener(KernelEvents::REQUEST, array($listener, 'onKernelRequest'));
64+
$event = new GetResponseEvent($kernel, $request, HttpKernelInterface::SUB_REQUEST);
65+
$dispatcher->dispatch(KernelEvents::REQUEST, $event);
66+
}
67+
}

0 commit comments

Comments
 (0)