Commit 4838c5d5 authored by alexpott's avatar alexpott
Browse files

Issue #2362517 by Wim Leers: Improve default 403/404 exception HTML...

Issue #2362517 by Wim Leers: Improve default 403/404 exception HTML subscriber: don't duplicate the page render pipeline, use the routing system — add system.403 and system.404 routes.
parent 168dccd6
......@@ -774,7 +774,7 @@ services:
class: Drupal\Core\EventSubscriber\DefaultExceptionHtmlSubscriber
tags:
- { name: event_subscriber }
arguments: ['@html_fragment_renderer', '@html_page_renderer']
arguments: ['@http_kernel', '@logger.channel.php']
exception.default:
class: Drupal\Core\EventSubscriber\DefaultExceptionSubscriber
tags:
......
......@@ -9,17 +9,15 @@
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Path\AliasManagerInterface;
use Drupal\Core\Utility\Error;
use Psr\Log\LoggerInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
use Symfony\Component\HttpKernel\HttpKernelInterface;
/**
* Exception subscriber for handling core custom error pages.
* Exception subscriber for handling core custom HTML error pages.
*/
class CustomPageExceptionHtmlSubscriber extends HttpExceptionSubscriberBase {
class CustomPageExceptionHtmlSubscriber extends DefaultExceptionHtmlSubscriber {
/**
* The configuration factory.
......@@ -35,20 +33,6 @@ class CustomPageExceptionHtmlSubscriber extends HttpExceptionSubscriberBase {
*/
protected $aliasManager;
/**
* The HTTP kernel.
*
* @var \Symfony\Component\HttpKernel\HttpKernelInterface
*/
protected $httpKernel;
/**
* The logger instance.
*
* @var \Psr\Log\LoggerInterface
*/
protected $logger;
/**
* Constructs a new CustomPageExceptionHtmlSubscriber.
*
......@@ -62,10 +46,9 @@ class CustomPageExceptionHtmlSubscriber extends HttpExceptionSubscriberBase {
* The logger service.
*/
public function __construct(ConfigFactoryInterface $config_factory, AliasManagerInterface $alias_manager, HttpKernelInterface $http_kernel, LoggerInterface $logger) {
parent::__construct($http_kernel, $logger);
$this->configFactory = $config_factory;
$this->aliasManager = $alias_manager;
$this->httpKernel = $http_kernel;
$this->logger = $logger;
}
/**
......@@ -76,17 +59,7 @@ protected static function getPriority() {
}
/**
* {@inheritDoc}
*/
protected function getHandledFormats() {
return ['html'];
}
/**
* Handles a 403 error for HTML.
*
* @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event
* The event to process.
* {@inheritdoc}
*/
public function on403(GetResponseForExceptionEvent $event) {
$path = $this->aliasManager->getPathByAlias($this->configFactory->get('system.site')->get('page.403'));
......@@ -94,56 +67,11 @@ public function on403(GetResponseForExceptionEvent $event) {
}
/**
* Handles a 404 error for HTML.
*
* @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event
* The event to process.
* {@inheritdoc}
*/
public function on404(GetResponseForExceptionEvent $event) {
$path = $this->aliasManager->getPathByAlias($this->configFactory->get('system.site')->get('page.404'));
$this->makeSubrequest($event, $path, Response::HTTP_NOT_FOUND);
}
/**
* Makes a subrequest to retrieve a custom error page.
*
* @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event
* The event to process
* @param string $path
* The path to which to make a subrequest for this error message.
* @param int $status_code
* The status code for the error being handled.
*/
protected function makeSubrequest(GetResponseForExceptionEvent $event, $path, $status_code) {
$request = $event->getRequest();
// @todo Remove dependency on the internal _system_path attribute:
// https://www.drupal.org/node/2293523.
$system_path = $request->attributes->get('_system_path');
if ($path && $path != $system_path) {
// @todo The create() method expects a slash-prefixed path, but we store a
// normal system path in the site_404 variable.
if ($request->getMethod() === 'POST') {
$sub_request = Request::create($request->getBaseUrl() . '/' . $path, 'POST', ['destination' => $system_path, '_exception_statuscode' => $status_code] + $request->request->all(), $request->cookies->all(), [], $request->server->all());
}
else {
$sub_request = Request::create($request->getBaseUrl() . '/' . $path, 'GET', $request->query->all() + ['destination' => $system_path, '_exception_statuscode' => $status_code], $request->cookies->all(), [], $request->server->all());
}
try {
$response = $this->httpKernel->handle($sub_request, HttpKernelInterface::SUB_REQUEST);
$response->setStatusCode($status_code);
$event->setResponse($response);
}
catch (\Exception $e) {
// If an error happened in the subrequest we can't do much else.
// Instead, just log it. The DefaultExceptionHandler will catch the
// original exception and handle it normally.
$error = Error::decodeException($e);
$this->logger->log($error['severity_level'], '%type: !message in %function (line %line of %file).', $error);
}
}
}
}
......@@ -7,44 +7,43 @@
namespace Drupal\Core\EventSubscriber;
use Drupal\Core\Page\HtmlFragment;
use Drupal\Core\Page\HtmlFragmentRendererInterface;
use Drupal\Core\Page\HtmlPageRendererInterface;
use Drupal\Core\StringTranslation\StringTranslationTrait;
use Drupal\Core\Utility\Error;
use Psr\Log\LoggerInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
use Symfony\Component\HttpKernel\HttpKernelInterface;
/**
* Handle most HTTP errors for HTML.
* Exception subscriber for handling core default HTML error pages.
*/
class DefaultExceptionHtmlSubscriber extends HttpExceptionSubscriberBase {
use StringTranslationTrait;
/**
* The HTML fragment renderer.
* The HTTP kernel.
*
* @var \Drupal\Core\Page\HtmlFragmentRendererInterface
* @var \Symfony\Component\HttpKernel\HttpKernelInterface
*/
protected $fragmentRenderer;
protected $httpKernel;
/**
* The HTML page renderer.
* The logger instance.
*
* @var \Drupal\Core\Page\HtmlPageRendererInterface
* @var \Psr\Log\LoggerInterface
*/
protected $htmlPageRenderer;
protected $logger;
/**
* Constructs a new DefaultExceptionHtmlSubscriber.
*
* @param \Drupal\Core\Page\HtmlFragmentRendererInterface $fragment_renderer
* The fragment renderer.
* @param \Drupal\Core\Page\HtmlPageRendererInterface $page_renderer
* The page renderer.
* @param \Symfony\Component\HttpKernel\HttpKernelInterface $http_kernel
* The HTTP kernel.
* @param \Psr\Log\LoggerInterface $logger
* The logger service.
*/
public function __construct(HtmlFragmentRendererInterface $fragment_renderer, HtmlPageRendererInterface $page_renderer) {
$this->fragmentRenderer = $fragment_renderer;
$this->htmlPageRenderer = $page_renderer;
public function __construct(HttpKernelInterface $http_kernel, LoggerInterface $logger) {
$this->httpKernel = $http_kernel;
$this->logger = $logger;
}
/**
......@@ -70,9 +69,7 @@ protected function getHandledFormats() {
* The event to process.
*/
public function on403(GetResponseForExceptionEvent $event) {
$response = $this->createResponse($this->t('Access denied'), $this->t('You are not authorized to access this page.'), Response::HTTP_FORBIDDEN);
$response->headers->set('Content-type', 'text/html');
$event->setResponse($response);
$this->makeSubrequest($event, 'system/403', Response::HTTP_FORBIDDEN);
}
/**
......@@ -82,40 +79,50 @@ public function on403(GetResponseForExceptionEvent $event) {
* The event to process.
*/
public function on404(GetResponseForExceptionEvent $event) {
$path = $event->getRequest()->getPathInfo();
$response = $this->createResponse($this->t('Page not found'), $this->t('The requested page "@path" could not be found.', ['@path' => $path]), Response::HTTP_NOT_FOUND);
$response->headers->set('Content-type', 'text/html');
$event->setResponse($response);
$this->makeSubrequest($event, 'system/404', Response::HTTP_NOT_FOUND);
}
/**
* Handles a 405 error for HTML.
* Makes a subrequest to retrieve the default error page.
*
* @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event
* The event to process.
* The event to process
* @param string $path
* The path to which to make a subrequest for this error message.
* @param int $status_code
* The status code for the error being handled.
*/
public function on405(GetResponseForExceptionEvent $event) {
$response = new Response('Method Not Allowed', Response::HTTP_METHOD_NOT_ALLOWED);
$response->headers->set('Content-type', 'text/html');
$event->setResponse($response);
}
protected function makeSubrequest(GetResponseForExceptionEvent $event, $path, $status_code) {
$request = $event->getRequest();
/**
* @param $title
* The page title of the response.
* @param $body
* The body of the error page.
* @param $response_code
* The HTTP response code of the response.
* @return Response
* An error Response object ready to return to the browser.
*/
protected function createResponse($title, $body, $response_code) {
$fragment = new HtmlFragment($body);
$fragment->setTitle($title);
// @todo Remove dependency on the internal _system_path attribute:
// https://www.drupal.org/node/2293523.
$system_path = $request->attributes->get('_system_path');
if ($path && $path != $system_path) {
if ($request->getMethod() === 'POST') {
$sub_request = Request::create($request->getBaseUrl() . '/' . $path, 'POST', ['destination' => $system_path, '_exception_statuscode' => $status_code] + $request->request->all(), $request->cookies->all(), [], $request->server->all());
}
else {
$sub_request = Request::create($request->getBaseUrl() . '/' . $path, 'GET', $request->query->all() + ['destination' => $system_path, '_exception_statuscode' => $status_code], $request->cookies->all(), [], $request->server->all());
}
try {
// Persist the 'exception' attribute to the subrequest.
$sub_request->attributes->set('exception', $request->attributes->get('exception'));
$page = $this->fragmentRenderer->render($fragment, $response_code);
return new Response($this->htmlPageRenderer->render($page), $page->getStatusCode());
$response = $this->httpKernel->handle($sub_request, HttpKernelInterface::SUB_REQUEST);
$response->setStatusCode($status_code);
$event->setResponse($response);
}
catch (\Exception $e) {
// If an error happened in the subrequest we can't do much else. Instead,
// just log it. The DefaultExceptionSubscriber will catch the original
// exception and handle it normally.
$error = Error::decodeException($e);
$this->logger->log($error['severity_level'], '%type: !message in %function (line %line of %file).', $error);
}
}
}
}
......@@ -47,9 +47,6 @@ function testBlockVisibility() {
$this->drupalGet('user');
$this->assertNoText($title, 'Block was not displayed according to block visibility rules.');
$this->drupalGet('USER/' . $this->adminUser->id());
$this->assertNoText($title, 'Block was not displayed according to block visibility rules regardless of path case.');
// Confirm that the block is not displayed to anonymous users.
$this->drupalLogout();
$this->drupalGet('');
......
......@@ -55,7 +55,7 @@ public function testDelete() {
// Try to delete an entity that does not exist.
$response = $this->httpRequest($entity_type . '/9999', 'DELETE');
$this->assertResponse(404);
$this->assertText('The requested page "/' . $entity_type . '/9999" could not be found.');
$this->assertText('The requested page could not be found.');
// Try to delete an entity without proper permissions.
$this->drupalLogout();
......
<?php
/**
* @file
* Contains \Drupal\system\Controller\Http4xxController.
*/
namespace Drupal\system\Controller;
use Drupal\Core\Controller\ControllerBase;
/**
* Controller for default HTTP 4xx responses.
*/
class Http4xxController extends ControllerBase {
/**
* The default 403 content.
*
* @return array
* A render array containing the message to display for 404 pages.
*/
public function on403() {
return [
'#markup' => $this->t('You are not authorized to access this page.'),
];
}
/**
* The default 404 content.
*
* @return array
* A render array containing the message to display for 404 pages.
*/
public function on404() {
return [
'#markup' => $this->t('The requested page could not be found.'),
];
}
}
......@@ -77,14 +77,14 @@ public function testHtml403() {
/** @var \Symfony\Component\HttpKernel\HttpKernelInterface $kernel */
$kernel = \Drupal::getContainer()->get('http_kernel');
$response = $kernel->handle($request);
$response = $kernel->handle($request)->prepare($request);
$this->assertEqual($response->getStatusCode(), Response::HTTP_FORBIDDEN);
$this->assertEqual($response->headers->get('Content-type'), 'text/html');
$this->assertEqual($response->headers->get('Content-type'), 'text/html; charset=UTF-8');
}
/**
* Tests the exception handling for HTML and 403 status code.
* Tests the exception handling for HTML and 404 status code.
*/
public function testHtml404() {
$request = Request::create('/not-found');
......@@ -93,27 +93,10 @@ public function testHtml404() {
/** @var \Symfony\Component\HttpKernel\HttpKernelInterface $kernel */
$kernel = \Drupal::getContainer()->get('http_kernel');
$response = $kernel->handle($request);
$response = $kernel->handle($request)->prepare($request);
$this->assertEqual($response->getStatusCode(), Response::HTTP_NOT_FOUND);
$this->assertEqual($response->headers->get('Content-type'), 'text/html');
}
/**
* Tests the exception handling for HTML and 405 status code.
*/
public function testHtml405() {
$request = Request::create('/admin', 'NOT_EXISTING');
$request->headers->set('Accept', 'text/html');
$request->setFormat('html', ['text/html']);
/** @var \Symfony\Component\HttpKernel\HttpKernelInterface $kernel */
$kernel = \Drupal::getContainer()->get('http_kernel');
$response = $kernel->handle($request);
$this->assertEqual($response->getStatusCode(), Response::HTTP_METHOD_NOT_ALLOWED);
$this->assertEqual($response->headers->get('Content-type'), 'text/html');
$this->assertEqual($response->getContent(), 'Method Not Allowed');
$this->assertEqual($response->headers->get('Content-type'), 'text/html; charset=UTF-8');
}
}
......
......@@ -7,6 +7,22 @@ system.ajax:
requirements:
_access: 'TRUE'
system.403:
path: '/system/403'
defaults:
_content: '\Drupal\system\Controller\Http4xxController:on403'
_title: 'Access denied'
requirements:
_access: 'TRUE'
system.404:
path: '/system/404'
defaults:
_content: '\Drupal\system\Controller\Http4xxController:on404'
_title: 'Page not found'
requirements:
_access: 'TRUE'
system.admin:
path: '/admin'
defaults:
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment