Skip to content

Commit

Permalink
ensured that an exception is always converted to an error response (a…
Browse files Browse the repository at this point in the history
…nd that we keep the HTTP status code and headers)
  • Loading branch information
fabpot committed Jul 13, 2012
1 parent 46071f3 commit 3f05e70
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 14 deletions.
Expand Up @@ -45,7 +45,7 @@ public function showAction(FlattenException $exception, DebugLoggerInterface $lo
$templating = $this->container->get('templating');
$code = $exception->getStatusCode();

$response = $templating->renderResponse(
return $templating->renderResponse(
$this->findTemplate($templating, $format, $code, $this->container->get('kernel')->isDebug()),
array(
'status_code' => $code,
Expand All @@ -55,11 +55,6 @@ public function showAction(FlattenException $exception, DebugLoggerInterface $lo
'currentContent' => $currentContent,
)
);

$response->setStatusCode($code);
$response->headers->replace($exception->getHeaders());

return $response;
}

/**
Expand Down
Expand Up @@ -35,10 +35,6 @@ protected function setUp()
->expects($this->once())
->method('getStatusCode')
->will($this->returnValue(404));
$this->flatten
->expects($this->once())
->method('getHeaders')
->will($this->returnValue(array()));
$this->controller = new ExceptionController();
$this->kernel = $this->getMock('Symfony\\Component\\HttpKernel\\KernelInterface');
$this->templating = $this->getMockBuilder('Symfony\\Bundle\\TwigBundle\\TwigEngine')
Expand Down
18 changes: 16 additions & 2 deletions src/Symfony/Component/HttpKernel/HttpKernel.php
Expand Up @@ -13,6 +13,7 @@

use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface;
use Symfony\Component\HttpKernel\Event\FilterControllerEvent;
use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
Expand Down Expand Up @@ -191,10 +192,23 @@ private function handleException(\Exception $e, $request, $type)
throw $e;
}

$response = $event->getResponse();

// ensure that we actually have an error response
if (!$response->isClientError() && !$response->isServerError() && !$response->isRedirect()) {
if ($e instanceof HttpExceptionInterface) {
// keep the HTTP status code and headers
$response->setStatusCode($e->getStatusCode());
$response->headers->add($e->getHeaders());
} else {
$response->setStatusCode(500);
}
}

try {
return $this->filterResponse($event->getResponse(), $request, $type);
return $this->filterResponse($response, $request, $type);
} catch (\Exception $e) {
return $event->getResponse();
return $response;
}
}

Expand Down
35 changes: 34 additions & 1 deletion src/Symfony/Component/HttpKernel/Tests/HttpKernelTest.php
Expand Up @@ -14,8 +14,11 @@
use Symfony\Component\HttpKernel\HttpKernel;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\HttpKernel\Exception\MethodNotAllowedHttpException;
use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\EventDispatcher\EventDispatcher;

class HttpKernelTest extends \PHPUnit_Framework_TestCase
Expand Down Expand Up @@ -59,8 +62,38 @@ public function testHandleWhenControllerThrowsAnExceptionAndRawIsFalse()
});

$kernel = new HttpKernel($dispatcher, $this->getResolver(function () { throw new \RuntimeException('foo'); }));
$response = $kernel->handle(new Request());

$this->assertEquals('foo', $kernel->handle(new Request())->getContent());
$this->assertEquals('500', $response->getStatusCode());
$this->assertEquals('foo', $response->getContent());
}

public function testHandleExceptionWithARedirectionResponse()
{
$dispatcher = new EventDispatcher();
$dispatcher->addListener(KernelEvents::EXCEPTION, function ($event) {
$event->setResponse(new RedirectResponse('/login', 301));
});

$kernel = new HttpKernel($dispatcher, $this->getResolver(function () { throw new AccessDeniedHttpException(); }));
$response = $kernel->handle(new Request());

$this->assertEquals('301', $response->getStatusCode());
$this->assertEquals('/login', $response->headers->get('Location'));
}

public function testHandleHttpException()
{
$dispatcher = new EventDispatcher();
$dispatcher->addListener(KernelEvents::EXCEPTION, function ($event) {
$event->setResponse(new Response($event->getException()->getMessage()));
});

$kernel = new HttpKernel($dispatcher, $this->getResolver(function () { throw new MethodNotAllowedHttpException(array('POST')); }));
$response = $kernel->handle(new Request());

$this->assertEquals('405', $response->getStatusCode());
$this->assertEquals('POST', $response->headers->get('Allow'));
}

public function testHandleWhenAListenerReturnsAResponse()
Expand Down
Expand Up @@ -129,7 +129,6 @@ public function onKernelException(GetResponseForExceptionEvent $event)
$subRequest->attributes->set(SecurityContextInterface::ACCESS_DENIED_ERROR, $exception);

$response = $event->getKernel()->handle($subRequest, HttpKernelInterface::SUB_REQUEST, true);
$response->setStatusCode(403);
} else {
return;
}
Expand Down

0 comments on commit 3f05e70

Please sign in to comment.