8000 Merge branch '3.2' · symfony/symfony@4927993 · GitHub
[go: up one dir, main page]

Skip to content

Commit 4927993

Browse files
Merge branch '3.2'
* 3.2: Fixed pathinfo calculation for requests starting with a question mark. [HttpFoundation] Fix missing handling of for/host/proto info from "Forwarded" header [Validator] Add object handling of invalid constraints in Composite [WebProfilerBundle] Remove uneeded directive in the form collector styles removed usage of $that HttpCache: New test for revalidating responses with an expired TTL [Serializer] [XML] Ignore Process Instruction [Security] simplify the SwitchUserListenerTest Revert "bug #21841 [Console] Do not squash input changes made from console.command event (chalasr)" [HttpFoundation] Fix Request::getHost() when having several hosts in X_FORWARDED_HOST
2 parents 1635a6a + f296648 commit 4927993

File tree

17 files changed

+278
-267
lines changed

17 files changed

+278
-267
lines changed

src/Symfony/Bundle/FrameworkBundle/Tests/Command/CacheClearCommand/CacheClearCommandTest.php

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,10 @@ public function testCacheIsFreshAfterCacheClearedWithWarmup()
6060
// simply check that cache is warmed up
6161
$this->assertGreaterThanOrEqual(1, count($metaFiles));
6262
$configCacheFactory = new ConfigCacheFactory(true);
63-
$that = $this;
6463

6564
foreach ($metaFiles as $file) {
66-
$configCacheFactory->cache(substr($file, 0, -5), function () use ($that, $file) {
67-
$that->fail(sprintf('Meta file "%s" is not fresh', (string) $file));
65+
$configCacheFactory->cache(substr($file, 0, -5), function () use ($file) {
66+
$this->fail(sprintf('Meta file "%s" is not fresh', (string) $file));
6867
});
6968
}
7069

src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/form.html.twig

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@
6060
}
6161
#tree-menu .empty {
6262
border: 0;
63-
margin: 0;
6463
padding: 0;
6564
}
6665
#tree-details-container {

src/Symfony/Component/Console/Application.php

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -851,10 +851,6 @@ protected function doRunCommand(Command $command, InputInterface $input, OutputI
851851
// ignore invalid options/arguments for now, to allow the event listeners to customize the InputDefinition
852852
}
853853

854-
// don't bind the input again as it would override any input argument/option set from the command event in
855-
// addition to being useless
856-
$command->setInputBound(true);
857-
858854
$event = new ConsoleCommandEvent($command, $input, $output);
859855
$this->dispatcher->dispatch(ConsoleEvents::COMMAND, $event);
860856

src/Symfony/Component/Console/Command/Command.php

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ class Command
4040
private $ignoreValidationErrors = false;
4141
private $applicationDefinitionMerged = false;
4242
private $applicationDefinitionMergedWithArgs = false;
43-
private $inputBound = false;
4443
private $code;
4544
private $synopsis = array();
4645
private $usages = array();
@@ -217,13 +216,11 @@ public function run(InputInterface $input, OutputInterface $output)
217216
$this->mergeApplicationDefinition();
218217

