8000 [HttpFoundation] refactored Request exceptions · symfony/symfony@32ec288 · GitHub
[go: up one dir, main page]
Skip to content

Commit 32ec288

Browse files
fabpotfabpot
committed
[HttpFoundation] refactored Request exceptions
1 parent d876809 commit 32ec288

File tree

11 files changed

+86
-26
lines changed

11 files changed

+86
-26
lines changed

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

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

1212
namespace Symfony\Component\Debug\Exception;
1313

14-
use Symfony\Component\HttpFoundation\Exception\SuspiciousOperationException;
14+
use Symfony\Component\HttpFoundation\Exception\RequestExceptionInterface;
1515
use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface;
1616

1717
/**
@@ -42,7 +42,7 @@ public static function create(\Exception $exception, $statusCode = null, array $
4242
if ($exception instanceof HttpExceptionInterface) {
4343
$statusCode = $exception->getStatusCode();
4444
$headers = array_merge($headers, $exception->getHeaders());
45-
} elseif ($exception instanceof SuspiciousOperationException) {
45+
} elseif ($exception instanceof RequestExceptionInterface) {
4646
$statusCode = 400;
4747
}
4848

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

Lines 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

Lines 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+
}
< B41A div class="DiffFileHeader-module__diff-file-header--TjXyn flex-wrap flex-sm-nowrap">

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,6 @@
1515
* Raised when a user has performed an operation that should be considered
1616
* suspicious from a security perspective.
1717
*/
18-
class SuspiciousOperationException extends \UnexpectedValueException
18+
class SuspiciousOperationException extends \UnexpectedValueException implements RequestExceptionInterface
1919
{
2020
}

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

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,9 @@ class Request
207207

208208
protected static $requestFactory;
209209

210+
private $isHostValid = true;
211+
private $isClientIpsValid = true;
212+
210213
/**
211214
* Constructor.
212215
*
@@ -820,7 +823,12 @@ public function getClientIps()
820823
}
821824

822825
if ($hasTrustedForwardedHeader && $hasTrustedClientIpHeader && $forwardedClientIps !== $xForwardedForClientIps) {
823-
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.');
824832
}
825833

826834
if (!$hasTrustedForwardedHeader && !$hasTrustedClientIpHeader) {
@@ -1199,7 +1207,7 @@ public function isSecure()
11991207
*
12001208
* @return string
12011209
*
1202-
* @throws SuspiciousOperationException when the host name is invalid
1210+
* @throws SuspiciousOperationException when the host name is invalid or not trusted
12031211
*/
12041212
public function getHost()
12051213
{
@@ -1221,7 +1229,12 @@ public function getHost()
12211229
// check that it does not contain forbidden characters (see RFC 952 and RFC 2181)
12221230
// use preg_replace() instead of preg_match() to prevent DoS attacks with long host names
12231231
if ($host && '' !== preg_replace('/(?:^\[)?[a-zA-Z0-9-:\]_]+\.?/', '', $host)) {
1224-
throw new SuspiciousOperationException(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));
12251238
}
12261239

12271240
if (count(self::$trustedHostPatterns) > 0) {
@@ -1239,7 +1252,12 @@ public function getHost()
12391252
}
12401253
}
12411254

1242-
throw new SuspiciousOperationException(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));
12431261
}
12441262

12451263
return $host;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1873,7 +1873,7 @@ public function testTrustedHosts()
18731873
$request->getHost();
18741874
$this->fail('Request::getHost() should throw an exception when host is not trusted.');
18751875
} catch (SuspiciousOperationException $e) {
1876-
$this->assertEquals('Untrusted Host "evil.com"', $e->getMessage());
1876+
$this->assertEquals('Untrusted Host "evil.com".', $e->getMessage());
18771877
}
18781878

18791879
// trusted hosts

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

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
namespace Symfony\Component\HttpKernel\EventListener;
1313

1414
use Psr\Log\LoggerInterface;
15-
use Symfony\Component\HttpFoundation\Exception\SuspiciousOperationException;
1615
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
1716
use Symfony\Component\HttpKernel\Event\FinishRequestEvent;
1817
use Symfony\Component\HttpKernel\KernelEvents;
@@ -69,11 +68,7 @@ public function __construct($matcher, RequestStack $requestStack, RequestContext
6968
private function setCurrentRequest(Request $request = null)
7069
{
7170
if (null !== $request) {
72-
try {
73-
$this->context->fromRequest($request);
74-
} catch (SuspiciousOperationException $e) {
75-
// Do nothing.
76-
}
71+
$this->context->fromRequest($request);
7772
}
7873
}
7974

< E377 div class="DiffFileHeader-module__diff-file-header--TjXyn flex-wrap flex-sm-nowrap">

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

Lines 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

Lines 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

Lines 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

Lines 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