Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 4009280

Browse filesBrowse files
fabpotfabpot
committed
feature #20962 Request exceptions (thewilkybarkid, fabpot)
This PR was merged into the 3.3-dev branch. Discussion ---------- Request exceptions | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #20389, #20615, #20662 | License | MIT | Doc PR | n/a Replaces #20389 and #20662. The idea is to generically manage 400 responses when an exception implements `RequestExceptionInterface`. The "weird" caches on the request for the host and the clients IPs allows to correctly manage exceptions in an exception listener/controller (as we are duplicating the request there, but we don't want to throw an exception there). Commits ------- 32ec288 [HttpFoundation] refactored Request exceptions d876809 Return a 400 response for suspicious operations
2 parents db9a008 + 32ec288 commit 4009280
Copy full SHA for 4009280

File tree

11 files changed

+115
-19
lines changed
Filter options

11 files changed

+115
-19
lines changed

‎src/Symfony/Component/Debug/Exception/FlattenException.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Debug/Exception/FlattenException.php
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\Debug\Exception;
1313

14+
use Symfony\Component\HttpFoundation\Exception\RequestExceptionInterface;
1415
use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface;
1516

1617
/**
@@ -41,6 +42,8 @@ public static function create(\Exception $exception, $statusCode = null, array $
4142
if ($exception instanceof HttpExceptionInterface) {
4243
$statusCode = $exception->getStatusCode();
4344
$headers = array_merge($headers, $exception->getHeaders());
45+
} elseif ($exception instanceof RequestExceptionInterface) {
46+
$statusCode = 400;
4447
}
4548

4649
if (null === $statusCode) {

‎src/Symfony/Component/Debug/Tests/Exception/FlattenExceptionTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Debug/Tests/Exception/FlattenExceptionTest.php
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\Debug\Tests\Exception;
1313

1414
use Symfony\Component\Debug\Exception\FlattenException;
15+
use Symfony\Component\HttpFoundation\Exception\SuspiciousOperationException;
1516
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
1617
use Symfony\Component\HttpKernel\Exception\MethodNotAllowedHttpException;
1718
use Symfony\Component\HttpKernel\Exception\UnauthorizedHttpException;
@@ -78,6 +79,11 @@ public function testStatusCode()
7879

7980
$flattened = FlattenException::create(new UnsupportedMediaTypeHttpException());
8081
$this->assertEquals('415', $flattened->getStatusCode());
82+
83+
if (class_exists(SuspiciousOperationException::class)) {
84+
$flattened = FlattenException::create(new SuspiciousOperationException());
85+
$this->assertEquals('400', $flattened->getStatusCode());
86+
}
8187
}
8288

8389
public function testHeadersForHttpException()

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpFoundation/Exception/ConflictingHeadersException.php
+1-3Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,8 @@
1414
/**
1515
* The HTTP request contains headers with conflicting information.
1616
*
17-
* This exception should trigger an HTTP 400 response in your application code.
18-
*
1917
* @author Magnus Nordlander <magnus@fervo.se>
2018
*/
21-
class ConflictingHeadersException extends \RuntimeException
19+
class ConflictingHeadersException extends \UnexpectedValueException implements RequestExceptionInterface
2220
{
2321
}

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

Copy file name to clipboard
+21Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
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+
* Interface for Request exceptions.
16+
*
17+
* Exceptions implementing this interface should trigger an HTTP 400 response in the application code.
18+
*/
19+
interface RequestExceptionInterface
20+
{
21+
}

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

