From f2daf5121b5697a4d659573aba250091519545ea Mon Sep 17 00:00:00 2001
From: catch <catch@35733.no-reply.drupal.org>
Date: Sun, 31 Dec 2023 10:24:08 +0000
Subject: [PATCH] Issue #3379220 by kristiaanvandeneynde, smustgrave, Wim
Leers: system_page_attachments() varies by authenticated user role but does
not add said cache context
---
core/modules/system/system.module | 1 +
.../AssertPageCacheContextsAndTagsTrait.php | 13 +++++++++++--
.../Entity/EntityCacheTagsTestBase.php | 2 +-
.../tests/src/Functional/Routing/RouterTest.php | 4 ++--
.../FunctionalTests/BrowserTestBaseTest.php | 16 ++++++++++++++++
5 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/core/modules/system/system.module b/core/modules/system/system.module
index 73c5634e4ec4..7bbb411c5d24 100644
--- a/core/modules/system/system.module
+++ b/core/modules/system/system.module
@@ -672,6 +672,7 @@ function system_page_attachments(array &$page) {
// @see \Drupal\Core\Utility\LinkGenerator::generate()
// @see template_preprocess_links()
// @see \Drupal\Core\EventSubscriber\ActiveLinkResponseFilter
+ $page['#cache']['contexts'][] = 'user.roles:authenticated';
if (\Drupal::currentUser()->isAuthenticated()) {
$page['#attached']['library'][] = 'core/drupal.active-link';
}
diff --git a/core/modules/system/tests/src/Functional/Cache/AssertPageCacheContextsAndTagsTrait.php b/core/modules/system/tests/src/Functional/Cache/AssertPageCacheContextsAndTagsTrait.php
index fefd3a462b5e..36925395091d 100644
--- a/core/modules/system/tests/src/Functional/Cache/AssertPageCacheContextsAndTagsTrait.php
+++ b/core/modules/system/tests/src/Functional/Cache/AssertPageCacheContextsAndTagsTrait.php
@@ -130,10 +130,19 @@ protected function assertCacheTags(array $expected_tags, $include_default_tags =
protected function assertCacheContexts(array $expected_contexts, $message = NULL, $include_default_contexts = TRUE) {
if ($include_default_contexts) {
$default_contexts = ['languages:language_interface', 'theme'];
- // Add the user.permission context to the list of default contexts except
- // when user is already there.
+ // Add the user based contexts to the list of default contexts except when
+ // user is already there.
if (!in_array('user', $expected_contexts)) {
$default_contexts[] = 'user.permissions';
+
+ if (!in_array('user.roles', $expected_contexts)) {
+ // The system_page_attachments() hook is only called when dealing with
+ // the HtmlRenderer, so check the Content-Type header.
+ // @see \Drupal\Core\Render\MainContent\HtmlRenderer::invokePageAttachmentHooks()
+ if ($this->getSession()->getResponseHeader('Content-Type') === 'text/html; charset=UTF-8') {
+ $default_contexts[] = 'user.roles:authenticated';
+ }
+ }
}
$expected_contexts = Cache::mergeContexts($expected_contexts, $default_contexts);
}
diff --git a/core/modules/system/tests/src/Functional/Entity/EntityCacheTagsTestBase.php b/core/modules/system/tests/src/Functional/Entity/EntityCacheTagsTestBase.php
index b2e340583e76..20c6c9238464 100644
--- a/core/modules/system/tests/src/Functional/Entity/EntityCacheTagsTestBase.php
+++ b/core/modules/system/tests/src/Functional/Entity/EntityCacheTagsTestBase.php
@@ -322,7 +322,7 @@ public function testReferencedEntity() {
// The default cache contexts for rendered entities.
$default_cache_contexts = ['languages:' . LanguageInterface::TYPE_INTERFACE, 'theme', 'user.permissions'];
$entity_cache_contexts = Cache::mergeContexts($default_cache_contexts, ['url.site']);
- $page_cache_contexts = Cache::mergeContexts($default_cache_contexts, ['url.query_args:' . MainContentViewSubscriber::WRAPPER_FORMAT]);
+ $page_cache_contexts = Cache::mergeContexts($default_cache_contexts, ['url.query_args:' . MainContentViewSubscriber::WRAPPER_FORMAT, 'user.roles:authenticated']);
// Cache tags present on every rendered page.
// 'user.permissions' is a required cache context, and responses that vary
diff --git a/core/modules/system/tests/src/Functional/Routing/RouterTest.php b/core/modules/system/tests/src/Functional/Routing/RouterTest.php
index 02137db62fb6..f780ee2d19bb 100644
--- a/core/modules/system/tests/src/Functional/Routing/RouterTest.php
+++ b/core/modules/system/tests/src/Functional/Routing/RouterTest.php
@@ -35,7 +35,7 @@ class RouterTest extends BrowserTestBase {
*/
public function testFinishResponseSubscriber() {
$renderer_required_cache_contexts = ['languages:' . LanguageInterface::TYPE_INTERFACE, 'theme', 'user.permissions'];
- $expected_cache_contexts = Cache::mergeContexts($renderer_required_cache_contexts, ['url.query_args:' . MainContentViewSubscriber::WRAPPER_FORMAT]);
+ $expected_cache_contexts = Cache::mergeContexts($renderer_required_cache_contexts, ['url.query_args:' . MainContentViewSubscriber::WRAPPER_FORMAT, 'user.roles:authenticated']);
sort($expected_cache_contexts);
// Confirm that the router can get to a controller.
@@ -69,7 +69,7 @@ public function testFinishResponseSubscriber() {
// X-Drupal-Cache-Contexts and X-Drupal-Cache-Tags headers.
// 1. controller result: render array, globally cacheable route access.
$this->drupalGet('router_test/test18');
- $expected_cache_contexts = Cache::mergeContexts($renderer_required_cache_contexts, ['url']);
+ $expected_cache_contexts = Cache::mergeContexts($renderer_required_cache_contexts, ['url', 'user.roles:authenticated']);
sort($expected_cache_contexts);
$this->assertSession()->responseHeaderEquals('X-Drupal-Cache-Contexts', implode(' ', $expected_cache_contexts));
$this->assertSession()->responseHeaderEquals('X-Drupal-Cache-Tags', 'config:user.role.anonymous foo http_response rendered');
diff --git a/core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php b/core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php
index f0035f4b1f4e..f1da9fc46f09 100644
--- a/core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php
+++ b/core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php
@@ -40,6 +40,22 @@ class BrowserTestBaseTest extends BrowserTestBase {
*/
protected $defaultTheme = 'stark';
+ /**
+ * Tests that JavaScript Drupal settings can be read.
+ */
+ public function testDrupalSettings() {
+ // Trigger a 403 because those pages have very little else going on.
+ $this->drupalGet('admin');
+ $this->assertSame([], $this->getDrupalSettings());
+
+ // Now try the same 403 as an authenticated user and verify that Drupal
+ // settings do show up.
+ $account = $this->drupalCreateUser();
+ $this->drupalLogin($account);
+ $this->drupalGet('admin');
+ $this->assertNotSame([], $this->getDrupalSettings());
+ }
+
/**
* Tests basic page test.
*/
--
GitLab