.../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 | 66 ++++++++++++++++++++-- 7 files changed, 74 insertions(+), 34 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..0c10678 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 still result in a response with the same Content-Type. $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..e58d980 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\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; +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,26 @@ protected static function getPriority() { } /** + * {@inheritdoc} + */ + public function onException(GetResponseForExceptionEvent $event) { + parent::onException($event); + + // If the exception was an HttpExceptionInterface exception, then + // propagation will already have stopped. If propagation has not yet stopped + // this must be a fatal error. + if (!$event->isPropagationStopped() && in_array($event->getRequest()->getRequestFormat(), $this->getHandledFormats(), TRUE)) { + $exception = $event->getException(); + $error = Error::decodeException($exception); + $message = error_displayable($error) + ? sprintf('A fatal error occurred: %s', $exception->getMessage()) + : 'A fatal error occurred.'; + + $event->setResponse($this->buildResponse($message, 500, [], $event->getRequest())); + } + } + + /** * Handles all 4xx errors for all serialization failures. * * @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event @@ -64,18 +87,49 @@ protected static function getPriority() { public function on4xx(GetResponseForExceptionEvent $event) { /** @var \Symfony\Component\HttpKernel\Exception\HttpExceptionInterface $exception */ $exception = $event->getException(); - $request = $event->getRequest(); + $event->setResponse($this->buildResponse( + $exception->getMessage(), + $exception->getStatusCode(), + $exception->getHeaders(), + $event->getRequest() + )); + } + + /** + * 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); + } + + /** + * Builds an error response. + * + * @param string $message + * The message to use in the error response. + * @param int $status_code + * The response status code. + * @param array $headers + * An array of response headers. + * @param \Symfony\Component\HttpFoundation\Request $request + * The current request. + * + * @return \Symfony\Component\HttpFoundation\Response + * The error response to send. + */ + protected function buildResponse($message, $status_code, $headers, Request $request) { $format = $request->getRequestFormat(); - $content = ['message' => $event->getException()->getMessage()]; + $content = ['message' => $message]; $encoded_content = $this->serializer->serialize($content, $format); - $headers = $exception->getHeaders(); // Add the MIME type from the request to send back in the header. $headers['Content-Type'] = $request->getMimeType($format); - $response = new Response($encoded_content, $exception->getStatusCode(), $headers); - $event->setResponse($response); + return new Response($encoded_content, $status_code, $headers); } }