Skip to content

Commit

Permalink
bug #25532 [HttpKernel] Disable CSP header on exception pages (ostrol…
Browse files Browse the repository at this point in the history
…ucky)

This PR was merged into the 2.7 branch.

Discussion
----------

[HttpKernel] Disable CSP header on exception pages

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #24772
| License       | MIT
| Doc PR        | -

This makes exception pages styled normally when using CSP listener. Technically it's new feature, but from user POV it's bugfix, because it's really confusing to get a blank page. It takes a while to realize it is because of enabled CSP. Therefore I'm trying to push it as patch release.

Commits
-------

f33a383 [HttpKernel] Disable CSP header on exception pages
  • Loading branch information
fabpot committed Jan 4, 2018
2 parents 62c11e5 + f33a383 commit 395eb0f
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@

use Psr\Log\LoggerInterface;
use Symfony\Component\Debug\Exception\FlattenException;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
use Symfony\Component\HttpKernel\Log\DebugLoggerInterface;
use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
use Symfony\Component\HttpKernel\KernelEvents;
Expand Down Expand Up @@ -41,6 +43,7 @@ public function onKernelException(GetResponseForExceptionEvent $event)
{
$exception = $event->getException();
$request = $event->getRequest();
$eventDispatcher = func_num_args() > 2 ? func_get_arg(2) : null;

$this->logException($exception, sprintf('Uncaught PHP Exception %s: "%s" at %s line %s', get_class($exception), $exception->getMessage(), $exception->getFile(), $exception->getLine()));

Expand All @@ -67,6 +70,14 @@ public function onKernelException(GetResponseForExceptionEvent $event)
}

$event->setResponse($response);

if ($eventDispatcher instanceof EventDispatcherInterface) {
$cspRemovalListener = function (FilterResponseEvent $event) use (&$cspRemovalListener, $eventDispatcher) {
$event->getResponse()->headers->remove('Content-Security-Policy');
$eventDispatcher->removeListener(KernelEvents::RESPONSE, $cspRemovalListener);
};
$eventDispatcher->addListener(KernelEvents::RESPONSE, $cspRemovalListener, -128);
}
}

public static function getSubscribedEvents()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,11 @@
namespace Symfony\Component\HttpKernel\Tests\EventListener;

use PHPUnit\Framework\TestCase;
use Symfony\Component\EventDispatcher\EventDispatcher;
use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\HttpKernel\EventListener\ExceptionListener;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\HttpKernel\Log\DebugLoggerInterface;
use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
use Symfony\Component\HttpFoundation\Request;
Expand Down Expand Up @@ -122,6 +125,32 @@ public function testSubRequestFormat()
$response = $event->getResponse();
$this->assertEquals('xml', $response->getContent());
}

public function testCSPHeaderIsRemoved()
{
$dispatcher = new EventDispatcher();
$kernel = $this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock();
$kernel->expects($this->once())->method('handle')->will($this->returnCallback(function (Request $request) {
return new Response($request->getRequestFormat());
}));

$listener = new ExceptionListener('foo', $this->getMockBuilder('Psr\Log\LoggerInterface')->getMock());

$dispatcher->addSubscriber($listener);

$request = Request::create('/');
$event = new GetResponseForExceptionEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST, new \Exception('foo'));
$dispatcher->dispatch(KernelEvents::EXCEPTION, $event);

$response = new Response('', 200, array('content-security-policy' => "style-src 'self'"));
$this->assertTrue($response->headers->has('content-security-policy'));

$event = new FilterResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST, $response);
$dispatcher->dispatch(KernelEvents::RESPONSE, $event);

$this->assertFalse($response->headers->has('content-security-policy'), 'CSP header has been removed');
$this->assertFalse($dispatcher->hasListeners(KernelEvents::RESPONSE), 'CSP removal listener has been removed');
}
}

class TestLogger extends Logger implements DebugLoggerInterface
Expand Down

0 comments on commit 395eb0f

Please sign in to comment.