diff --git a/core/core.services.yml b/core/core.services.yml index 31733463b430..7646382d48ca 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -1232,8 +1232,8 @@ services: tags: - { name: event_subscriber } arguments: ['@http_kernel', '@logger.channel.php', '@redirect.destination', '@router.no_access_checks'] - exception.default: - class: Drupal\Core\EventSubscriber\DefaultExceptionSubscriber + exception.final: + class: Drupal\Core\EventSubscriber\FinalExceptionSubscriber tags: - { name: event_subscriber } arguments: ['@config.factory'] diff --git a/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/FinalExceptionSubscriber.php similarity index 60% rename from core/lib/Drupal/Core/EventSubscriber/DefaultExceptionSubscriber.php rename to core/lib/Drupal/Core/EventSubscriber/FinalExceptionSubscriber.php index 7b5ecc1280af..4317ff08cb38 100644 --- a/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/FinalExceptionSubscriber.php @@ -7,19 +7,31 @@ use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\StringTranslation\StringTranslationTrait; use Drupal\Core\Utility\Error; use Symfony\Component\EventDispatcher\EventSubscriberInterface; -use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface; use Symfony\Component\HttpKernel\KernelEvents; /** - * Last-chance handler for exceptions. + * Last-chance handler for exceptions: the final exception subscriber. * * This handler will catch any exceptions not caught elsewhere and report * them as an error page. + * + * Each format has its own way of handling exceptions: + * - html: exception.default_html, exception.custom_page_html and + * exception.fast_404_html + * - json: exception.default_json + * + * And when the serialization module is installed, all serialization formats are + * handled by a single exception subscriber:: serialization.exception.default. + * + * This exception subscriber runs after all the above (it has a lower priority), + * which makes it the last-chance exception handler. It always sends a plain + * text response. If it's a displayable error and the error level is configured + * to be verbose, then a helpful backtrace is also printed. */ -class DefaultExceptionSubscriber implements EventSubscriberInterface { +class FinalExceptionSubscriber implements EventSubscriberInterface { use StringTranslationTrait; /** @@ -37,7 +49,7 @@ class DefaultExceptionSubscriber implements EventSubscriberInterface { protected $configFactory; /** - * Constructs a new DefaultExceptionSubscriber. + * Constructs a new FinalExceptionSubscriber. * * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory * The configuration factory. @@ -59,19 +71,19 @@ class DefaultExceptionSubscriber implements EventSubscriberInterface { } /** - * Handles any exception as a generic error page for HTML. + * Handles exceptions for this subscriber. * * @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event * The event to process. */ - protected function onHtml(GetResponseForExceptionEvent $event) { + public function onException(GetResponseForExceptionEvent $event) { $exception = $event->getException(); $error = Error::decodeException($exception); // Display the message if the current error reporting level allows this type // of message to be displayed, and unconditionally in update.php. $message = ''; - if (error_displayable($error)) { + if ($this->isErrorDisplayable($error)) { // If error type is 'User notice' then treat it as debug information // instead of an error message. // @see debug() @@ -79,16 +91,11 @@ class DefaultExceptionSubscriber implements EventSubscriberInterface { $error['%type'] = 'Debug'; } - // Attempt to reduce verbosity by removing DRUPAL_ROOT from the file path - // in the message. This does not happen for (false) security. - $root_length = strlen(DRUPAL_ROOT); - if (substr($error['%file'], 0, $root_length) == DRUPAL_ROOT) { - $error['%file'] = substr($error['%file'], $root_length + 1); - } + $error = $this->simplifyFileInError($error); unset($error['backtrace']); - if ($this->getErrorLevel() != ERROR_REPORTING_DISPLAY_VERBOSE) { + if (!$this->isErrorLevelVerbose()) { // Without verbose logging, use a simple message. // We call SafeMarkup::format directly here, rather than use t() since @@ -132,75 +139,57 @@ class DefaultExceptionSubscriber implements EventSubscriberInterface { } /** - * Handles an HttpExceptionInterface exception for unknown formats. - * - * @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event - * The event to process. - */ - protected function onFormatUnknown(GetResponseForExceptionEvent $event) { - /** @var \Symfony\Component\HttpKernel\Exception\HttpExceptionInterface|\Exception $exception */ - $exception = $event->getException(); - - $response = new Response($exception->getMessage(), $exception->getStatusCode(), $exception->getHeaders()); - $event->setResponse($response); - } - - /** - * Handles errors for this subscriber. - * - * @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event - * The event to process. - */ - public function onException(GetResponseForExceptionEvent $event) { - $format = $this->getFormat($event->getRequest()); - $exception = $event->getException(); - - $method = 'on' . $format; - if (!method_exists($this, $method)) { - if ($exception instanceof HttpExceptionInterface) { - $this->onFormatUnknown($event); - $response = $event->getResponse(); - $response->headers->set('Content-Type', 'text/plain'); - } - else { - $this->onHtml($event); - } - } - else { - $this->$method($event); - } - } - - /** - * Gets the error-relevant format from the request. - * - * @param \Symfony\Component\HttpFoundation\Request $request - * The request object. - * - * @return string - * The format as which to treat the exception. - */ - protected function getFormat(Request $request) { - $format = $request->query->get(MainContentViewSubscriber::WRAPPER_FORMAT, $request->getRequestFormat()); - - // These are all JSON errors for our purposes. Any special handling for - // them can/should happen in earlier listeners if desired. - if (in_array($format, ['drupal_modal', 'drupal_dialog', 'drupal_ajax'])) { - $format = 'json'; - } - - return $format; - } - - /** - * Registers the methods in this class that should be listeners. - * - * @return array - * An array of event listener definitions. + * {@inheritdoc} */ public static function getSubscribedEvents() { + // Run as the final (very late) KernelEvents::EXCEPTION subscriber. $events[KernelEvents::EXCEPTION][] = ['onException', -256]; return $events; } + /** + * Checks whether the error level is verbose or not. + * + * @return bool + */ + protected function isErrorLevelVerbose() { + return $this->getErrorLevel() === ERROR_REPORTING_DISPLAY_VERBOSE; + } + + /** + * Wrapper for error_displayable(). + * + * @param $error + * Optional error to examine for ERROR_REPORTING_DISPLAY_SOME. + * + * @return bool + * + * @see \error_displayable + */ + protected function isErrorDisplayable($error) { + return error_displayable($error); + } + + /** + * Attempts to reduce error verbosity in the error message's file path. + * + * Attempts to reduce verbosity by removing DRUPAL_ROOT from the file path in + * the message. This does not happen for (false) security. + * + * @param $error + * Optional error to examine for ERROR_REPORTING_DISPLAY_SOME. + * + * @return + * The updated $error. + */ + protected function simplifyFileInError($error) { + // Attempt to reduce verbosity by removing DRUPAL_ROOT from the file path + // in the message. This does not happen for (false) security. + $root_length = strlen(DRUPAL_ROOT); + if (substr($error['%file'], 0, $root_length) == DRUPAL_ROOT) { + $error['%file'] = substr($error['%file'], $root_length + 1); + } + return $error; + } + } diff --git a/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php index b46eec9eac4e..a7317279c76c 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php @@ -281,7 +281,7 @@ abstract class CommentResourceTestBase extends EntityResourceTestBase { // @todo Uncomment, remove next 3 lines in https://www.drupal.org/node/2820364. $this->assertSame(500, $response->getStatusCode()); $this->assertSame(['text/plain; charset=UTF-8'], $response->getHeader('Content-Type')); - $this->assertSame('Internal Server Error', (string) $response->getBody()); + $this->assertStringStartsWith('The website encountered an unexpected error. Please try again later.

Symfony\Component\HttpKernel\Exception\HttpException: Internal Server Error in Drupal\rest\Plugin\rest\resource\EntityResource->post()', (string) $response->getBody()); //$this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\nentity_type: This value should not be null.\n", $response); // DX: 422 when missing 'entity_id' field. diff --git a/core/tests/Drupal/KernelTests/Core/Routing/ExceptionHandlingTest.php b/core/tests/Drupal/KernelTests/Core/Routing/ExceptionHandlingTest.php index f7e60746eab0..a791454f7247 100644 --- a/core/tests/Drupal/KernelTests/Core/Routing/ExceptionHandlingTest.php +++ b/core/tests/Drupal/KernelTests/Core/Routing/ExceptionHandlingTest.php @@ -192,10 +192,11 @@ class ExceptionHandlingTest extends KernelTestBase { $kernel = \Drupal::getContainer()->get('http_kernel'); $response = $kernel->handle($request)->prepare($request); // As the Content-type is text/plain the fact that the raw string is - // contained in the output does not matter. + // contained in the output would not matter, but because it is output by the + // final exception subscriber, it is printed as partial HTML, and hence + // escaped. $this->assertEqual($response->headers->get('Content-type'), 'text/plain; charset=UTF-8'); - $this->setRawContent($response->getContent()); - $this->assertRaw($string); + $this->assertStringStartsWith('The website encountered an unexpected error. Please try again later.

Symfony\Component\HttpKernel\Exception\NotAcceptableHttpException: Not acceptable format: json<script>alert(123);</script> in ', $response->getContent()); } } diff --git a/core/tests/Drupal/Tests/Core/EventSubscriber/DefaultExceptionSubscriberTest.php b/core/tests/Drupal/Tests/Core/EventSubscriber/FinalExceptionSubscriberTest.php similarity index 58% rename from core/tests/Drupal/Tests/Core/EventSubscriber/DefaultExceptionSubscriberTest.php rename to core/tests/Drupal/Tests/Core/EventSubscriber/FinalExceptionSubscriberTest.php index 6582500ee649..09b7dd3e2946 100644 --- a/core/tests/Drupal/Tests/Core/EventSubscriber/DefaultExceptionSubscriberTest.php +++ b/core/tests/Drupal/Tests/Core/EventSubscriber/FinalExceptionSubscriberTest.php @@ -2,7 +2,7 @@ namespace Drupal\Tests\Core\EventSubscriber; -use Drupal\Core\EventSubscriber\DefaultExceptionSubscriber; +use Drupal\Core\EventSubscriber\FinalExceptionSubscriber; use Drupal\Tests\UnitTestCase; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -11,14 +11,13 @@ use Symfony\Component\HttpKernel\Exception\MethodNotAllowedHttpException; use Symfony\Component\HttpKernel\HttpKernelInterface; /** - * @coversDefaultClass \Drupal\Core\EventSubscriber\DefaultExceptionSubscriber + * @coversDefaultClass \Drupal\Core\EventSubscriber\FinalExceptionSubscriber * @group EventSubscriber */ -class DefaultExceptionSubscriberTest extends UnitTestCase { +class FinalExceptionSubscriberTest extends UnitTestCase { /** * @covers ::onException - * @covers ::onFormatUnknown */ public function testOnExceptionWithUnknownFormat() { $config_factory = $this->getConfigFactoryStub(); @@ -27,12 +26,13 @@ class DefaultExceptionSubscriberTest extends UnitTestCase { $request = Request::create('/test?_format=bananas'); $e = new MethodNotAllowedHttpException(['POST', 'PUT'], 'test message'); $event = new GetResponseForExceptionEvent($kernel->reveal(), $request, 'GET', $e); - $subscriber = new DefaultExceptionSubscriber($config_factory); + $subscriber = new TestDefaultExceptionSubscriber($config_factory); + $subscriber->setStringTranslation($this->getStringTranslationStub()); $subscriber->onException($event); $response = $event->getResponse(); $this->assertInstanceOf(Response::class, $response); - $this->assertEquals('test message', $response->getContent()); + $this->stringStartsWith('The website encountered an unexpected error. Please try again later.

Symfony\Component\HttpKernel\Exception\MethodNotAllowedHttpException: test message in ', $response->getContent()); $this->assertEquals(405, $response->getStatusCode()); $this->assertEquals('POST, PUT', $response->headers->get('Allow')); // Also check that that text/plain content type was added. @@ -40,3 +40,19 @@ class DefaultExceptionSubscriberTest extends UnitTestCase { } } + +class TestDefaultExceptionSubscriber extends FinalExceptionSubscriber { + + protected function isErrorDisplayable($error) { + return TRUE; + } + + protected function simplifyFileInError($error) { + return $error; + } + + protected function isErrorLevelVerbose() { + return TRUE; + } + +}