219218
// bind the input against the command specific arguments/options
220-
if (!$this->inputBound) {
221-
try {
222-
$input->bind($this->definition);
223-
} catch (ExceptionInterface $e) {
224-
if (!$this->ignoreValidationErrors) {
225-
throw $e;
226-
}
219+
try {
220+
$input->bind($this->definition);
221+
} catch (ExceptionInterface $e) {
222+
if (!$this->ignoreValidationErrors) {
223+
throw $e;
227224
}
228225
}
229226

@@ -652,14 +649,6 @@ public function getHelper($name)
652649
return $this->helperSet->get($name);
653650
}
654651

655-
/**
656-
* @internal
657-
*/
658-
public function setInputBound($inputBound)
659-
{
660-
$this->inputBound = $inputBound;
661-
}
662-
663652
/**
664653
* Validates a command name.
665654
*

src/Symfony/Component/Console/Tests/ApplicationTest.php

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1155,31 +1155,6 @@ public function testRunWithDispatcherAddingInputOptions()
11551155
$this->assertEquals('some test value', $extraValue);
11561156
}
11571157

1158-
public function testUpdateInputFromConsoleCommandEvent()
1159-
{
1160-
$dispatcher = $this->getDispatcher();
1161-
$dispatcher->addListener('console.command', function (ConsoleCommandEvent $event) {
1162-
$event->getInput()->setOption('extra', 'overriden');
1163-
});
1164-
1165-
$application = new Application();
1166-
$application->setDispatcher($dispatcher);
1167-
$application->setAutoExit(false);
1168-
1169-
$application
1170-
->register('foo')
1171-
->addOption('extra', null, InputOption::VALUE_REQUIRED)
1172-
->setCode(function (InputInterface $input, OutputInterface $output) {
1173-
$output->write('foo.');
1174-
})
1175-
;
1176-
1177-
$tester = new ApplicationTester($application);
1178-
$tester->run(array('command' => 'foo', '--extra' => 'original'));
1179-
1180-
$this->assertEquals('overriden', $tester->getInput()->getOption('extra'));
1181-
}
1182-
11831158
/**
11841159
* @group legacy
11851160
*/

src/Symfony/Component/HttpFoundation/Request.php

Lines changed: 67 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ class Request
221221

222222
private $isHostValid = true;
223223
private $isClientIpsValid = true;
224+
private $isForwardedValid = true;
224225

225226
private static $trustedHeaderSet = -1;
226227

@@ -233,6 +234,13 @@ class Request
233234
self::HEADER_CLIENT_PORT => 'X_FORWARDED_PORT',
234235
);
235236

237+
private static $forwardedParams = array(
238+
self::HEADER_X_FORWARDED_FOR => 'for',
239+
self::HEADER_X_FORWARDED_HOST => 'host',
240+
self::HEADER_X_FORWARDED_PROTO => 'proto',
241+
self::HEADER_X_FORWARDED_PORT => 'host',
242+
);
243+
236244
/**
237245
* Constructor.
238246
*
@@ -859,46 +867,13 @@ public function setSession(SessionInterface $session)
859867
*/
860868
public function getClientIps()
861869
{
862-
$clientIps = array();
863870
$ip = $this->server->get('REMOTE_ADDR');
864871

865872
if (!$this->isFromTrustedProxy()) {
866873
return array($ip);
867874
}
868875

869-
$hasTrustedForwardedHeader = self::$trustedHeaders[self::HEADER_FORWARDED] && $this->headers->has(self::$trustedHeaders[self::HEADER_FORWARDED]);
870-
$hasTrustedClientIpHeader = self::$trustedHeaders[self::HEADER_CLIENT_IP] && $this->headers->has(self::$trustedHeaders[self::HEADER_CLIENT_IP]);
871-
872-
if ($hasTrustedForwardedHeader) {
873-
$forwardedHeader = $this->headers->get(self::$trustedHeaders[self::HEADER_FORWARDED]);
874-
preg_match_all('{(for)=("?\[?)([a-z0-9\.:_\-/]*)}', $forwardedHeader, $matches);
875-
$forwardedClientIps = $matches[3];
876-
877-
$forwardedClientIps = $this->normalizeAndFilterClientIps($forwardedClientIps, $ip);
878-
$clientIps = $forwardedClientIps;
879-
}
880-
881-
if ($hasTrustedClientIpHeader) {
882-
$xForwardedForClientIps = array_map('trim', explode(',', $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_IP])));
883-
884-
$xForwardedForClientIps = $this->normalizeAndFilterClientIps($xForwardedForClientIps, $ip);
885-
$clientIps = $xForwardedForClientIps;
886-
}
887-
888-
if ($hasTrustedForwardedHeader && $hasTrustedClientIpHeader && $forwardedClientIps !== $xForwardedForClientIps) {
889-
if (!$this->isClientIpsValid) {
890-
return array('0.0.0.0');
891-
}
892-
$this->isClientIpsValid = false;
893-
894-
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 your project to distrust one of them.');
895-
}
896-
897-
if (!$hasTrustedForwardedHeader && !$hasTrustedClientIpHeader) {
898-
return $this->normalizeAndFilterClientIps(array(), $ip);
899-
}
900-
901-
return $clientIps;
876+
return $this->getTrustedValues(self::HEADER_CLIENT_IP, $ip) ?: array($ip);
902877
}
903878

