Verified Commit 8d6cde79 authored by Alex Pott's avatar Alex Pott
Browse files

Issue #3454346 by kristiaanvandeneynde, bbrala, mxr576:...

Issue #3454346 by kristiaanvandeneynde, bbrala, mxr576: JsonApiRequestValidator does not set cacheable metadata when the filter allows the request

(cherry picked from commit a3f99099)
parent 07c0011d
Loading
Loading
Loading
Loading
Loading
+34 −6
Original line number Diff line number Diff line
@@ -3,15 +3,17 @@
namespace Drupal\jsonapi\EventSubscriber;

use Drupal\Core\Cache\CacheableMetadata;
use Drupal\Core\Cache\CacheableResponseInterface;
use Drupal\jsonapi\JsonApiSpec;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Event\RequestEvent;
use Drupal\Core\Http\Exception\CacheableBadRequestHttpException;
use Symfony\Component\HttpKernel\Event\ResponseEvent;
use Symfony\Component\HttpKernel\KernelEvents;

/**
 * Request subscriber that validates a JSON:API request.
 * Subscriber that validates the query parameter names on a JSON:API request.
 *
 * @internal JSON:API maintains no PHP API. The API is the HTTP API. This class
 *   may change at any time and could break any dependencies on it.
@@ -36,6 +38,28 @@ public function onRequest(RequestEvent $event) {
    $this->validateQueryParams($request);
  }

  /**
   * Validates JSON:API requests.
   *
   * @param \Symfony\Component\HttpKernel\Event\ResponseEvent $event
   *   The event to process.
   */
  public function onResponse(ResponseEvent $event) {
    $request = $event->getRequest();
    if ($request->getRequestFormat() !== 'api_json') {
      return;
    }

    // At this point, we've already run validation on the request by checking
    // the query arguments. This means that if the query arguments change, we
    // may need to run this validation again. Therefore, all responses need to
    // vary by url.query_args.
    $response = $event->getResponse();
    if ($response instanceof CacheableResponseInterface) {
      $response->addCacheableDependency((new CacheableMetadata())->addCacheContexts(['url.query_args']));
    }
  }

  /**
   * Validates custom (implementation-specific) query parameter names.
   *
@@ -60,20 +84,20 @@ protected function validateQueryParams(Request $request) {
      }
    }

    if (empty($invalid_query_params)) {
      return NULL;
    }

    // Drupal uses the `_format` query parameter for Content-Type negotiation.
    // Using it violates the JSON:API spec. Nudge people nicely in the correct
    // direction. (This is special cased because using it is pretty common.)
    if (in_array('_format', $invalid_query_params, TRUE)) {
      $uri_without_query_string = $request->getSchemeAndHttpHost() . $request->getBaseUrl() . $request->getPathInfo();
      $exception = new CacheableBadRequestHttpException((new CacheableMetadata())->addCacheContexts(['url.query_args:_format']), 'JSON:API does not need that ugly \'_format\' query string! 🤘 Use the URL provided in \'links\' 🙏');
      $exception = new CacheableBadRequestHttpException((new CacheableMetadata())->addCacheContexts(['url.query_args']), 'JSON:API does not need that ugly \'_format\' query string! 🤘 Use the URL provided in \'links\' 🙏');
      $exception->setHeaders(['Link' => $uri_without_query_string]);
      throw $exception;
    }

    if (empty($invalid_query_params)) {
      return NULL;
    }

    $message = sprintf('The following query parameters violate the JSON:API spec: \'%s\'.', implode("', '", $invalid_query_params));
    $exception = new CacheableBadRequestHttpException((new CacheableMetadata())->addCacheContexts(['url.query_args']), $message);
    $exception->setHeaders(['Link' => 'http://jsonapi.org/format/#query-parameters']);
@@ -85,6 +109,10 @@ protected function validateQueryParams(Request $request) {
   */
  public static function getSubscribedEvents(): array {
    $events[KernelEvents::REQUEST][] = ['onRequest'];

    // Run before the resource response subscriber (priority 128), so that said
    // subscriber gets the cacheable metadata from this one.
    $events[KernelEvents::RESPONSE][] = ['onResponse', 129];
    return $events;
  }

+1 −1
Original line number Diff line number Diff line
@@ -184,7 +184,7 @@ protected function getExpectedUnauthorizedAccessCacheability() {
        'http_response',
        'user:2',
      ])
      ->setCacheContexts(['url.site', 'user.roles']);
      ->setCacheContexts(['url.query_args', 'url.site', 'user.roles']);
  }

  /**
+1 −1
Original line number Diff line number Diff line
@@ -173,7 +173,7 @@ protected function getSparseFieldSets() {
  protected function getExpectedCacheContexts(?array $sparse_fieldset = NULL) {
    $cache_contexts = parent::getExpectedCacheContexts($sparse_fieldset);
    if ($sparse_fieldset === NULL || in_array('computed_test_cacheable_string_field', $sparse_fieldset)) {
      $cache_contexts = Cache::mergeContexts($cache_contexts, ['url.query_args:computed_test_cacheable_string_field']);
      $cache_contexts = Cache::mergeContexts($cache_contexts, ['url.query_args']);
    }

    return $cache_contexts;
+1 −0
Original line number Diff line number Diff line
@@ -46,6 +46,7 @@ public function testEntryPoint(): void {
    $response = $this->request('GET', Url::fromUri('base://jsonapi'), $request_options);
    $document = $this->getDocumentFromResponse($response);
    $expected_cache_contexts = [
      'url.query_args',
      'url.site',
      'user.roles:authenticated',
    ];
+1 −1
Original line number Diff line number Diff line
@@ -319,7 +319,7 @@ public function testPostFileUploadAndUseInSingleRequest(): void {
    // This request fails despite the upload succeeding, because we're not
    // allowed to view the entity we're uploading to.
    $response = $this->fileRequest($uri, $this->testFileData);
    $this->assertResourceErrorResponse(403, $this->getExpectedUnauthorizedAccessMessage('GET'), $uri, $response, FALSE, ['4xx-response', 'http_response'], ['url.site', 'user.permissions']);
    $this->assertResourceErrorResponse(403, $this->getExpectedUnauthorizedAccessMessage('GET'), $uri, $response, FALSE, ['4xx-response', 'http_response'], ['url.query_args', 'url.site', 'user.permissions']);

    $this->setUpAuthorization('GET');

Loading