core/core.services.yml | 2 +- .../DefaultExceptionHtmlSubscriber.php | 4 ++ .../EventSubscriber/FinishResponseSubscriber.php | 46 +++++++++++++++++++++- core/lib/Drupal/Core/Routing/AccessAwareRouter.php | 10 ++++- .../Core/Routing/AccessAwareRouterInterface.php | 5 +++ .../modules/node/src/Tests/Views/FrontPageTest.php | 2 +- .../Tests/Cache/PageCacheTagsIntegrationTest.php | 1 + .../system/src/Tests/Routing/RouterTest.php | 38 +++++++++++++++--- .../router_test_directory/router_test.routing.yml | 21 ++++++++++ .../router_test_directory/src/TestControllers.php | 13 ++++++ .../Tests/Core/Routing/AccessAwareRouterTest.php | 23 ++++++++--- 11 files changed, 150 insertions(+), 15 deletions(-) diff --git a/core/core.services.yml b/core/core.services.yml index 589447d..ac57f6e 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -947,7 +947,7 @@ services: class: Drupal\Core\EventSubscriber\FinishResponseSubscriber tags: - { name: event_subscriber } - arguments: ['@language_manager', '@config.factory', '@page_cache_request_policy', '@page_cache_response_policy'] + arguments: ['@language_manager', '@config.factory', '@page_cache_request_policy', '@page_cache_response_policy', '@cache_contexts'] redirect_response_subscriber: class: Drupal\Core\EventSubscriber\RedirectResponseSubscriber arguments: ['@url_generator', '@router.request_context'] diff --git a/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php index a98bf57..f5a4b79 100644 --- a/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php @@ -7,6 +7,7 @@ namespace Drupal\Core\EventSubscriber; +use Drupal\Core\Routing\AccessAwareRouterInterface; use Drupal\Core\Url; use Drupal\Core\Utility\Error; use Psr\Log\LoggerInterface; @@ -113,6 +114,9 @@ protected function makeSubrequest(GetResponseForExceptionEvent $event, $url, $st try { // Persist the 'exception' attribute to the subrequest. $sub_request->attributes->set('exception', $request->attributes->get('exception')); + // Persist the access result attribute to the subrequest, so that the + // error page inherits the access result of the master request. + $sub_request->attributes->set(AccessAwareRouterInterface::ACCESS_RESULT, $request->attributes->get(AccessAwareRouterInterface::ACCESS_RESULT)); // Carry over the session to the subrequest. if ($session = $request->getSession()) { diff --git a/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php index 3c8bdf2..0e508a2 100644 --- a/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php @@ -8,11 +8,15 @@ namespace Drupal\Core\EventSubscriber; use Drupal\Component\Datetime\DateTimePlus; +use Drupal\Core\Cache\Cache; +use Drupal\Core\Cache\CacheableInterface; +use Drupal\Core\Cache\CacheContexts; use Drupal\Core\Config\Config; use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Language\LanguageManagerInterface; use Drupal\Core\PageCache\RequestPolicyInterface; use Drupal\Core\PageCache\ResponsePolicyInterface; +use Drupal\Core\Routing\AccessAwareRouterInterface; use Drupal\Core\Site\Settings; use Symfony\Component\HttpFoundation\BinaryFileResponse; use Symfony\Component\HttpFoundation\Request; @@ -57,6 +61,13 @@ class FinishResponseSubscriber implements EventSubscriberInterface { protected $responsePolicy; /** + * The cache contexts service. + * + * @var \Drupal\Core\Cache\CacheContexts + */ + protected $cacheContexts; + + /** * Constructs a FinishResponseSubscriber object. * * @param \Drupal\Core\Language\LanguageManagerInterface $language_manager @@ -67,12 +78,15 @@ class FinishResponseSubscriber implements EventSubscriberInterface { * A policy rule determining the cacheability of a request. * @param \Drupal\Core\PageCache\ResponsePolicyInterface $response_policy * A policy rule determining the cacheability of a response. + * @param \Drupal\Core\Cache\CacheContexts $cache_contexts + * The cache contexts service. */ - public function __construct(LanguageManagerInterface $language_manager, ConfigFactoryInterface $config_factory, RequestPolicyInterface $request_policy, ResponsePolicyInterface $response_policy) { + public function __construct(LanguageManagerInterface $language_manager, ConfigFactoryInterface $config_factory, RequestPolicyInterface $request_policy, ResponsePolicyInterface $response_policy, CacheContexts $cache_contexts) { $this->languageManager = $language_manager; $this->config = $config_factory->get('system.performance'); $this->requestPolicy = $request_policy; $this->responsePolicy = $response_policy; + $this->cacheContexts = $cache_contexts; } /** @@ -119,6 +133,12 @@ public function onRespond(FilterResponseEvent $event) { $response->headers->set($name, $value, FALSE); } + // Apply the request's access result cacheability metadata, if it has any. + $access_result = $request->attributes->get(AccessAwareRouterInterface::ACCESS_RESULT); + if ($access_result instanceof CacheableInterface) { + $this->updateDrupalCacheHeaders($response, $access_result); + } + $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 @@ -139,6 +159,30 @@ public function onRespond(FilterResponseEvent $event) { } /** + * Updates Drupal's cache headers using the route's cacheable access result. + * + * @param Response $response + * @param CacheableInterface $cacheable_access_result + */ + protected function updateDrupalCacheHeaders(Response $response, CacheableInterface $cacheable_access_result) { + // X-Drupal-Cache-Tags + $cache_tags = $cacheable_access_result->getCacheTags(); + if ($response->headers->has('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)); + + // X-Drupal-Cache-Contexts + $cache_contexts = $cacheable_access_result->getCacheContexts(); + if ($response->headers->has('X-Drupal-Cache-Contexts')) { + $existing_cache_contexts = explode(' ', $response->headers->get('X-Drupal-Cache-Contexts')); + $cache_contexts = Cache::mergeContexts($existing_cache_contexts, $cache_contexts); + } + $response->headers->set('X-Drupal-Cache-Contexts', implode(' ', $this->cacheContexts->optimizeTokens($cache_contexts))); + } + + /** * Determine whether the given response has a custom Cache-Control header. * * Upon construction, the ResponseHeaderBag is initialized with an empty diff --git a/core/lib/Drupal/Core/Routing/AccessAwareRouter.php b/core/lib/Drupal/Core/Routing/AccessAwareRouter.php index 1cf937c..20b0ecf 100644 --- a/core/lib/Drupal/Core/Routing/AccessAwareRouter.php +++ b/core/lib/Drupal/Core/Routing/AccessAwareRouter.php @@ -101,7 +101,15 @@ public function matchRequest(Request $request) { * The request to access check. */ protected function checkAccess(Request $request) { - if (!$this->accessManager->checkRequest($request, $this->account)) { + // The cacheability (if any) of this request's access check result must be + // applied to the response. + $access_result = $this->accessManager->checkRequest($request, $this->account, TRUE); + // Allow a master request to set the access result for a subrequest: if an + // access result attribute is already set, don't overwrite it. + if (!$request->attributes->has(AccessAwareRouterInterface::ACCESS_RESULT)) { + $request->attributes->set(AccessAwareRouterInterface::ACCESS_RESULT, $access_result); + } + if (!$access_result->isAllowed()) { throw new AccessDeniedHttpException(); } } diff --git a/core/lib/Drupal/Core/Routing/AccessAwareRouterInterface.php b/core/lib/Drupal/Core/Routing/AccessAwareRouterInterface.php index 34fa75a..b6cfe4a 100644 --- a/core/lib/Drupal/Core/Routing/AccessAwareRouterInterface.php +++ b/core/lib/Drupal/Core/Routing/AccessAwareRouterInterface.php @@ -16,6 +16,11 @@ interface AccessAwareRouterInterface extends RouterInterface, RequestMatcherInterface { /** + * Attribute name of the access result for the request.. + */ + const ACCESS_RESULT = '_access_result'; + + /** * {@inheritdoc} * * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException diff --git a/core/modules/node/src/Tests/Views/FrontPageTest.php b/core/modules/node/src/Tests/Views/FrontPageTest.php index ed6fd3d..1e27ace 100644 --- a/core/modules/node/src/Tests/Views/FrontPageTest.php +++ b/core/modules/node/src/Tests/Views/FrontPageTest.php @@ -241,7 +241,7 @@ protected function assertFrontPageViewCacheTags($do_assert_views_caches) { $view = Views::getView('frontpage'); $view->setDisplay('page_1'); - $cache_contexts = ['user.node_grants:view', 'languages']; + $cache_contexts = ['user.node_grants:view', 'languages', 'user.permissions']; // Test before there are any nodes. $empty_node_listing_cache_tags = [ diff --git a/core/modules/system/src/Tests/Cache/PageCacheTagsIntegrationTest.php b/core/modules/system/src/Tests/Cache/PageCacheTagsIntegrationTest.php index 8078913..110482d 100644 --- a/core/modules/system/src/Tests/Cache/PageCacheTagsIntegrationTest.php +++ b/core/modules/system/src/Tests/Cache/PageCacheTagsIntegrationTest.php @@ -75,6 +75,7 @@ function testPageCacheTags() { 'route.menu_active_trails:tools', 'theme', 'timezone', + 'user.permissions', 'user.roles', ]; diff --git a/core/modules/system/src/Tests/Routing/RouterTest.php b/core/modules/system/src/Tests/Routing/RouterTest.php index 92e9608..e70ac6a 100644 --- a/core/modules/system/src/Tests/Routing/RouterTest.php +++ b/core/modules/system/src/Tests/Routing/RouterTest.php @@ -22,32 +22,58 @@ class RouterTest extends WebTestBase { * * @var array */ - public static $modules = array('block', 'router_test'); + public static $modules = array('router_test'); /** - * Confirms that our default controller logic works properly. + * Confirms that our FinishResponseSubscriber logic works properly. */ - public function testDefaultController() { + public function testFinishResponseSubscriber() { // Confirm that the router can get to a controller. $this->drupalGet('router_test/test1'); $this->assertRaw('test1', 'The correct string was returned because the route was successful.'); - // Check expected headers from FinishResponseSubscriber. $headers = $this->drupalGetHeaders(); $this->assertEqual($headers['x-ua-compatible'], 'IE=edge'); $this->assertEqual($headers['content-language'], 'en'); $this->assertEqual($headers['x-content-type-options'], 'nosniff'); + $this->drupalGet('router_test/test2'); $this->assertRaw('test2', 'The correct string was returned because the route was successful.'); - + // Check expected headers from FinishResponseSubscriber. + $headers = $this->drupalGetHeaders(); + $this->assertEqual($headers['x-drupal-cache-contexts'], ''); + $this->assertEqual($headers['x-drupal-cache-tags'], 'rendered'); // Confirm that the page wrapping is being added, so we're not getting a // raw body returned. $this->assertRaw('', 'Page markup was found.'); - // In some instances, the subrequest handling may get confused and render // a page inception style. This test verifies that is not happening. $this->assertNoPattern('#.*#s', 'There was no double-page effect from a misrendered subrequest.'); + + + // Confirm that route-level access check's cacheability is applied to the + // X-Drupal-Cache-Contexts and X-Drupal-Cache-Tags headers. + // 1. controller result: render array, globally cacheable route access. + $this->drupalGet('router_test/test18'); + $headers = $this->drupalGetHeaders(); + $this->assertEqual($headers['x-drupal-cache-contexts'], 'url'); + $this->assertEqual($headers['x-drupal-cache-tags'], 'foo rendered'); + // 2. controller result: render array, per-role cacheable route access. + $this->drupalGet('router_test/test19'); + $headers = $this->drupalGetHeaders(); + $this->assertEqual($headers['x-drupal-cache-contexts'], 'url user.roles'); + $this->assertEqual($headers['x-drupal-cache-tags'], 'foo rendered'); + // 3. controller result: Response object, globally cacheable route access. + $this->drupalGet('router_test/test1'); + $headers = $this->drupalGetHeaders(); + $this->assertEqual($headers['x-drupal-cache-contexts'], ''); + $this->assertEqual($headers['x-drupal-cache-tags'], ''); + // 4. controller result: Response object, per-role cacheable route access. + $this->drupalGet('router_test/test20'); + $headers = $this->drupalGetHeaders(); + $this->assertEqual($headers['x-drupal-cache-contexts'], 'user.roles'); + $this->assertEqual($headers['x-drupal-cache-tags'], ''); } /** diff --git a/core/modules/system/tests/modules/router_test_directory/router_test.routing.yml b/core/modules/system/tests/modules/router_test_directory/router_test.routing.yml index 97b0b0c..08dd057 100644 --- a/core/modules/system/tests/modules/router_test_directory/router_test.routing.yml +++ b/core/modules/system/tests/modules/router_test_directory/router_test.routing.yml @@ -106,6 +106,27 @@ router_test.17: requirements: _access: 'TRUE' +router_test.18: + path: '/router_test/test18' + defaults: + _controller: '\Drupal\router_test\TestControllers::test18' + requirements: + _access: 'TRUE' + +router_test.19: + path: '/router_test/test19' + defaults: + _controller: '\Drupal\router_test\TestControllers::test18' + requirements: + _role: 'anonymous' + +router_test.20: + path: '/router_test/test20' + defaults: + _controller: '\Drupal\router_test\TestControllers::test1' + requirements: + _role: 'anonymous' + router_test.hierarchy_parent: path: '/menu-test/parent' defaults: diff --git a/core/modules/system/tests/modules/router_test_directory/src/TestControllers.php b/core/modules/system/tests/modules/router_test_directory/src/TestControllers.php index 0af49b0..f2a1fd2 100644 --- a/core/modules/system/tests/modules/router_test_directory/src/TestControllers.php +++ b/core/modules/system/tests/modules/router_test_directory/src/TestControllers.php @@ -84,6 +84,19 @@ public function test10() { $this->throwException(''); } + public function test18() { + return [ + '#cache' => [ + 'contexts' => ['url'], + 'tags' => ['foo'], + 'max-age' => 60, + ], + 'content' => [ + '#markup' => 'test18', + ], + ]; + } + /** * Throws an exception. * diff --git a/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php b/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php index 3e8676f..da58cd9 100644 --- a/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php +++ b/core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php @@ -7,7 +7,9 @@ namespace Drupal\Tests\Core\Routing; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Routing\AccessAwareRouter; +use Drupal\Core\Routing\AccessAwareRouterInterface; use Drupal\Tests\UnitTestCase; use Symfony\Cmf\Component\Routing\RouteObjectInterface; use Symfony\Component\HttpFoundation\Request; @@ -73,13 +75,18 @@ protected function setupRouter() { public function testMatchRequestAllowed() { $this->setupRouter(); $request = new Request(); + $access_result = AccessResult::allowed(); $this->accessManager->expects($this->once()) ->method('checkRequest') ->with($request) - ->will($this->returnValue(TRUE)); + ->willReturn($access_result); $parameters = $this->router->matchRequest($request); - $this->assertSame($request->attributes->all(), array(RouteObjectInterface::ROUTE_OBJECT => $this->route)); - $this->assertSame($parameters, array(RouteObjectInterface::ROUTE_OBJECT => $this->route)); + $expected = [ + RouteObjectInterface::ROUTE_OBJECT => $this->route, + AccessAwareRouterInterface::ACCESS_RESULT => $access_result, + ]; + $this->assertSame($expected, $request->attributes->all()); + $this->assertSame($expected, $parameters); } /** @@ -90,11 +97,17 @@ public function testMatchRequestAllowed() { public function testMatchRequestDenied() { $this->setupRouter(); $request = new Request(); + $access_result = AccessResult::forbidden(); $this->accessManager->expects($this->once()) ->method('checkRequest') ->with($request) - ->will($this->returnValue(FALSE)); - $this->router->matchRequest($request); + ->willReturn($access_result); + $parameters = $this->router->matchRequest($request); + $expected = [ + AccessAwareRouterInterface::ACCESS_RESULT => $access_result, + ]; + $this->assertSame($expected, $request->attributes->all()); + $this->assertSame($expected, $parameters); } /**