Commit fbe023ba authored by catch's avatar catch
Browse files

Issue #3085360 by bradjones1, josephdpurcell, Giuseppe87, ravi.shankar,...

Issue #3085360 by bradjones1, josephdpurcell, Giuseppe87, ravi.shankar, rajandro, ridhimaabrol24, bbrala, andregp, jhedstrom: RouteProvider::getRouteCollectionForRequest() can poison query string of next request

(cherry picked from commit 8b19e428)
parent 65625e63
Loading
Loading
Loading
Loading
Loading
+48 −1
Original line number Diff line number Diff line
@@ -453,6 +453,8 @@ protected function getRouteCollectionCacheId(Request $request) {
    // based on the domain.
    $this->addExtraCacheKeyPart('language', $this->getCurrentLanguageCacheIdPart());

    $this->addExtraCacheKeyPart('query_parameters', $this->getQueryParametersCacheIdPart($request));

    // Sort the cache key parts by their provider in order to have predictable
    // cache keys.
    ksort($this->extraCacheKeyParts);
@@ -461,7 +463,52 @@ protected function getRouteCollectionCacheId(Request $request) {
      $key_parts[] = '[' . $provider . ']=' . $key_part;
    }

    return 'route:' . implode(':', $key_parts) . ':' . $request->getPathInfo() . ':' . $request->getQueryString();
    return 'route:' . implode(':', $key_parts) . ':' . $request->getPathInfo();
  }

  /**
   * Returns the query parameters identifier for the route collection cache.
   *
   * The query parameters on the request may be altered programmatically, e.g.
   * while serving private files or in subrequests. As such, we must vary on
   * both the query string from the client and the parameter bag after incoming
   * route processors have modified the request object.
   *
   * @param \Symfony\Component\HttpFoundation\Request $request
   *   Request.
   *
   * @return string
   */
  protected function getQueryParametersCacheIdPart(Request $request) {
    // @todo Use \Symfony\Component\HttpFoundation\Request::normalizeQueryString
    //   for recursive key ordering if support is added in the future.
    $recursive_sort = function (&$array) use (&$recursive_sort) {
      foreach ($array as &$v) {
        if (is_array($v)) {
          $recursive_sort($v);
        }
      }
      ksort($array);
    };
    // Recursively normalize the query parameters to ensure maximal cache hits.
    // If we did not normalize the order, functionally identical query string
    // sets could be sent in differing order creating a potential DoS vector
    // and decreasing cache hit rates.
    $sorted_resolved_parameters = $request->query->all();
    $recursive_sort($sorted_resolved_parameters);
    $sorted_original_parameters = Request::create('/?' . $request->getQueryString())->query->all();
    $recursive_sort($sorted_original_parameters);
    // Hash this portion to help shorten the total key length.
    $resolved_hash = $sorted_resolved_parameters
      ? sha1(http_build_query($sorted_resolved_parameters))
      : NULL;
    return implode(
      ',',
      array_filter([
        http_build_query($sorted_original_parameters),
        $resolved_hash,
      ])
    );
  }

  /**
+25 −0
Original line number Diff line number Diff line
<?php

/**
 * @file
 * Test module.
 */

use Drupal\Core\Url;

/**
 * Implements hook_preprocess_HOOK().
 *
 * Performs an operation that calls the RouteProvider's collection method
 * during an exception page view. (which is rendered during a subrequest.)
 *
 * @see \Drupal\FunctionalTests\Routing\RouteCachingQueryAlteredTest
 */
function router_test_preprocess_page(&$variables) {
  $request = \Drupal::request();
  if ($request->getPathInfo() === '/router-test/rejects-query-strings') {
    // Create a URL from the request, e.g. for a breadcrumb or other contextual
    // information.
    Url::createFromRequest($request);
  }
}
+7 −0
Original line number Diff line number Diff line
@@ -247,3 +247,10 @@ router_test.case_sensitive_duplicate3:
    _controller: '\Drupal\router_test\TestControllers::testRouteName'
  requirements:
    _access: 'TRUE'

router_test.rejects_query_strings:
  path: '/router-test/rejects-query-strings'
  defaults:
    _controller: '\Drupal\router_test\TestControllers::rejectsQueryStrings'
  requirements:
    _access: 'TRUE'
+44 −0
Original line number Diff line number Diff line
<?php

declare(strict_types=1);

namespace Drupal\router_test;

use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Event\RequestEvent;
use Symfony\Component\HttpKernel\Exception\HttpException;
use Symfony\Component\HttpKernel\KernelEvents;

/**
 * Event subscribers for exceptions thrown in early kernel middleware.
 */
class RouterTestEarlyExceptionSubscriber implements EventSubscriberInterface {

  /**
   * Throw an exception, which will trigger exception-handling subscribers.
   *
   * See DefaultExceptionHtmlSubscriber.
   */
  public function onKernelRequest(RequestEvent $event): void {
    if ($event->isMainRequest() && $event->getRequest()->headers->get('Authorization') === 'Bearer invalid') {
      throw new HttpException(
        Response::HTTP_UNAUTHORIZED,
        'This is a common exception during authentication.'
      );
    }
  }

  /**
   * {@inheritdoc}
   */
  public static function getSubscribedEvents(): array {
    // This is the same priority as AuthenticationSubscriber, however
    // exceptions are not restricted to authentication; this is a common,
    // early point to emulate an exception, e.g. when an OAuth token is
    // rejected.
    $events[KernelEvents::REQUEST][] = ['onKernelRequest', 300];
    return $events;
  }

}
+4 −1
Original line number Diff line number Diff line
@@ -14,9 +14,12 @@ class RouterTestServiceProvider implements ServiceProviderInterface {
   * {@inheritdoc}
   */
  public function register(ContainerBuilder $container) {
    $container->register('router_test.subscriber', 'Drupal\router_test\RouteTestSubscriber')->addTag('event_subscriber');
    $container->register('router_test.subscriber', 'Drupal\router_test\RouteTestSubscriber')
      ->addTag('event_subscriber');
    $container->register('access_check.router_test', 'Drupal\router_test\Access\TestAccessCheck')
      ->addTag('access_check', ['applies_to' => '_access_router_test']);
    $container->register('router_test.early_exception.subscriber', 'Drupal\router_test\RouterTestEarlyExceptionSubscriber')
      ->addTag('event_subscriber');
  }

}
Loading