[HttpFoundation] refactored Request exceptions

This commit is contained in:
Fabien Potencier 2016-12-14 09:55:21 +01:00
parent d876809cec
commit 32ec28857a
11 changed files with 86 additions and 26 deletions

View File

@ -11,7 +11,7 @@
namespace Symfony\Component\Debug\Exception;
use Symfony\Component\HttpFoundation\Exception\SuspiciousOperationException;
use Symfony\Component\HttpFoundation\Exception\RequestExceptionInterface;
use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface;
/**
@ -42,7 +42,7 @@ class FlattenException
if ($exception instanceof HttpExceptionInterface) {
$statusCode = $exception->getStatusCode();
$headers = array_merge($headers, $exception->getHeaders());
} elseif ($exception instanceof SuspiciousOperationException) {
} elseif ($exception instanceof RequestExceptionInterface) {
$statusCode = 400;
}

View File

@ -14,10 +14,8 @@ namespace Symfony\Component\HttpFoundation\Exception;
/**
* The HTTP request contains headers with conflicting information.
*
* This exception should trigger an HTTP 400 response in your application code.
*
* @author Magnus Nordlander <magnus@fervo.se>
*/
class ConflictingHeadersException extends \RuntimeException
class ConflictingHeadersException extends \UnexpectedValueException implements RequestExceptionInterface
{
}

View File

@ -0,0 +1,21 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Component\HttpFoundation\Exception;
/**
* Interface for Request exceptions.
*
* Exceptions implementing this interface should trigger an HTTP 400 response in the application code.
*/
interface RequestExceptionInterface
{
}

View File

@ -15,6 +15,6 @@ namespace Symfony\Component\HttpFoundation\Exception;
* Raised when a user has performed an operation that should be considered
* suspicious from a security perspective.
*/
class SuspiciousOperationException extends \UnexpectedValueException
class SuspiciousOperationException extends \UnexpectedValueException implements RequestExceptionInterface
{
}

View File

@ -207,6 +207,9 @@ class Request
protected static $requestFactory;
private $isHostValid = true;
private $isClientIpsValid = true;
/**
* Constructor.
*
@ -820,7 +823,12 @@ class Request
}
if ($hasTrustedForwardedHeader && $hasTrustedClientIpHeader && $forwardedClientIps !== $xForwardedForClientIps) {
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.');
if (!$this->isClientIpsValid) {
return array('0.0.0.0');
}
$this->isClientIpsValid = false;
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.');
}
if (!$hasTrustedForwardedHeader && !$hasTrustedClientIpHeader) {
@ -1199,7 +1207,7 @@ class Request
*
* @return string
*
* @throws SuspiciousOperationException when the host name is invalid
* @throws SuspiciousOperationException when the host name is invalid or not trusted
*/
public function getHost()
{
@ -1221,7 +1229,12 @@ class Request
// check that it does not contain forbidden characters (see RFC 952 and RFC 2181)
// use preg_replace() instead of preg_match() to prevent DoS attacks with long host names
if ($host && '' !== preg_replace('/(?:^\[)?[a-zA-Z0-9-:\]_]+\.?/', '', $host)) {
throw new SuspiciousOperationException(sprintf('Invalid Host "%s"', $host));
if (!$this->isHostValid) {
return '';
}
$this->isHostValid = false;
throw new SuspiciousOperationException(sprintf('Invalid Host "%s".', $host));
}
if (count(self::$trustedHostPatterns) > 0) {
@ -1239,7 +1252,12 @@ class Request
}
}
throw new SuspiciousOperationException(sprintf('Untrusted Host "%s"', $host));
if (!$this->isHostValid) {
return '';
}
$this->isHostValid = false;
throw new SuspiciousOperationException(sprintf('Untrusted Host "%s".', $host));
}
return $host;

View File

@ -1873,7 +1873,7 @@ class RequestTest extends \PHPUnit_Framework_TestCase
$request->getHost();
$this->fail('Request::getHost() should throw an exception when host is not trusted.');
} catch (SuspiciousOperationException $e) {
$this->assertEquals('Untrusted Host "evil.com"', $e->getMessage());
$this->assertEquals('Untrusted Host "evil.com".', $e->getMessage());
}
// trusted hosts