Copy file name to clipboard
+20Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
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+
* Raised when a user has performed an operation that should be considered
16+
* suspicious from a security perspective.
17+
*/
18+
class SuspiciousOperationException extends \UnexpectedValueException implements RequestExceptionInterface
19+
{
20+
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpFoundation/Request.php
+23-4Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\HttpFoundation;
1313

1414
use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException;
15+
use Symfony\Component\HttpFoundation\Exception\SuspiciousOperationException;
1516
use Symfony\Component\HttpFoundation\Session\SessionInterface;
1617

1718
/**
@@ -206,6 +207,9 @@ class Request
206207

207208
protected static $requestFactory;
208209

210+
private $isHostValid = true;
211+
private $isClientIpsValid = true;
212+
209213
/**
210214
* Constructor.
211215
*
@@ -819,7 +823,12 @@ public function getClientIps()
819823
}
820824

821825
if ($hasTrustedForwardedHeader && $hasTrustedClientIpHeader && $forwardedClientIps !== $xForwardedForClientIps) {
822-
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.');
826+
if (!$this->isClientIpsValid) {
827+
return array('0.0.0.0');
828+
}
829+
$this->isClientIpsValid = false;
830+
831+
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.');
823832
}
824833

825834
if (!$hasTrustedForwardedHeader && !$hasTrustedClientIpHeader) {
@@ -1198,7 +1207,7 @@ public function isSecure()
11981207
*
11991208
* @return string
12001209
*
1201-
* @throws \UnexpectedValueException when the host name is invalid
1210+
* @throws SuspiciousOperationException when the host name is invalid or not trusted
12021211
*/
12031212
public function getHost()
12041213
{
@@ -1220,7 +1229,12 @@ public function getHost()
12201229
// check that it does not contain forbidden characters (see RFC 952 and RFC 2181)
12211230
// use preg_replace() instead of preg_match() to prevent DoS attacks with long host names
12221231
if ($host && '' !== preg_replace('/(?:^\[)?[a-zA-Z0-9-:\]_]+\.?/', '', $host)) {
1223-
throw new \UnexpectedValueException(sprintf('Invalid Host "%s"', $host));
1232+
if (!$this->isHostValid) {
1233+
return '';
1234+
}
1235+
$this->isHostValid = false;
1236+
1237+
throw new SuspiciousOperationException(sprintf('Invalid Host "%s".', $host));
12241238
}
12251239

12261240
if (count(self::$trustedHostPatterns) > 0) {
@@ -1238,7 +1252,12 @@ public function getHost()
12381252
}
12391253
}
12401254

1241-
throw new \UnexpectedValueException(sprintf('Untrusted Host "%s"', $host));
1255+
if (!$this->isHostValid) {
1256+
return '';
1257+
}
1258+
$this->isHostValid = false;
1259+
1260+
throw new SuspiciousOperationException(sprintf('Untrusted Host "%s".', $host));
12421261
}
12431262

12441263
return $host;

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpFoundation/Tests/RequestTest.php
+4-3Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\HttpFoundation\Tests;
1313

14+
use Symfony\Component\HttpFoundation\Exception\SuspiciousOperationException;
1415
use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage;
1516
use Symfony\Component\HttpFoundation\Session\Session;
1617
use Symfony\Component\HttpFoundation\Request;
@@ -1871,8 +1872,8 @@ public function testTrustedHosts()
18711872
try {
18721873
$request->getHost();
18731874
$this->fail('Request::getHost() should throw an exception when host is not trusted.');
1874-
} catch (\UnexpectedValueException $e) {
1875-
$this->assertEquals('Untrusted Host "evil.com"', $e->getMessage());
1875+
} catch (SuspiciousOperationException $e) {
1876+
$this->assertEquals('Untrusted Host "evil.com".', $e->getMessage());
18761877
}
18771878

18781879
// trusted hosts
@@ -1935,7 +1936,7 @@ public function testHostValidity($host, $isValid, $expectedHost = null, $expecte
19351936
$this->assertSame($expectedPort, $request->getPort());
19361937
}
19371938
} else {
1938-
$this->setExpectedException('UnexpectedValueException', 'Invalid Host');
1939+
$this->setExpectedException(SuspiciousOperationException::class, 'Invalid Host');
19391940
$request->getHost();
19401941
}
19411942
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/EventListener/ValidateRequestListener.php
+3-3Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@
1616
use Symfony\Component\HttpKernel\KernelEvents;
1717

1818
/**
19-
* Validates that the headers and other information indicating the
20-
* client IP address of a request are consistent.
19+
* Validates Requests.
2120
*
2221
* @author Magnus Nordlander <magnus@fervo.se>
2322
*/
@@ -36,9 +35,10 @@ public function onKernelRequest(GetResponseEvent $event)
3635
$request = $event->getRequest();
3736

