From e24cd89759e26d21a7b8df5413708d534df3078e Mon Sep 17 00:00:00 2001 From: catch <catch@35733.no-reply.drupal.org> Date: Fri, 11 Feb 2022 12:33:39 +0000 Subject: [PATCH] Issue #2949457 by idebr, jibran, Wim Leers, dungahk, fago, kim.pepper, neclimdul, ravi.shankar, Suresh Prabhu Parkala, Sam152, joshua1234511, Kristen Pol, kualee, MiroslavBanov, acbramley, alexpott, Fabianx: Enhance Toolbar's subtree caching so that menu links with CSRF token do not need one subtree cache item per session --- .../Core/Access/CsrfRequestHeaderAccessCheck.php | 2 +- .../modules/csrf_test/csrf_test.links.menu.yml | 13 +++++++++++++ .../tests/modules/csrf_test/csrf_test.routing.yml | 7 +++++++ .../tests/src/Functional/CsrfRequestHeaderTest.php | 2 +- .../toolbar/src/Controller/ToolbarController.php | 6 +++++- .../src/Functional/ToolbarCacheContextsTest.php | 7 ++++++- core/modules/toolbar/toolbar.module | 10 +++++++++- 7 files changed, 42 insertions(+), 5 deletions(-) create mode 100644 core/modules/system/tests/modules/csrf_test/csrf_test.links.menu.yml diff --git a/core/lib/Drupal/Core/Access/CsrfRequestHeaderAccessCheck.php b/core/lib/Drupal/Core/Access/CsrfRequestHeaderAccessCheck.php index 052d9d12ffe6..f2ba6cb462ff 100644 --- a/core/lib/Drupal/Core/Access/CsrfRequestHeaderAccessCheck.php +++ b/core/lib/Drupal/Core/Access/CsrfRequestHeaderAccessCheck.php @@ -57,7 +57,7 @@ public function applies(Route $route) { '_access_rest_csrf', ]; if ($route->hasRequirement('_access_rest_csrf')) { - @trigger_error('Route requirement _access_rest_csrf is deprecated in drupal:8.2.0 and is removed in drupal:10.0.0. Use _csrf_request_header_token instead. See https://www.drupal.org/node/2772399', E_USER_DEPRECATED); + @trigger_error('Route requirement _access_rest_csrf is deprecated in drupal:9.2.0 and is removed in drupal:10.0.0. Use _csrf_request_header_token instead. See https://www.drupal.org/node/2772399', E_USER_DEPRECATED); } $requirement_keys = array_keys($requirements); diff --git a/core/modules/system/tests/modules/csrf_test/csrf_test.links.menu.yml b/core/modules/system/tests/modules/csrf_test/csrf_test.links.menu.yml new file mode 100644 index 000000000000..0966a977c987 --- /dev/null +++ b/core/modules/system/tests/modules/csrf_test/csrf_test.links.menu.yml @@ -0,0 +1,13 @@ +csrf_test.help: + title: 'Tools' + route_name: <front> + menu_name: admin + parent: system.admin + weight: -100 + +csrf_test.protected: + title: 'Route with csrf protection' + route_name: csrf_test.route_with_csrf_token + menu_name: admin + parent: system.admin + weight: -100 diff --git a/core/modules/system/tests/modules/csrf_test/csrf_test.routing.yml b/core/modules/system/tests/modules/csrf_test/csrf_test.routing.yml index 56e64dcd5f74..d177a2a83f3c 100644 --- a/core/modules/system/tests/modules/csrf_test/csrf_test.routing.yml +++ b/core/modules/system/tests/modules/csrf_test/csrf_test.routing.yml @@ -25,3 +25,10 @@ csrf_test.deprecated.csrftoken: _controller: '\Drupal\csrf_test\Controller\DeprecatedCsrfTokenController::csrfToken' requirements: _access: 'TRUE' +csrf_test.route_with_csrf_token: + path: csrf/protected/route + defaults: + _controller: '\Drupal\csrf_test\Controller\TestController::testMethod' + requirements: + _permission: 'administer site configuration' + _csrf_token: 'TRUE' diff --git a/core/modules/system/tests/src/Functional/CsrfRequestHeaderTest.php b/core/modules/system/tests/src/Functional/CsrfRequestHeaderTest.php index 7fab46fc1e5e..9839bb42a593 100644 --- a/core/modules/system/tests/src/Functional/CsrfRequestHeaderTest.php +++ b/core/modules/system/tests/src/Functional/CsrfRequestHeaderTest.php @@ -33,7 +33,7 @@ class CsrfRequestHeaderTest extends BrowserTestBase { * @group legacy */ public function testRouteAccess() { - $this->expectDeprecation('Route requirement _access_rest_csrf is deprecated in drupal:8.2.0 and is removed in drupal:10.0.0. Use _csrf_request_header_token instead. See https://www.drupal.org/node/2772399'); + $this->expectDeprecation('Route requirement _access_rest_csrf is deprecated in drupal:9.2.0 and is removed in drupal:10.0.0. Use _csrf_request_header_token instead. See https://www.drupal.org/node/2772399'); $client = $this->getHttpClient(); $csrf_token_paths = ['deprecated/session/token', 'session/token']; // Test using the both the current path and a test path that returns diff --git a/core/modules/toolbar/src/Controller/ToolbarController.php b/core/modules/toolbar/src/Controller/ToolbarController.php index 75986f901b0f..d811192fbb4d 100644 --- a/core/modules/toolbar/src/Controller/ToolbarController.php +++ b/core/modules/toolbar/src/Controller/ToolbarController.php @@ -7,6 +7,7 @@ use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Controller\ControllerBase; use Drupal\Core\Menu\MenuTreeParameters; +use Drupal\Core\Render\RenderContext; use Drupal\Core\Security\TrustedCallbackInterface; use Drupal\toolbar\Ajax\SetSubtreesCommand; @@ -91,6 +92,7 @@ public static function preRenderAdministrationTray(array $element) { */ public static function preRenderGetRenderedSubtrees(array $data) { $menu_tree = \Drupal::service('toolbar.menu_tree'); + $renderer = \Drupal::service('renderer'); // Load the administration menu. The first level is the "Administration" // link. In order to load the children of that link and the subsequent two // levels, start at the second level and end at the fourth. @@ -112,7 +114,9 @@ public static function preRenderGetRenderedSubtrees(array $data) { $link = $element->link; if ($element->subtree) { $subtree = $menu_tree->build($element->subtree); - $output = \Drupal::service('renderer')->renderPlain($subtree); + $output = $renderer->executeInRenderContext(new RenderContext(), function () use ($renderer, $subtree) { + return $renderer->render($subtree); + }); $cacheability = $cacheability->merge(CacheableMetadata::createFromRenderArray($subtree)); } else { diff --git a/core/modules/toolbar/tests/src/Functional/ToolbarCacheContextsTest.php b/core/modules/toolbar/tests/src/Functional/ToolbarCacheContextsTest.php index c7ff8b304f48..627b1ce75b5d 100644 --- a/core/modules/toolbar/tests/src/Functional/ToolbarCacheContextsTest.php +++ b/core/modules/toolbar/tests/src/Functional/ToolbarCacheContextsTest.php @@ -65,14 +65,19 @@ protected function setUp(): void { /** * Tests toolbar cache integration. + * + * @group legacy */ public function testCacheIntegration() { - $this->installExtraModules(['dynamic_page_cache']); + $this->expectDeprecation('Route requirement _access_rest_csrf is deprecated in drupal:9.2.0 and is removed in drupal:10.0.0. Use _csrf_request_header_token instead. See https://www.drupal.org/node/2772399'); + $this->installExtraModules(['csrf_test', 'dynamic_page_cache']); $this->drupalLogin($this->adminUser); $this->drupalGet('test-page'); $this->assertSession()->responseHeaderEquals('X-Drupal-Dynamic-Cache', 'MISS'); + $this->assertCacheContexts(['session', 'user', 'url.query_args:' . MainContentViewSubscriber::WRAPPER_FORMAT], 'Expected cache contexts found with CSRF token link.'); $this->drupalGet('test-page'); $this->assertSession()->responseHeaderEquals('X-Drupal-Dynamic-Cache', 'HIT'); + $this->assertCacheContexts(['session', 'user', 'url.query_args:' . MainContentViewSubscriber::WRAPPER_FORMAT], 'Expected cache contexts found with CSRF token link.'); } /** diff --git a/core/modules/toolbar/toolbar.module b/core/modules/toolbar/toolbar.module index 963c708c9f9e..e02fa585ecb2 100644 --- a/core/modules/toolbar/toolbar.module +++ b/core/modules/toolbar/toolbar.module @@ -7,6 +7,7 @@ use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Render\Element; +use Drupal\Core\Render\RenderContext; use Drupal\Core\Routing\RouteMatchInterface; use Drupal\Core\Template\Attribute; use Drupal\Component\Utility\Crypt; @@ -275,7 +276,14 @@ function toolbar_get_rendered_subtrees() { ], '#cache_properties' => ['#subtrees'], ]; - \Drupal::service('renderer')->renderPlain($data); + /** @var \Drupal\Core\Render\Renderer $renderer */ + $renderer = \Drupal::service('renderer'); + // The pre_render process populates $data during the render pipeline. + // We need to pass by reference so that populated data can be returned and + // used to resolve cacheability. + $renderer->executeInRenderContext(new RenderContext(), function () use ($renderer, &$data) { + $renderer->render($data); + }); return [$data['#subtrees'], CacheableMetadata::createFromRenderArray($data)]; } -- GitLab