View File

@ -12,7 +12,6 @@
namespace Symfony\Component\HttpKernel\EventListener;
use Psr\Log\LoggerInterface;
use Symfony\Component\HttpFoundation\Exception\SuspiciousOperationException;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\Event\FinishRequestEvent;
use Symfony\Component\HttpKernel\KernelEvents;
@ -69,11 +68,7 @@ class RouterListener implements EventSubscriberInterface
private function setCurrentRequest(Request $request = null)
{
if (null !== $request) {
try {
$this->context->fromRequest($request);
} catch (SuspiciousOperationException $e) {
// Do nothing.
}
$this->context->fromRequest($request);
}
}

View File

@ -16,8 +16,7 @@ use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\KernelEvents;
/**
* Validates that the headers and other information indicating the
* client IP address of a request are consistent.
* Validates Requests.
*
* @author Magnus Nordlander <magnus@fervo.se>
*/
@ -36,9 +35,10 @@ class ValidateRequestListener implements EventSubscriberInterface
$request = $event->getRequest();
if ($request::getTrustedProxies()) {
// This will throw an exception if the headers are inconsistent.
$request->getClientIps();
}
$request->getHost();
}
/**

View File

@ -25,7 +25,7 @@ use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\Event\GetResponseForControllerResultEvent;
use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
use Symfony\Component\HttpKernel\Event\PostResponseEvent;
use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException;
use Symfony\Component\HttpFoundation\Exception\RequestExceptionInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\HttpFoundation\Response;
@ -67,8 +67,8 @@ class HttpKernel implements HttpKernelInterface, TerminableInterface
try {
return $this->handleRaw($request, $type);
} catch (\Exception $e) {
if ($e instanceof ConflictingHeadersException) {
$e = new BadRequestHttpException('The request headers contain conflicting information regarding the origin of this request.', $e);
if ($e instanceof RequestExceptionInterface) {
$e = new BadRequestHttpException($e->getMessage(), $e);
}
if (false === $catch) {
$this->finishRequest($request, $type);

View File

@ -9,11 +9,17 @@
* file that was distributed with this source code.
*/
namespace Symfony\Component\HttpKernel\Tests\EventListener;
use Symfony\Component\EventDispatcher\EventDispatcher;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Controller\ArgumentResolver;
use Symfony\Component\HttpKernel\Controller\ControllerResolver;
use Symfony\Component\HttpKernel\EventListener\ExceptionListener;
use Symfony\Component\HttpKernel\EventListener\RouterListener;
use Symfony\Component\HttpKernel\EventListener\ValidateRequestListener;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\HttpKernel\HttpKernel;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\Routing\RequestContext;
@ -154,4 +160,26 @@ class RouterListenerTest extends \PHPUnit_Framework_TestCase
array(array(), 'Matched route "{route}".', array('route' => 'n/a', 'route_parameters' => array(), 'request_uri' => 'http://localhost/', 'method' => 'GET')),
);
}
public function testWithBadRequest()
{
$requestStack = new RequestStack();
$requestMatcher = $this->getMock('Symfony\Component\Routing\Matcher\RequestMatcherInterface');
$requestMatcher->expects($this->never())->method('matchRequest');
$dispatcher = new EventDispatcher();
$dispatcher->addSubscriber(new ValidateRequestListener());
$dispatcher->addSubscriber(new RouterListener($requestMatcher, $requestStack, new RequestContext()));
$dispatcher->addSubscriber(new ExceptionListener(function () {
return new Response('Exception handled', 400);
}));
$kernel = new HttpKernel($dispatcher, new ControllerResolver(), $requestStack, new ArgumentResolver());
$request = Request::create('http://localhost/');
$request->headers->set('host', '###');
$response = $kernel->handle($request);
$this->assertSame(400, $response->getStatusCode());
}
}

View File

@ -18,7 +18,7 @@
"require": {
"php": ">=5.5.9",
"symfony/event-dispatcher": "~2.8|~3.0",
"symfony/http-foundation": "~2.8.13|~3.1.6|~3.2",
"symfony/http-foundation": "~3.3",
"symfony/debug": "~2.8|~3.0",
"psr/log": "~1.0"
},