3837
if ($request::getTrustedProxies()) {
39-
// This will throw an exception if the headers are inconsistent.
4038
$request->getClientIps();
4139
}
40+
41+
$request->getHost();
4242
}
4343

4444
/**

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/HttpKernel.php
+3-3Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
use Symfony\Component\HttpKernel\Event\GetResponseForControllerResultEvent;
2626
use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
2727
use Symfony\Component\HttpKernel\Event\PostResponseEvent;
28-
use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException;
28+
use Symfony\Component\HttpFoundation\Exception\RequestExceptionInterface;
2929
use Symfony\Component\HttpFoundation\Request;
3030
use Symfony\Component\HttpFoundation\RequestStack;
3131
use Symfony\Component\HttpFoundation\Response;
@@ -67,8 +67,8 @@ public function handle(Request $request, $type = HttpKernelInterface::MASTER_REQ
6767
try {
6868
return $this->handleRaw($request, $type);
6969
} catch (\Exception $e) {
70-
if ($e instanceof ConflictingHeadersException) {
71-
$e = new BadRequestHttpException('The request headers contain conflicting information regarding the origin of this request.', $e);
70+
if ($e instanceof RequestExceptionInterface) {
71+
$e = new BadRequestHttpException($e->getMessage(), $e);
7272
}
7373
if (false === $catch) {
7474
$this->finishRequest($request, $type);

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/Tests/EventListener/RouterListenerTest.php
+30-2Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,17 @@
99
* file that was distributed with this source code.
1010
*/
1111

12-
namespace Symfony\Component\HttpKernel\Tests\EventListener;
13-
12+
use Symfony\Component\EventDispatcher\EventDispatcher;
1413
use Symfony\Component\HttpFoundation\Request;
14+
use Symfony\Component\HttpFoundation\RequestStack;
15+
use Symfony\Component\HttpFoundation\Response;
16+
use Symfony\Component\HttpKernel\Controller\ArgumentResolver;
17+
use Symfony\Component\HttpKernel\Controller\ControllerResolver;
18+
use Symfony\Component\HttpKernel\EventListener\ExceptionListener;
1519
use Symfony\Component\HttpKernel\EventListener\RouterListener;
20+
use Symfony\Component\HttpKernel\EventListener\ValidateRequestListener;
1621
use Symfony\Component\HttpKernel\HttpKernelInterface;
22+
use Symfony\Component\HttpKernel\HttpKernel;
1723
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
1824
use Symfony\Component\Routing\RequestContext;
1925

@@ -154,4 +160,26 @@ public function getLoggingParameterData()
154160
array(array(), 'Matched route "{route}".', array('route' => 'n/a', 'route_parameters' => array(), 'request_uri' => 'http://localhost/', 'method' => 'GET')),
155161
);
156162
}
163+
164+
public function testWithBadRequest()
165+
{
166+
$requestStack = new RequestStack();
167+
168+
$requestMatcher = $this->getMock('Symfony\Component\Routing\Matcher\RequestMatcherInterface');
169+
$requestMatcher->expects($this->never())->method('matchRequest');
170+
171+
$dispatcher = new EventDispatcher();
172+
$dispatcher->addSubscriber(new ValidateRequestListener());
173+
$dispatcher->addSubscriber(new RouterListener($requestMatcher, $requestStack, new RequestContext()));
174+
$dispatcher->addSubscriber(new ExceptionListener(function () {
175+
return new Response('Exception handled', 400);
176+
}));
177+
178+
$kernel = new HttpKernel($dispatcher, new ControllerResolver(), $requestStack, new ArgumentResolver());
179+
180+
$request = Request::create('http://localhost/');
181+
$request->headers->set('host', '###');
182+
$response = $kernel->handle($request);
183+
$this->assertSame(400, $response->getStatusCode());
184+
}
157185
}

‎src/Symfony/Component/HttpKernel/composer.json

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/composer.json
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
"require": {
1919
"php": ">=5.5.9",
2020
"symfony/event-dispatcher": "~2.8|~3.0",
21-
"symfony/http-foundation": "~2.8.13|~3.1.6|~3.2",
21+
"symfony/http-foundation": "~3.3",
2222
"symfony/debug": "~2.8|~3.0",
2323
"psr/log": "~1.0"
2424
},

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.