From b8ae2f145537c80570e5581214165a9cd195f0ee Mon Sep 17 00:00:00 2001 From: catch <catch@35733.no-reply.drupal.org> Date: Sun, 6 Aug 2023 10:56:06 +0100 Subject: [PATCH] Issue #3374253 by TwoD, smustgrave, catch, lauriii: The renderer throws away cache metadata from access result if it is not allowed --- core/lib/Drupal/Core/Render/Renderer.php | 11 ++++- core/phpstan-baseline.neon | 5 --- .../Drupal/Tests/Core/Render/RendererTest.php | 42 ++++++++++++++++--- 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/core/lib/Drupal/Core/Render/Renderer.php b/core/lib/Drupal/Core/Render/Renderer.php index 6d08e4d81c95..5d794f451787 100644 --- a/core/lib/Drupal/Core/Render/Renderer.php +++ b/core/lib/Drupal/Core/Render/Renderer.php @@ -233,6 +233,15 @@ protected function doRender(&$elements, $is_root_call = FALSE) { if ($elements['#access'] instanceof AccessResultInterface) { $this->addCacheableDependency($elements, $elements['#access']); if (!$elements['#access']->isAllowed()) { + // Abort, but bubble new cache metadata from the access result. + $context = $this->getCurrentRenderContext(); + if (!isset($context)) { + trigger_error("Render context is empty, because render() was called outside of a renderRoot() or renderPlain() call. Use renderPlain()/renderRoot() or #lazy_builder/#pre_render instead.", E_USER_WARNING); + return ''; + } + $context->push(new BubbleableMetadata()); + $context->update($elements); + $context->bubble(); return ''; } } @@ -592,7 +601,7 @@ public function executeInRenderContext(RenderContext $context, callable $callabl /** * Returns the current render context. * - * @return \Drupal\Core\Render\RenderContext + * @return \Drupal\Core\Render\RenderContext|null * The current render context. */ protected function getCurrentRenderContext() { diff --git a/core/phpstan-baseline.neon b/core/phpstan-baseline.neon index ae4b01cb4791..62bb1e0a6f31 100644 --- a/core/phpstan-baseline.neon +++ b/core/phpstan-baseline.neon @@ -645,11 +645,6 @@ parameters: count: 1 path: lib/Drupal/Core/Render/RenderCache.php - - - message: "#^Variable \\$context in isset\\(\\) always exists and is not nullable\\.$#" - count: 1 - path: lib/Drupal/Core/Render/Renderer.php - - message: "#^Variable \\$transaction in isset\\(\\) always exists and is not nullable\\.$#" count: 1 diff --git a/core/tests/Drupal/Tests/Core/Render/RendererTest.php b/core/tests/Drupal/Tests/Core/Render/RendererTest.php index 7a2a7eedfbb5..3a942e829f84 100644 --- a/core/tests/Drupal/Tests/Core/Render/RendererTest.php +++ b/core/tests/Drupal/Tests/Core/Render/RendererTest.php @@ -790,14 +790,48 @@ public function testRenderWithThemeArguments() { $this->assertEquals($this->renderer->renderRoot($element), $element['#foo'] . $element['#bar'], 'Passing arguments to theme functions works'); } + /** + * Provides a list of access conditions and expected cache metadata. + * + * @return array + */ + public function providerRenderCache() { + return [ + 'full access' => [ + NULL, + [ + 'render_cache_tag', + 'render_cache_tag_child:1', + 'render_cache_tag_child:2', + ], + ], + 'no child access' => [ + AccessResult::forbidden() + ->addCacheTags([ + 'render_cache_tag_child_access:1', + 'render_cache_tag_child_access:2', + ]), + [ + 'render_cache_tag', + 'render_cache_tag_child:1', + 'render_cache_tag_child:2', + 'render_cache_tag_child_access:1', + 'render_cache_tag_child_access:2', + ], + ], + ]; + } + /** * @covers ::render * @covers ::doRender * @covers \Drupal\Core\Render\RenderCache::get * @covers \Drupal\Core\Render\RenderCache::set * @covers \Drupal\Core\Render\RenderCache::createCacheID + * + * @dataProvider providerRenderCache */ - public function testRenderCache() { + public function testRenderCache($child_access, $expected_tags) { $this->setUpRequest(); $this->setupMemoryCache(); @@ -809,6 +843,7 @@ public function testRenderCache() { ], '#markup' => '', 'child' => [ + '#access' => $child_access, '#cache' => [ 'keys' => ['render_cache_test_child'], 'tags' => ['render_cache_tag_child:1', 'render_cache_tag_child:2'], @@ -831,11 +866,6 @@ public function testRenderCache() { // Test that cache tags are correctly collected from the render element, // including the ones from its subchild. - $expected_tags = [ - 'render_cache_tag', - 'render_cache_tag_child:1', - 'render_cache_tag_child:2', - ]; $this->assertEquals($expected_tags, $element['#cache']['tags'], 'Cache tags were collected from the element and its subchild.'); // The cache item also has a 'rendered' cache tag. -- GitLab