From 5cd5a9d0aeb53e6e6ab27ba21d343588b29d9e79 Mon Sep 17 00:00:00 2001 From: catch Date: Thu, 5 Aug 2021 21:27:51 +0100 Subject: [PATCH] Issue #3002352 by longwave, andypost, gabesullice, googletorp, daffie, alexpott, borisson_: CacheableHttpException must pass a $headers argument to HttpException --- .../Http/Exception/CacheableHttpException.php | 10 +-- .../ResourceVersionRouteEnhancer.php | 4 +- .../Core/Http/CacheableExceptionTest.php | 80 +++++++++++++++++++ 3 files changed, 84 insertions(+), 10 deletions(-) create mode 100644 core/tests/Drupal/Tests/Core/Http/CacheableExceptionTest.php diff --git a/core/lib/Drupal/Core/Http/Exception/CacheableHttpException.php b/core/lib/Drupal/Core/Http/Exception/CacheableHttpException.php index 5d8ec67dde4..a1b9edfdf99 100644 --- a/core/lib/Drupal/Core/Http/Exception/CacheableHttpException.php +++ b/core/lib/Drupal/Core/Http/Exception/CacheableHttpException.php @@ -16,15 +16,9 @@ class CacheableHttpException extends HttpException implements CacheableDependenc /** * {@inheritdoc} */ - public function __construct(CacheableDependencyInterface $cacheability, $statusCode = 0, $message = NULL, \Exception $previous = NULL, $code = 0) { + public function __construct(CacheableDependencyInterface $cacheability, $statusCode = 0, $message = NULL, \Exception $previous = NULL, array $headers = [], $code = 0) { $this->setCacheability($cacheability); - // @todo Remove condition in https://www.drupal.org/node/3002352 - if (is_array($code)) { - parent::__construct($statusCode, $message, $previous, $code); - } - else { - parent::__construct($statusCode, $message, $previous, [], $code); - } + parent::__construct($statusCode, $message, $previous, $headers, $code); } } diff --git a/core/modules/jsonapi/src/Revisions/ResourceVersionRouteEnhancer.php b/core/modules/jsonapi/src/Revisions/ResourceVersionRouteEnhancer.php index ad2186bef7c..96be23ad2a4 100644 --- a/core/modules/jsonapi/src/Revisions/ResourceVersionRouteEnhancer.php +++ b/core/modules/jsonapi/src/Revisions/ResourceVersionRouteEnhancer.php @@ -105,7 +105,7 @@ final class ResourceVersionRouteEnhancer implements EnhancerInterface { $message = 'JSON:API does not yet support resource versioning for this resource type.'; $message .= ' For context, see https://www.drupal.org/project/drupal/issues/2992833#comment-12818258.'; $message .= ' To contribute, see https://www.drupal.org/project/drupal/issues/2350939 and https://www.drupal.org/project/drupal/issues/2809177.'; - throw new CacheableHttpException($cacheability, 501, $message, NULL, 0); + throw new CacheableHttpException($cacheability, 501, $message); } return $defaults; } @@ -151,7 +151,7 @@ final class ResourceVersionRouteEnhancer implements EnhancerInterface { ]; $cacheability = (new CacheableMetadata())->addCacheContexts($cache_contexts); $message = 'JSON:API does not support filtering on revisions other than the latest version because a secure Drupal core API does not yet exist to do so.'; - throw new CacheableHttpException($cacheability, 501, $message, NULL, 0); + throw new CacheableHttpException($cacheability, 501, $message); } // 'latest-version' and 'working-copy' are the only acceptable version // identifiers for a collection resource. diff --git a/core/tests/Drupal/Tests/Core/Http/CacheableExceptionTest.php b/core/tests/Drupal/Tests/Core/Http/CacheableExceptionTest.php new file mode 100644 index 00000000000..59290c3149d --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Http/CacheableExceptionTest.php @@ -0,0 +1,80 @@ +setCacheContexts(['route']), 500, 'test message', NULL, ['X-Drupal-Exception' => 'Test'], 123); + $this->assertSame(['route'], $exception->getCacheContexts()); + $this->assertSame(500, $exception->getStatusCode()); + $this->assertSame('test message', $exception->getMessage()); + $this->assertSame(['X-Drupal-Exception' => 'Test'], $exception->getHeaders()); + $this->assertSame(123, $exception->getCode()); + } + + /** + * @dataProvider providerTestExceptions + */ + public function testExceptions($status_code, $class, $argument = NULL, $expected_headers = []) { + $cacheable_metadata = (new CacheableMetadata())->setCacheContexts(['route']); + $message = "$class test message"; + if ($argument) { + $exception = new $class($cacheable_metadata, $argument, $message, NULL, 123); + } + else { + $exception = new $class($cacheable_metadata, $message, NULL, 123); + } + $this->assertSame(['route'], $exception->getCacheContexts()); + $this->assertSame($message, $exception->getMessage()); + $this->assertSame($status_code, $exception->getStatusCode()); + $this->assertSame($expected_headers, $exception->getHeaders()); + $this->assertSame(123, $exception->getCode()); + } + + public function providerTestExceptions() { + return [ + [400, CacheableBadRequestHttpException::class], + [401, CacheableUnauthorizedHttpException::class, 'test challenge', ['WWW-Authenticate' => 'test challenge']], + [403, CacheableAccessDeniedHttpException::class], + [404, CacheableNotFoundHttpException::class], + [405, CacheableMethodNotAllowedHttpException::Class, ['POST', 'PUT'], ['Allow' => 'POST, PUT']], + [406, CacheableNotAcceptableHttpException::class], + [409, CacheableConflictHttpException::class], + [410, CacheableGoneHttpException::class], + [411, CacheableLengthRequiredHttpException::class], + [412, CacheablePreconditionFailedHttpException::class], + [415, CacheableUnsupportedMediaTypeHttpException::class], + [422, CacheableUnprocessableEntityHttpException::class], + [428, CacheablePreconditionRequiredHttpException::class], + [429, CacheableTooManyRequestsHttpException::class, 60, ['Retry-After' => 60]], + [503, CacheableServiceUnavailableHttpException::class, 60, ['Retry-After' => 60]], + ]; + } + +}