.../EventSubscriber/DefaultExceptionSubscriber.php | 10 --- .../EventSubscriber/ExceptionJsonSubscriber.php | 2 +- .../src/Tests/QuickEditIntegrationLoadingTest.php | 6 +- .../quickedit/src/Tests/QuickEditLoadingTest.php | 4 +- .../Comment/CommentResourceTestBase.php | 12 ++-- .../EntityResource/EntityResourceTestBase.php | 8 +-- .../EventSubscriber/DefaultExceptionSubscriber.php | 39 ++++++++++- .../DefaultExceptionSubscriberTest.php | 80 +++++++++++++++++++++- 8 files changed, 130 insertions(+), 31 deletions(-) diff --git a/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionSubscriber.php index 4737e80..836b667 100644 --- a/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionSubscriber.php @@ -218,16 +218,6 @@ protected function getFormat(Request $request) { $format = 'json'; } - // Make an educated guess that any Accept header type that includes "json" - // can probably handle a generic JSON response for errors. As above, for - // any format this doesn't catch or that wants custom handling should - // register its own exception listener. - foreach ($request->getAcceptableContentTypes() as $mime) { - if (strpos($mime, 'html') === FALSE && strpos($mime, 'json') !== FALSE) { - $format = 'json'; - } - } - return $format; } diff --git a/core/lib/Drupal/Core/EventSubscriber/ExceptionJsonSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/ExceptionJsonSubscriber.php index 76cc55e..6ccb743 100644 --- a/core/lib/Drupal/Core/EventSubscriber/ExceptionJsonSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/ExceptionJsonSubscriber.php @@ -14,7 +14,7 @@ class ExceptionJsonSubscriber extends HttpExceptionSubscriberBase { * {@inheritdoc} */ protected function getHandledFormats() { - return ['json']; + return ['json', 'drupal_modal', 'drupal_dialog', 'drupal_ajax']; } /** diff --git a/core/modules/editor/src/Tests/QuickEditIntegrationLoadingTest.php b/core/modules/editor/src/Tests/QuickEditIntegrationLoadingTest.php index 9b9d13c..c827450 100644 --- a/core/modules/editor/src/Tests/QuickEditIntegrationLoadingTest.php +++ b/core/modules/editor/src/Tests/QuickEditIntegrationLoadingTest.php @@ -89,12 +89,12 @@ public function testUsersWithoutPermission() { $response = $this->drupalPost('editor/' . 'node/1/body/en/full', '', array(), array('query' => array(MainContentViewSubscriber::WRAPPER_FORMAT => 'drupal_ajax'))); $this->assertResponse(403); if (!$user->hasPermission('access in-place editing')) { - $message = "A fatal error occurred: The 'access in-place editing' permission is required."; - $this->assertIdentical(Json::encode(['message' => $message]), $response); + $message = "The 'access in-place editing' permission is required."; } else { - $this->assertIdentical('{}', $response); + $message = ''; } + $this->assertIdentical(Json::encode(['message' => $message]), $response); } } diff --git a/core/modules/quickedit/src/Tests/QuickEditLoadingTest.php b/core/modules/quickedit/src/Tests/QuickEditLoadingTest.php index ab66227..1570a27 100644 --- a/core/modules/quickedit/src/Tests/QuickEditLoadingTest.php +++ b/core/modules/quickedit/src/Tests/QuickEditLoadingTest.php @@ -115,12 +115,12 @@ public function testUserWithoutPermission() { $this->assertIdentical(Json::encode(['message' => "The 'access in-place editing' permission is required."]), $response); $this->assertResponse(403); - // Quick Edit's JavaScript would SearchRankingTestnever hit these endpoints if the metadata + // Quick Edit's JavaScript would never hit these endpoints if the metadata // was empty as above, but we need to make sure that malicious users aren't // able to use any of the other endpoints either. $post = array('editors[0]' => 'form') + $this->getAjaxPageStatePostData(); $response = $this->drupalPost('quickedit/attachments', '', $post, ['query' => [MainContentViewSubscriber::WRAPPER_FORMAT => 'drupal_ajax']]); - $message = Json::encode(['message' => "A fatal error occurred: The 'access in-place editing' permission is required."]); + $message = Json::encode(['message' => "The 'access in-place editing' permission is required."]); $this->assertIdentical($message, $response); $this->assertResponse(403); $post = array('nocssjs' => 'true') + $this->getAjaxPageStatePostData(); 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 7e38b33..7eb62d0 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/Comment/CommentResourceTestBase.php @@ -278,10 +278,8 @@ public function testPostDxWithoutCriticalBaseFields() { // DX: 422 when missing 'entity_type' field. $request_options[RequestOptions::BODY] = $this->serializer->encode(array_diff_key($this->getNormalizedPostEntity(), ['entity_type' => TRUE]), static::$format); $response = $this->request('POST', $url, $request_options); - // @todo Uncomment, remove next 3 lines in https://www.drupal.org/node/2820364. - $this->assertSame(500, $response->getStatusCode()); - $this->assertSame(['application/json'], $response->getHeader('Content-Type')); - $this->assertSame('{"message":"A fatal error occurred: Internal Server Error"}', (string) $response->getBody()); + // @todo Uncomment, remove next line in https://www.drupal.org/node/2820364. + $this->assertResourceErrorResponse(500, 'Internal Server Error', $response); //$this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\nentity_type: This value should not be null.\n", $response); // DX: 422 when missing 'entity_id' field. @@ -303,10 +301,8 @@ public function testPostDxWithoutCriticalBaseFields() { // DX: 422 when missing 'entity_type' field. $request_options[RequestOptions::BODY] = $this->serializer->encode(array_diff_key($this->getNormalizedPostEntity(), ['field_name' => TRUE]), static::$format); $response = $this->request('POST', $url, $request_options); - // @todo Uncomment, remove next 3 lines in https://www.drupal.org/node/2820364. - $this->assertSame(500, $response->getStatusCode()); - $this->assertSame(['application/json'], $response->getHeader('Content-Type')); - $this->assertSame('{"message":"A fatal error occurred: Field is unknown."}', (string) $response->getBody()); + // @todo Uncomment, remove next line in https://www.drupal.org/node/2820364. + $this->assertResourceErrorResponse(500, 'A fatal error occurred: Field is unknown.', $response); //$this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\nfield_name: This value should not be null.\n", $response); } diff --git a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php index 73dfc62..9f326bc 100644 --- a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php @@ -501,17 +501,17 @@ public function testGet() { // DX: 406 when requesting unsupported format. $response = $this->request('GET', $url, $request_options); $this->assert406Response($response); - $this->assertNotSame([static::$mimeType], $response->getHeader('Content-Type')); + $this->assertSame(['text/plain; charset=UTF-8'], $response->getHeader('Content-Type')); $request_options[RequestOptions::HEADERS]['Accept'] = static::$mimeType; - // DX: 406 when requesting unsupported format but specifying Accept header. - // @todo Update in https://www.drupal.org/node/2825347. + // DX: 406 when requesting unsupported format but specifying Accept header: + // should result in a text/plain response. $response = $this->request('GET', $url, $request_options); $this->assert406Response($response); - $this->assertSame(['application/json'], $response->getHeader('Content-Type')); + $this->assertSame(['text/plain; charset=UTF-8'], $response->getHeader('Content-Type')); $url = Url::fromRoute('rest.entity.' . static::$entityTypeId . '.GET.' . static::$format); diff --git a/core/modules/serialization/src/EventSubscriber/DefaultExceptionSubscriber.php b/core/modules/serialization/src/EventSubscriber/DefaultExceptionSubscriber.php index a1e7bad..b157f62 100644 --- a/core/modules/serialization/src/EventSubscriber/DefaultExceptionSubscriber.php +++ b/core/modules/serialization/src/EventSubscriber/DefaultExceptionSubscriber.php @@ -3,12 +3,15 @@ namespace Drupal\serialization\EventSubscriber; use Drupal\Core\EventSubscriber\HttpExceptionSubscriberBase; +use Drupal\Core\Utility\Error; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; +use Symfony\Component\HttpKernel\Exception\HttpException; +use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface; use Symfony\Component\Serializer\SerializerInterface; /** - * Handles default error responses in serialization formats. + * Handles default and fatal error responses in serialization formats. */ class DefaultExceptionSubscriber extends HttpExceptionSubscriberBase { @@ -56,6 +59,30 @@ protected static function getPriority() { } /** + * {@inheritdoc} + */ + public function onException(GetResponseForExceptionEvent $event) { + $exception = $event->getException(); + + // Map non-HttpExceptionInterface exceptions for fatal errors onto the same + // HttpExceptionInterface object, so we can handle them consistently. + if (!$exception instanceof HttpExceptionInterface && in_array($event->getRequest()->getRequestFormat(), $this->getHandledFormats(), TRUE)) { + $error = Error::decodeException($exception); + $message = error_displayable($error) + ? sprintf('A fatal error occurred: %s', $exception->getMessage()) + : 'A fatal error occurred.'; + $event->setException(new HttpException(500, $message, $exception)); + } + + parent::onException($event); + + // If we didn't generate an error response, restore the original exception. + if (!$event->isPropagationStopped()) { + $event->setException($exception); + } + } + + /** * Handles all 4xx errors for all serialization failures. * * @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event @@ -78,4 +105,14 @@ public function on4xx(GetResponseForExceptionEvent $event) { $event->setResponse($response); } + /** + * Handles all 500 errors for all serialization failures. + * + * @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event + * The event to process. + */ + public function on500(GetResponseForExceptionEvent $event) { + return $this->on4xx($event); + } + } diff --git a/core/modules/serialization/tests/src/Unit/EventSubscriber/DefaultExceptionSubscriberTest.php b/core/modules/serialization/tests/src/Unit/EventSubscriber/DefaultExceptionSubscriberTest.php index e20f6f7..bead596 100644 --- a/core/modules/serialization/tests/src/Unit/EventSubscriber/DefaultExceptionSubscriberTest.php +++ b/core/modules/serialization/tests/src/Unit/EventSubscriber/DefaultExceptionSubscriberTest.php @@ -1,6 +1,6 @@ reveal(), $request, 'GET', $e); - $subscriber = new DefaultExceptionSubscriber(new Serializer([], [new JsonEncoder()]), []); + $subscriber = new DefaultExceptionSubscriber(new Serializer([], [new JsonEncoder()]), ['json']); $subscriber->on4xx($event); $response = $event->getResponse(); @@ -39,4 +40,79 @@ public function testOn4xx() { $this->assertEquals('application/json', $response->headers->get('Content-Type')); } + /** + * @covers ::on500 + */ + public function testOn500() { + $kernel = $this->prophesize(HttpKernelInterface::class); + $request = Request::create('/test'); + $request->setRequestFormat('json'); + + $e = new HttpException(500, 'Internal Server Error'); + $event = new GetResponseForExceptionEvent($kernel->reveal(), $request, 'GET', $e); + $subscriber = new DefaultExceptionSubscriber(new Serializer([], [new JsonEncoder()]), ['json']); + $subscriber->on500($event); + $response = $event->getResponse(); + + $this->assertInstanceOf(Response::class, $response); + $this->assertEquals('{"message":"Internal Server Error"}', $response->getContent()); + $this->assertEquals(500, $response->getStatusCode()); + $this->assertEquals('application/json', $response->headers->get('Content-Type')); + } + + /** + * @covers ::on500 + */ + public function testOnFatalErrorDefault() { + $kernel = $this->prophesize(HttpKernelInterface::class); + $request = Request::create('/test'); + $request->setRequestFormat('json'); + + $e = new \InvalidArgumentException('Some explanation here.'); + $event = new GetResponseForExceptionEvent($kernel->reveal(), $request, 'GET', $e); + $subscriber = new DefaultExceptionSubscriber(new Serializer([], [new JsonEncoder()]), ['json']); + $subscriber->onException($event); + $response = $event->getResponse(); + + $this->assertInstanceOf(Response::class, $response); + $this->assertEquals('{"message":"A fatal error occurred."}', $response->getContent()); + $this->assertEquals(500, $response->getStatusCode()); + $this->assertEquals('application/json', $response->headers->get('Content-Type')); + } + + /** + * @covers ::on500 + */ + public function testOnFatalErrorVerbose() { + $kernel = $this->prophesize(HttpKernelInterface::class); + $request = Request::create('/test'); + $request->setRequestFormat('json'); + + $e = new \LogicException('Some explanation here.'); + $event = new GetResponseForExceptionEvent($kernel->reveal(), $request, 'GET', $e); + $subscriber = new DefaultExceptionSubscriber(new Serializer([], [new JsonEncoder()]), ['json']); + $subscriber->onException($event); + $response = $event->getResponse(); + + $this->assertInstanceOf(Response::class, $response); + $this->assertEquals('{"message":"A fatal error occurred: Some explanation here."}', $response->getContent()); + $this->assertEquals(500, $response->getStatusCode()); + $this->assertEquals('application/json', $response->headers->get('Content-Type')); + } + +} + +} + +namespace { + if (!function_exists('error_displayable')) { + function error_displayable($error = NULL) { + if ($error['%type'] === 'LogicException') { + return TRUE; + } + else { + return FALSE; + } + } + } }