904879
/**
@@ -1024,31 +999,25 @@ public function getScheme()
1024999
*/
10251000
public function getPort()
10261001
{
1027-
if ($this->isFromTrustedProxy()) {
1028-
if (self::$trustedHeaders[self::HEADER_CLIENT_PORT] && $port = $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_PORT])) {
1029-
return $port;
1030-
}
1031-
1032-
if (self::$trustedHeaders[self::HEADER_CLIENT_PROTO] && 'https' === $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_PROTO], 'http')) {
1033-
return 443;
1034-
}
1002+
if ($this->isFromTrustedProxy() && $host = $this->getTrustedValues(self::HEADER_CLIENT_PORT)) {
1003+
$host = $host[0];
1004+
} elseif ($this->isFromTrustedProxy() && $host = $this->getTrustedValues(self::HEADER_CLIENT_HOST)) {
1005+
$host = $host[0];
1006+
} elseif (!$host = $this->headers->get('HOST')) {
1007+
return $this->server->get('SERVER_PORT');
10351008
}
10361009

1037-
if ($host = $this->headers->get('HOST')) {
1038-
if ($host[0] === '[') {
1039-
$pos = strpos($host, ':', strrpos($host, ']'));
1040-
} else {
1041-
$pos = strrpos($host, ':');
1042-
}
1043-
1044-
if (false !== $pos) {
1045-
return (int) substr($host, $pos + 1);
1046-
}
1010+
if ($host[0] === '[') {
1011+
$pos = strpos($host, ':', strrpos($host, ']'));
1012+
} else {
1013+
$pos = strrpos($host, ':');
1014+
}
10471015

1048-
return 'https' === $this->getScheme() ? 443 : 80;
1016+
if (false !== $pos) {
1017+
return (int) substr($host, $pos + 1);
10491018
}
10501019

1051-
return $this->server->get('SERVER_PORT');
1020+
return 'https' === $this->getScheme() ? 443 : 80;
10521021
}
10531022

