Commit 9bd08a00 authored by catch's avatar catch

Issue #2825347 by Wim Leers: 'Accept' header still is used for determining...

Issue #2825347 by Wim Leers: 'Accept' header still is used for determining response format of error responses, ?_format= has no effect AND serialization module's exception subscriber does not support status code 500 error responses
parent 55a4f988
......@@ -7,7 +7,6 @@
use Drupal\Core\StringTranslation\StringTranslationTrait;
use Drupal\Core\Utility\Error;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
......@@ -119,7 +118,7 @@ protected function onHtml(GetResponseForExceptionEvent $event) {
$content = $this->t('The website encountered an unexpected error. Please try again later.');
$content .= $message ? '</br></br>' . $message : '';
$response = new Response($content, 500);
$response = new Response($content, 500, ['Content-Type' => 'text/plain']);
if ($exception instanceof HttpExceptionInterface) {
$response->setStatusCode($exception->getStatusCode());
......@@ -132,34 +131,6 @@ protected function onHtml(GetResponseForExceptionEvent $event) {
$event->setResponse($response);
}
/**
* Handles any exception as a generic error page for JSON.
*
* @todo This should probably check the error reporting level.
*
* @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event
* The event to process.
*/
protected function onJson(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,
$data = NULL;
if (error_displayable($error) && $message = $exception->getMessage()) {
$data = ['message' => sprintf('A fatal error occurred: %s', $message)];
}
$response = new JsonResponse($data, Response::HTTP_INTERNAL_SERVER_ERROR);
if ($exception instanceof HttpExceptionInterface) {
$response->setStatusCode($exception->getStatusCode());
$response->headers->add($exception->getHeaders());
}
$event->setResponse($response);
}
/**
* Handles an HttpExceptionInterface exception for unknown formats.
*
......@@ -218,16 +189,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;
}
......
......@@ -14,7 +14,7 @@ class ExceptionJsonSubscriber extends HttpExceptionSubscriberBase {
* {@inheritdoc}
*/
protected function getHandledFormats() {
return ['json'];
return ['json', 'drupal_modal', 'drupal_dialog', 'drupal_ajax'];
}
/**
......
......@@ -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);
}
}
......
......@@ -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();
......
......@@ -280,8 +280,8 @@ public function testPostDxWithoutCriticalBaseFields() {
$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());
$this->assertSame(['text/plain; charset=UTF-8'], $response->getHeader('Content-Type'));
$this->assertSame('Internal Server Error', (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.
......@@ -303,10 +303,9 @@ 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.
// @todo Uncomment, remove next 2 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());
$this->assertSame(['text/plain; charset=UTF-8'], $response->getHeader('Content-Type'));
//$this->assertResourceErrorResponse(422, "Unprocessable Entity: validation failed.\nfield_name: This value should not be null.\n", $response);
}
......
......@@ -5,7 +5,6 @@
use Drupal\Component\Utility\NestedArray;
use Drupal\Core\Cache\Cache;
use Drupal\Core\Config\Entity\ConfigEntityInterface;
use Drupal\Core\Entity\EntityChangedInterface;
use Drupal\Core\Entity\FieldableEntityInterface;
use Drupal\Core\Url;
use Drupal\field\Entity\FieldConfig;
......@@ -487,17 +486,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);
......@@ -566,11 +565,11 @@ public function testPost() {
$url->setOption('query', []);
// DX: 415 when no Content-Type request header, but HTML if canonical route.
// DX: 415 when no Content-Type request header, plain text if canonical URL.
$response = $this->request('POST', $url, $request_options);
if ($has_canonical_url) {
$this->assertSame(415, $response->getStatusCode());
$this->assertSame(['text/html; charset=UTF-8'], $response->getHeader('Content-Type'));
$this->assertSame(['text/plain; charset=UTF-8'], $response->getHeader('Content-Type'));
$this->assertContains(htmlspecialchars('No "Content-Type" request header specified'), (string) $response->getBody());
}
else {
......@@ -728,12 +727,12 @@ public function testPatch() {
$request_options = [];
// DX: 405 when resource not provisioned, but HTML if canonical route.
// DX: 404 when resource not provisioned, but 405 if canonical route.
$response = $this->request('PATCH', $url, $request_options);
if ($has_canonical_url) {
$this->assertSame(405, $response->getStatusCode());
$this->assertSame(['GET, POST, HEAD'], $response->getHeader('Allow'));
$this->assertSame(['text/html; charset=UTF-8'], $response->getHeader('Content-Type'));
$this->assertSame(['text/plain; charset=UTF-8'], $response->getHeader('Content-Type'));
}
else {
$this->assertResourceErrorResponse(404, 'No route found for "PATCH ' . str_replace($this->baseUrl, '', $this->getUrl()->setAbsolute()->toString()) . '"', $response);
......@@ -758,7 +757,7 @@ public function testPatch() {
$response = $this->request('PATCH', $url, $request_options);
if ($has_canonical_url) {
$this->assertSame(415, $response->getStatusCode());
$this->assertSame(['text/html; charset=UTF-8'], $response->getHeader('Content-Type'));
$this->assertSame(['text/plain; charset=UTF-8'], $response->getHeader('Content-Type'));
$this->assertTrue(FALSE !== strpos((string) $response->getBody(), htmlspecialchars('No "Content-Type" request header specified')));
}
else {
......@@ -922,12 +921,12 @@ public function testDelete() {
$request_options = [];
// DX: 405 when resource not provisioned, but HTML if canonical route.
// DX: 404 when resource not provisioned, but 405 if canonical route.
$response = $this->request('DELETE', $url, $request_options);
if ($has_canonical_url) {
$this->assertSame(405, $response->getStatusCode());
$this->assertSame(['GET, POST, HEAD'], $response->getHeader('Allow'));
$this->assertSame(['text/html; charset=UTF-8'], $response->getHeader('Content-Type'));
$this->assertSame(['text/plain; charset=UTF-8'], $response->getHeader('Content-Type'));
}
else {
$this->assertResourceErrorResponse(404, 'No route found for "DELETE ' . str_replace($this->baseUrl, '', $this->getUrl()->setAbsolute()->toString()) . '"', $response);
......
......@@ -155,7 +155,7 @@ public function testBacktraceEscaping() {
$kernel = \Drupal::getContainer()->get('http_kernel');
$response = $kernel->handle($request)->prepare($request);
$this->assertEqual($response->getStatusCode(), Response::HTTP_INTERNAL_SERVER_ERROR);
$this->assertEqual($response->headers->get('Content-type'), 'text/html; charset=UTF-8');
$this->assertEqual($response->headers->get('Content-type'), 'text/plain; charset=UTF-8');
// Test both that the backtrace is properly escaped, and that the unescaped
// string is not output at all.
......@@ -178,7 +178,7 @@ public function testExceptionEscaping() {
$kernel = \Drupal::getContainer()->get('http_kernel');
$response = $kernel->handle($request)->prepare($request);
$this->assertEqual($response->getStatusCode(), Response::HTTP_INTERNAL_SERVER_ERROR);
$this->assertEqual($response->headers->get('Content-type'), 'text/html; charset=UTF-8');
$this->assertEqual($response->headers->get('Content-type'), 'text/plain; charset=UTF-8');
// Test message is properly escaped, and that the unescaped string is not
// output at all.
......
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