core/core.services.yml | 5 ++ core/lib/Drupal/Core/Access/RouteProcessorCsrf.php | 40 +++++++++++-- .../Core/Cache/Context/SessionCacheContext.php | 29 ++++++++++ core/lib/Drupal/Core/Render/BubbleableMetadata.php | 21 +++++++ .../MenuLinkContentCacheabilityBubblingTest.php | 4 +- core/modules/system/src/Tests/Common/UrlTest.php | 16 +++--- .../Tests/Core/Access/RouteProcessorCsrfTest.php | 66 ++++++++++++++-------- 7 files changed, 143 insertions(+), 38 deletions(-) diff --git a/core/core.services.yml b/core/core.services.yml index 5fdfe30..736378f 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -24,6 +24,11 @@ services: arguments: ['@request_stack'] tags: - { name: cache.context } + cache_context.session: + class: Drupal\Core\Cache\Context\SessionCacheContext + arguments: ['@request_stack'] + tags: + - { name: cache.context} cache_context.request_format: class: Drupal\Core\Cache\Context\RequestFormatCacheContext arguments: ['@request_stack'] diff --git a/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php b/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php index b5ebc5a..5300cd7 100644 --- a/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php +++ b/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php @@ -45,13 +45,43 @@ public function processOutbound($route_name, Route $route, array &$parameters, B } // Adding this to the parameters means it will get merged into the query // string when the route is compiled. - $parameters['token'] = $this->csrfToken->get($path); - if ($bubbleable_metadata) { - // Tokens are per user and per session, so not cacheable. - // @todo Improve in https://www.drupal.org/node/2351015. - $bubbleable_metadata->setCacheMaxAge(0); + if (!$bubbleable_metadata) { + $parameters['token'] = $this->csrfToken->get($path); + } + else { + // Generate a placeholder and a render array to replace it. + $placeholder = hash('sha1', $path); + $placeholder_render_array = [ + '#lazy_builder' => ['route_processor_csrf:renderPlaceholderCsrfToken', [$path]], + // Tokens are per user and per session. + '#cache' => [ + 'contexts' => [ + 'user', + 'session', + ], + ], + ]; + + // Instead of setting an actual CSRF token as the query string, we set + // the placeholder, which will be replaced at the very last moment. This + // ensures links with CSRF tokens don't break cacheability. + $parameters['token'] = $placeholder; + $bubbleable_metadata->addAttachments(['placeholders' => [$placeholder => $placeholder_render_array]]); } } } + /** + * #lazy_builder callback; gets a CSRF token for the given path. + * + * @param string $path + * The path to get a CSRF token for. + * + * @return array + * A renderable array representing the CSRF token. + */ + public function renderPlaceholderCsrfToken($path) { + return ['#markup' => $this->csrfToken->get($path)]; + } + } diff --git a/core/lib/Drupal/Core/Cache/Context/SessionCacheContext.php b/core/lib/Drupal/Core/Cache/Context/SessionCacheContext.php new file mode 100644 index 0000000..7f3c3de --- /dev/null +++ b/core/lib/Drupal/Core/Cache/Context/SessionCacheContext.php @@ -0,0 +1,29 @@ +requestStack->getCurrentRequest()->getSession()->getId(); + } + +} diff --git a/core/lib/Drupal/Core/Render/BubbleableMetadata.php b/core/lib/Drupal/Core/Render/BubbleableMetadata.php index dd53b73..81e7237 100644 --- a/core/lib/Drupal/Core/Render/BubbleableMetadata.php +++ b/core/lib/Drupal/Core/Render/BubbleableMetadata.php @@ -74,6 +74,27 @@ public static function createFromRenderArray(array $build) { } /** + * Creates a bubbleable metadata object from a depended object. + * + * @param \Drupal\Core\Cache\CacheableDependencyInterface|mixed $object + * The object whose cacheability metadata to retrieve. If it implements + * CacheableDependencyInterface, its cacheability metadata will be used, + * otherwise, the passed in object must be assumed to be uncacheable, so + * max-age 0 is set. + * + * @return static + */ + public static function createFromObject($object) { + $meta = parent::createFromObject($object); + + if ($object instanceof AttachmentsInterface) { + $meta->attachments = $object->getAttachments(); + } + + return $meta; + } + + /** * Merges two attachments arrays (which live under the '#attached' key). * * The values under the 'drupalSettings' key are merged in a special way, to diff --git a/core/modules/menu_link_content/src/Tests/MenuLinkContentCacheabilityBubblingTest.php b/core/modules/menu_link_content/src/Tests/MenuLinkContentCacheabilityBubblingTest.php index aba6f5b..23e9d50 100644 --- a/core/modules/menu_link_content/src/Tests/MenuLinkContentCacheabilityBubblingTest.php +++ b/core/modules/menu_link_content/src/Tests/MenuLinkContentCacheabilityBubblingTest.php @@ -89,10 +89,10 @@ public function testOutboundPathAndRouteProcessing() { 'uri' => 'route:', 'cacheability' => (new BubbleableMetadata())->setCacheContexts(['route']), ], - // \Drupal\Core\Access\RouteProcessorCsrf: max-age = 0. + // \Drupal\Core\Access\RouteProcessorCsrf: placeholder. [ 'uri' => 'route:outbound_processing_test.route.csrf', - 'cacheability' => (new BubbleableMetadata())->setCacheMaxAge(0), + 'cacheability' => (new BubbleableMetadata())->setCacheContexts(['session', 'user'])->setAttachments(['placeholders' => []]), ], // \Drupal\Core\PathProcessor\PathProcessorFront: permanently cacheable. [ diff --git a/core/modules/system/src/Tests/Common/UrlTest.php b/core/modules/system/src/Tests/Common/UrlTest.php index 38f5769..5067efc 100644 --- a/core/modules/system/src/Tests/Common/UrlTest.php +++ b/core/modules/system/src/Tests/Common/UrlTest.php @@ -49,16 +49,16 @@ function testLinkXSS() { */ function testLinkBubbleableMetadata() { $cases = [ - ['Regular link', 'internal:/user', [], ['contexts' => [], 'tags' => [], 'max-age' => Cache::PERMANENT]], - ['Regular link, absolute', 'internal:/user', ['absolute' => TRUE], ['contexts' => ['url.site'], 'tags' => [], 'max-age' => Cache::PERMANENT]], - ['Route processor link', 'route:system.run_cron', [], ['contexts' => [], 'tags' => [], 'max-age' => 0]], - ['Route processor link, absolute', 'route:system.run_cron', ['absolute' => TRUE], ['contexts' => ['url.site'], 'tags' => [], 'max-age' => 0]], - ['Path processor link', 'internal:/user/1', [], ['contexts' => [], 'tags' => ['user:1'], 'max-age' => Cache::PERMANENT]], - ['Path processor link, absolute', 'internal:/user/1', ['absolute' => TRUE], ['contexts' => ['url.site'], 'tags' => ['user:1'], 'max-age' => Cache::PERMANENT]], + ['Regular link', 'internal:/user', [], ['contexts' => [], 'tags' => [], 'max-age' => Cache::PERMANENT], []], + ['Regular link, absolute', 'internal:/user', ['absolute' => TRUE], ['contexts' => ['url.site'], 'tags' => [], 'max-age' => Cache::PERMANENT], []], + ['Route processor link', 'route:system.run_cron', [], ['contexts' => ['session', 'user'], 'tags' => [], 'max-age' => Cache::PERMANENT], ['placeholders' => []]], + ['Route processor link, absolute', 'route:system.run_cron', ['absolute' => TRUE], ['contexts' => ['url.site', 'session', 'user'], 'tags' => [], 'max-age' => Cache::PERMANENT], ['placeholders' => []]], + ['Path processor link', 'internal:/user/1', [], ['contexts' => [], 'tags' => ['user:1'], 'max-age' => Cache::PERMANENT], []], + ['Path processor link, absolute', 'internal:/user/1', ['absolute' => TRUE], ['contexts' => ['url.site'], 'tags' => ['user:1'], 'max-age' => Cache::PERMANENT], []], ]; foreach ($cases as $case) { - list($title, $uri, $options, $expected_cacheability) = $case; + list($title, $uri, $options, $expected_cacheability, $expected_attachments) = $case; $expected_cacheability['contexts'] = Cache::mergeContexts($expected_cacheability['contexts'], ['languages:language_interface', 'theme']); $link = [ '#type' => 'link', @@ -69,7 +69,7 @@ function testLinkBubbleableMetadata() { \Drupal::service('renderer')->renderRoot($link); $this->pass($title); $this->assertEqual($expected_cacheability, $link['#cache']); - $this->assertEqual([], $link['#attached']); + $this->assertEqual($expected_attachments, $link['#attached']); } } diff --git a/core/tests/Drupal/Tests/Core/Access/RouteProcessorCsrfTest.php b/core/tests/Drupal/Tests/Core/Access/RouteProcessorCsrfTest.php index 1e29b47..581fb74 100644 --- a/core/tests/Drupal/Tests/Core/Access/RouteProcessorCsrfTest.php +++ b/core/tests/Drupal/Tests/Core/Access/RouteProcessorCsrfTest.php @@ -63,12 +63,6 @@ public function testProcessOutboundNoRequirement() { * Tests the processOutbound() method with a _csrf_token route requirement. */ public function testProcessOutbound() { - $this->csrfToken->expects($this->once()) - ->method('get') - // The leading '/' will be stripped from the path. - ->with('test-path') - ->will($this->returnValue('test_token')); - $route = new Route('/test-path', array(), array('_csrf_token' => 'TRUE')); $parameters = array(); @@ -76,45 +70,71 @@ public function testProcessOutbound() { $this->processor->processOutbound('test', $route, $parameters, $bubbleable_metadata); // 'token' should be added to the parameters array. $this->assertArrayHasKey('token', $parameters); - $this->assertSame($parameters['token'], 'test_token'); - // Cacheability of routes with a _csrf_token route requirement is max-age=0. - $this->assertEquals((new BubbleableMetadata())->setCacheMaxAge(0), $bubbleable_metadata); + // Bubbleable metadata of routes with a _csrf_token route requirement is a + // placeholder. + $path = 'test-path'; + $placeholder = hash('sha1', $path); + $placeholder_render_array = [ + '#lazy_builder' => ['route_processor_csrf:renderPlaceholderCsrfToken', [$path]], + '#cache' => [ + 'contexts' => [ + 'user', + 'session', + ], + ], + ]; + $this->assertSame($parameters['token'], $placeholder); + $this->assertEquals((new BubbleableMetadata())->setAttachments(['placeholders' => [$placeholder => $placeholder_render_array]]), $bubbleable_metadata); } /** * Tests the processOutbound() method with a dynamic path and one replacement. */ public function testProcessOutboundDynamicOne() { - $this->csrfToken->expects($this->once()) - ->method('get') - ->with('test-path/100') - ->will($this->returnValue('test_token')); - $route = new Route('/test-path/{slug}', array(), array('_csrf_token' => 'TRUE')); $parameters = array('slug' => 100); $bubbleable_metadata = new BubbleableMetadata(); $this->processor->processOutbound('test', $route, $parameters, $bubbleable_metadata); - // Cacheability of routes with a _csrf_token route requirement is max-age=0. - $this->assertEquals((new BubbleableMetadata())->setCacheMaxAge(0), $bubbleable_metadata); + // Bubbleable metadata of routes with a _csrf_token route requirement is a + // placeholder. + $path = 'test-path/100'; + $placeholder = hash('sha1', $path); + $placeholder_render_array = [ + '#lazy_builder' => ['route_processor_csrf:renderPlaceholderCsrfToken', [$path]], + '#cache' => [ + 'contexts' => [ + 'user', + 'session', + ], + ], + ]; + $this->assertEquals((new BubbleableMetadata())->setAttachments(['placeholders' => [$placeholder => $placeholder_render_array]]), $bubbleable_metadata); } /** * Tests the processOutbound() method with two parameter replacements. */ public function testProcessOutboundDynamicTwo() { - $this->csrfToken->expects($this->once()) - ->method('get') - ->with('100/test-path/test') - ->will($this->returnValue('test_token')); - $route = new Route('{slug_1}/test-path/{slug_2}', array(), array('_csrf_token' => 'TRUE')); $parameters = array('slug_1' => 100, 'slug_2' => 'test'); $bubbleable_metadata = new BubbleableMetadata(); $this->processor->processOutbound('test', $route, $parameters, $bubbleable_metadata); - // Cacheability of routes with a _csrf_token route requirement is max-age=0. - $this->assertEquals((new BubbleableMetadata())->setCacheMaxAge(0), $bubbleable_metadata); + // Bubbleable metadata of routes with a _csrf_token route requirement is a + // placeholder. + $path = '100/test-path/test'; + $placeholder = hash('sha1', $path); + $placeholder_render_array = [ + '#lazy_builder' => ['route_processor_csrf:renderPlaceholderCsrfToken', [$path]], + '#cache' => [ + 'contexts' => [ + 'user', + 'session', + ], + ], + ]; + $this->assertEquals((new BubbleableMetadata())->setAttachments(['placeholders' => [$placeholder => $placeholder_render_array]]), $bubbleable_metadata); } }