10541023
/**
@@ -1248,8 +1217,8 @@ public function getQueryString()
12481217
*/
12491218
public function isSecure()
12501219
{
1251-
if ($this->isFromTrustedProxy() && self::$trustedHeaders[self::HEADER_CLIENT_PROTO] && $proto = $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_PROTO])) {
1252-
return in_array(strtolower(current(explode(',', $proto))), array('https', 'on', 'ssl', '1'));
1220+
if ($this->isFromTrustedProxy() && $proto = $this->getTrustedValues(self::HEADER_CLIENT_PROTO)) {
1221+
return in_array(strtolower($proto[0]), array('https', 'on', 'ssl', '1'), true);
12531222
}
12541223

12551224
$https = $this->server->get('HTTPS');
@@ -1274,10 +1243,8 @@ public function isSecure()
12741243
*/
12751244
public function getHost()
12761245
{
1277-
if ($this->isFromTrustedProxy() && self::$trustedHeaders[self::HEADER_CLIENT_HOST] && $host = $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_HOST])) {
1278-
$elements = explode(',', $host);
1279-
1280-
$host = $elements[count($elements) - 1];
1246+
if ($this->isFromTrustedProxy() && $host = $this->getTrustedValues(self::HEADER_CLIENT_HOST)) {
1247+
$host = $host[0];
12811248
} elseif (!$host = $this->headers->get('HOST')) {
12821249
if (!$host = $this->server->get('SERVER_NAME')) {
12831250
$host = $this->server->get('SERVER_ADDR', '');
@@ -2058,8 +2025,48 @@ public function isFromTrustedProxy()
20582025
return self::$trustedProxies && IpUtils::checkIp($this->server->get('REMOTE_ADDR'), self::$trustedProxies);
20592026
}
20602027

2028+
private function getTrustedValues($type, $ip = null)
2029+
{
2030+
$clientValues = array();
2031+
$forwardedValues = array();
2032+
2033+
if (self::$trustedHeaders[$type] && $this->headers->has(self::$trustedHeaders[$type])) {
2034+
foreach (explode(',', $this->headers->get(self::$trustedHeaders[$type])) as $v) {
2035+
$clientValues[] = (self::HEADER_CLIENT_PORT === $type ? '0.0.0.0:' : '').trim($v);
2036+
}
2037+
}
2038+
2039+
if (self::$trustedHeaders[self::HEADER_FORWARDED] && $this->headers->has(self::$trustedHeaders[self::HEADER_FORWARDED])) {
2040+
$forwardedValues = $this->headers->get(self::$trustedHeaders[self::HEADER_FORWARDED]);
2041+
$forwardedValues = preg_match_all(sprintf('{(?:%s)=(?:"?\[?)([a-zA-Z0-9\.:_\-/]*+)}', self::$forwardedParams[$type]), $forwardedValues, $matches) ? $matches[1] : array();
2042+
}
2043+
2044+
if (null !== $ip) {
2045+
$clientValues = $this->normalizeAndFilterClientIps($clientValues, $ip);
2046+
$forwardedValues = $this->normalizeAndFilterClientIps($forwardedValues, $ip);
2047+
}
2048+
2049+
if ($forwardedValues === $clientValues || !$clientValues) {
2050+
return $forwardedValues;
2051+
}
2052+
2053+
if (!$forwardedValues) {
2054+
return $clientValues;
2055+
}
2056+
2057+
if (!$this->isForwardedValid) {
2058+
return null !== $ip ? array('0.0.0.0', $ip) : array();
2059+
}
2060+
$this->isForwardedValid = false;
2061+
2062+
throw new ConflictingHeadersException(sprintf('The request has both a trusted "%s" header and a trusted "%s" header, conflicting with each other. You should either configure your proxy to remove one of them, or configure your project to distrust the offending one.', self::$trustedHeaders[self::HEADER_FORWARDED], self::$trustedHeaders[$type]));
2063+
}
2064+
20612065
private function normalizeAndFilterClientIps(array $clientIps, $ip)
20622066
{
2067+
if (!$clientIps) {
2068+
return array();
2069+
}
20632070
$clientIps[] = $ip; // Complete the IP chain with the IP the request actually came from
20642071
$firstTrustedIp = null;
20652072

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

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1666,12 +1666,12 @@ private function getRequestInstanceForClientIpsForwardedTests($remoteAddr, $http
16661666
return $request;
16671667
}
16681668

1669-
public function testTrustedProxies()
1669+
public function testTrustedProxiesXForwardedFor()
16701670
{
16711671
$request = Request::create('http://example.com/');
16721672
$request->server->set('REMOTE_ADDR', '3.3.3.3');
16731673
$request->headers->set('X_FORWARDED_FOR', '1.1.1.1, 2.2.2.2');
1674-
$request->headers->set('X_FORWARDED_HOST', 'foo.example.com, real.example.com:8080');
1674+
$request->headers->set('X_FORWARDED_HOST', 'foo.example.com:1234, real.example.com:8080');
16751675
$request->headers->set('X_FORWARDED_PROTO', 'https');
16761676
$request->headers->set('X_FORWARDED_PORT', 443);
16771677

@@ -1698,7 +1698,7 @@ public function testTrustedProxies()
16981698
// trusted proxy via setTrustedProxies()
16991699
Request::setTrustedProxies(array('3.3.3.3', '2.2.2.2'), Request::HEADER_X_FORWARDED_ALL);
17001700
$this->assertEquals('1.1.1.1', $request->getClientIp());
1701-
$this->assertEquals('real.example.com', $request->getHost());
1701+
$this->assertEquals('foo.example.com', $request->getHost());
17021702
$this->assertEquals(443, $request->getPort());
17031703
$this->assertTrue($request->isSecure());
17041704

@@ -1765,6 +1765,55 @@ public function testLegacyTrustedProxies()
17651765
Request::setTrustedHeaderName(Request::HEADER_CLIENT_PROTO, 'X_FORWARDED_PROTO');
17661766
}
17671767

1768+
public function testTrustedProxiesForwarded()
1769+
{
1770+
$request = Request::create('http://example.com/');
1771+
$request->server->set('REMOTE_ADDR', '3.3.3.3');
1772+
$request->headers->set('FORWARDED', 'for=1.1.1.1, host=foo.example.com:8080, proto=https, for=2.2.2.2, host=real.example.com:8080');
1773+
1774+
// no trusted proxies
1775+
$this->assertEquals('3.3.3.3', $request->getClientIp());
1776+
$this->assertEquals('example.com', $request->getHost());
1777+
$this->assertEquals(80, $request->getPort());
1778+
$this->assertFalse($request->isSecure());
1779+
1780+
// disabling proxy trusting
1781+
Request::setTrustedProxies(array(), Request::HEADER_FORWARDED);
1782+
$this->assertEquals('3.3.3.3', $request->getClientIp());
1783+
$this->assertEquals('example.com', $request->getHost());
1784+
$this->assertEquals(80, $request->getPort());
1785+
$this->assertFalse($request->isSecure());
1786+
1787+
// request is forwarded by a non-trusted proxy
1788+
Request::setTrustedProxies(array('2.2.2.2'), Request::HEADER_FORWARDED);
1789+
$this->assertEquals('3.3.3.3', $request->getClientIp());
1790+
$this->assertEquals('example.com', $request->getHost());
1791+
$this->assertEquals(80, $request->getPort());
1792+
$this->assertFalse($request->isSecure());
1793+
1794+
// trusted proxy via setTrustedProxies()
1795+
Request::setTrustedProxies(array('3.3.3.3', '2.2.2.2'), Request::HEADER_FORWARDED);
1796+
$this->assertEquals('1.1.1.1', $request->getClientIp());
1797+
$this->assertEquals('foo.example.com', $request->getHost());
1798+
$this->assertEquals(8080, $request->getPort());
1799+
$this->assertTrue($request->isSecure());
1800+
1801+
// trusted proxy via setTrustedProxies()
1802+
Request::setTrustedProxies(array('3.3.3.4', '2.2.2.2'), Request::HEADER_FORWARDED);
1803+
$this->assertEquals('3.3.3.3', $request->getClientIp());
1804+
$this->assertEquals('example.com', $request->getHost());
1805+
$this->assertEquals(80, $request->getPort());
1806+
$this->assertFalse($request->isSecure());
1807+
1808+
// check various X_FORWARDED_PROTO header values
1809+
Request::setTrustedProxies(array('3.3.3.3', '2.2.2.2'), Request::HEADER_FORWARDED);
1810+
$request->headers->set('FORWARDED', 'proto=ssl');
1811+
$this->assertTrue($request->isSecure());
1812+
1813+
$request->headers->set('FORWARDED', 'proto=https, proto=http');
1814+
$this->assertTrue($request->isSecure());
1815+
}
1816+
17681817
/**
17691818
* @group legacy
17701819
* @expectedException \InvalidArgumentException

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public function testListenerThrowsWhenMasterRequestHasInconsistentClientIps()
3232
$request = new Request();
3333
$request->setTrustedProxies(array('1.1.1.1'), Request::HEADER_X_FORWARDED_FOR | Request::HEADER_FORWARDED);
3434
$request->server->set('REMOTE_ADDR', '1.1.1.1');
35-
$request->headers->set('FORWARDED', '2.2.2.2');
35+
$request->headers->set('FORWARDED', 'for=2.2.2.2');
3636
$request->headers->set('X_FORWARDED_FOR', '3.3.3.3');
3737

3838
$dispatcher->addListener(KernelEvents::REQUEST, array(new ValidateRequestListener(), 'onKernelRequest'));

0 commit comments

Comments
 (0)
0