.../EventSubscriber/FinishResponseSubscriber.php | 16 +++++++++++++++ .../LanguageNegotiationBrowser.php | 6 +++++- .../system/config/install/system.performance.yml | 2 +- .../system/src/Tests/Session/SessionTest.php | 6 +++++- .../system/src/Tests/System/CronRunTest.php | 8 ++++++-- core/modules/views/src/Tests/GlossaryTest.php | 23 +++++++++++----------- core/modules/views/src/Tests/Plugin/AccessTest.php | 8 ++++++++ sites/example.settings.local.php | 2 +- 8 files changed, 54 insertions(+), 17 deletions(-) diff --git a/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php index c1daada..269d452 100644 --- a/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php @@ -139,6 +139,22 @@ public function onRespond(FilterResponseEvent $event) { $this->updateDrupalCacheHeaders($response, $access_result); } + // The 'user.permissions' cache context ensures that if the permissions for + // a role are modified, users aren't served stale render cache content. But, + // when entire responses are cached in reverse proxies, the value for the + // cache context is never calculated, causing the stale response to not be + // invalidated. Therefore, when varying by permissions and the current user + // is the anonymous user, also add the cache tag for the 'anonymous' role. + $cache_contexts = $response->headers->get('X-Drupal-Cache-Contexts'); + if ($cache_contexts && in_array('user.permissions', explode(' ', $cache_contexts)) && \Drupal::currentUser()->isAnonymous()) { + $cache_tags = ['config:user.role.anonymous']; + if ($response->headers->get('X-Drupal-Cache-Tags')) { + $existing_cache_tags = explode(' ', $response->headers->get('X-Drupal-Cache-Tags')); + $cache_tags = Cache::mergeTags($existing_cache_tags, $cache_tags); + } + $response->headers->set('X-Drupal-Cache-Tags', implode(' ', $cache_tags)); + } + $is_cacheable = ($this->requestPolicy->check($request) === RequestPolicyInterface::ALLOW) && ($this->responsePolicy->check($response, $request) !== ResponsePolicyInterface::DENY); // Add headers necessary to specify whether the response should be cached by diff --git a/core/modules/language/src/Plugin/LanguageNegotiation/LanguageNegotiationBrowser.php b/core/modules/language/src/Plugin/LanguageNegotiation/LanguageNegotiationBrowser.php index 9acd295..af36c75 100644 --- a/core/modules/language/src/Plugin/LanguageNegotiation/LanguageNegotiationBrowser.php +++ b/core/modules/language/src/Plugin/LanguageNegotiation/LanguageNegotiationBrowser.php @@ -17,7 +17,6 @@ * @Plugin( * id = \Drupal\language\Plugin\LanguageNegotiation\LanguageNegotiationBrowser::METHOD_ID, * weight = -2, - * cache = 0, * name = @Translation("Browser"), * description = @Translation("Language from the browser's language settings."), * config_route_name = "language.negotiation_browser" @@ -36,6 +35,11 @@ class LanguageNegotiationBrowser extends LanguageNegotiationMethodBase { public function getLangcode(Request $request = NULL) { $langcode = NULL; + // Whenever browser-based language negotiation is used, the page cannot be + // cached by reverse proxies. + // @todo Solve more elegantly in https://www.drupal.org/node/2430335. + \Drupal::service('page_cache_kill_switch')->trigger(); + if ($this->languageManager && $request && $request->server->get('HTTP_ACCEPT_LANGUAGE')) { $http_accept_language = $request->server->get('HTTP_ACCEPT_LANGUAGE'); $langcodes = array_keys($this->languageManager->getLanguages()); diff --git a/core/modules/system/config/install/system.performance.yml b/core/modules/system/config/install/system.performance.yml index 1e75b4b..98f34f0 100644 --- a/core/modules/system/config/install/system.performance.yml +++ b/core/modules/system/config/install/system.performance.yml @@ -1,6 +1,6 @@ cache: page: - use_internal: false + use_internal: true max_age: 0 css: preprocess: true diff --git a/core/modules/system/src/Tests/Session/SessionTest.php b/core/modules/system/src/Tests/Session/SessionTest.php index 0921f66..75b194a 100644 --- a/core/modules/system/src/Tests/Session/SessionTest.php +++ b/core/modules/system/src/Tests/Session/SessionTest.php @@ -141,7 +141,11 @@ function testDataPersistence() { * Test that empty anonymous sessions are destroyed. */ function testEmptyAnonymousSession() { - // Verify that no session is automatically created for anonymous user. + // Verify that no session is automatically created for anonymous user when + // page caching is disabled. + $config = $this->config('system.performance'); + $config->set('cache.page.use_internal', 0); + $config->save(); $this->drupalGet(''); $this->assertSessionCookie(FALSE); $this->assertSessionEmpty(TRUE); diff --git a/core/modules/system/src/Tests/System/CronRunTest.php b/core/modules/system/src/Tests/System/CronRunTest.php index de8cdcc..e17bbf3 100644 --- a/core/modules/system/src/Tests/System/CronRunTest.php +++ b/core/modules/system/src/Tests/System/CronRunTest.php @@ -49,6 +49,12 @@ function testCronRun() { * need the exact time when cron is triggered. */ function testAutomaticCron() { + // Test with a logged in user; anonymous users likely don't cause Drupal to + // fully bootstrap, because of the internal page cache or an external + // reverse proxy. Reuse this user for disabling cron later in the test. + $admin_user = $this->drupalCreateUser(array('administer site configuration')); + $this->drupalLogin($admin_user); + // Ensure cron does not run when the cron threshold is enabled and was // not passed. $cron_last = time(); @@ -68,8 +74,6 @@ function testAutomaticCron() { $this->assertTrue($cron_last < \Drupal::state()->get('system.cron_last'), 'Cron runs when the cron threshold is passed.'); // Disable the cron threshold through the interface. - $admin_user = $this->drupalCreateUser(array('administer site configuration')); - $this->drupalLogin($admin_user); $this->drupalPostForm('admin/config/system/cron', array('cron_safe_threshold' => 0), t('Save configuration')); $this->assertText(t('The configuration options have been saved.')); $this->drupalLogout(); diff --git a/core/modules/views/src/Tests/GlossaryTest.php b/core/modules/views/src/Tests/GlossaryTest.php index ec27ea9..54172ee 100644 --- a/core/modules/views/src/Tests/GlossaryTest.php +++ b/core/modules/views/src/Tests/GlossaryTest.php @@ -70,10 +70,20 @@ public function testGlossaryView() { // Enable the glossary to be displayed. $view->storage->enable()->save(); $this->container->get('router.builder')->rebuildIfNeeded(); + $url = Url::fromRoute('view.glossary.page_1'); + + // Verify cache tags. + $this->assertPageCacheContextsAndTags($url, ['languages', 'theme', 'url', 'user.node_grants:view', 'user.permissions'], [ + 'config:views.view.glossary', + 'node:' . $nodes_by_char['a'][0]->id(), 'node:' . $nodes_by_char['a'][1]->id(), 'node:' . $nodes_by_char['a'][2]->id(), + 'node_list', + 'user_list', + 'rendered', + ]); + // Check the actual page response. - $this->drupalGet('glossary'); + $this->drupalGet($url); $this->assertResponse(200); - foreach ($nodes_per_char as $char => $count) { $href = Url::fromRoute('view.glossary.page_1', ['arg_0' => $char])->toString(); $label = Unicode::strtoupper($char); @@ -86,15 +96,6 @@ public function testGlossaryView() { $this->assertEqual($result_count, $count, 'The expected number got rendered.'); } - // Verify cache tags. - $this->enablePageCaching(); - $this->assertPageCacheContextsAndTags(Url::fromRoute('view.glossary.page_1'), ['languages', 'theme', 'url', 'user.node_grants:view', 'user.permissions'], [ - 'config:views.view.glossary', - 'node:' . $nodes_by_char['a'][0]->id(), 'node:' . $nodes_by_char['a'][1]->id(), 'node:' . $nodes_by_char['a'][2]->id(), - 'node_list', - 'user_list', - 'rendered', - ]); } } diff --git a/core/modules/views/src/Tests/Plugin/AccessTest.php b/core/modules/views/src/Tests/Plugin/AccessTest.php index 4bc77db..c756706 100644 --- a/core/modules/views/src/Tests/Plugin/AccessTest.php +++ b/core/modules/views/src/Tests/Plugin/AccessTest.php @@ -7,6 +7,7 @@ namespace Drupal\views\Tests\Plugin; +use Drupal\Core\Cache\Cache; use Drupal\views\Tests\ViewTestData; use Drupal\views\Views; @@ -101,6 +102,13 @@ function testStaticAccessPlugin() { // termination event fires. Simulate that here. $this->container->get('router.builder')->rebuildIfNeeded(); + // Clear the page cache. + // @todo Remove this. The root cause is that the access plugins alters the + // route's access requirements. That means that the 403 from above does + // not have any cache tags, so modifying the View entity does not cause + // the cached 403 page to be invalidated. + Cache::invalidateTags(['rendered']); + $this->assertTrue($access_plugin->access($this->normalUser)); $this->drupalGet('test_access_static'); diff --git a/sites/example.settings.local.php b/sites/example.settings.local.php index 7859fe5..4cc2109 100644 --- a/sites/example.settings.local.php +++ b/sites/example.settings.local.php @@ -28,7 +28,7 @@ $config['system.performance']['js']['preprocess'] = FALSE; /** - * Disable the render cache. + * Disable the render cache (this includes the page cache). * * This setting disables the render cache by using the Null cache back-end * defined by the development.services.yml file above.