From f992a8bf1d208dd446a9e05727e60977d054e7e1 Mon Sep 17 00:00:00 2001 From: catch Date: Mon, 21 Jun 2021 16:12:12 +0100 Subject: [PATCH] Issue #3183036 by claudiu.cristea, jibran, Berdir: Don't instantiate access checkers not used by any route --- core/lib/Drupal/Core/Access/CheckProvider.php | 20 ++++--------------- .../Compiler/RegisterAccessChecksPass.php | 10 ++++++++++ .../Tests/Core/Access/AccessManagerTest.php | 4 ++++ 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/core/lib/Drupal/Core/Access/CheckProvider.php b/core/lib/Drupal/Core/Access/CheckProvider.php index 51423bdab4e..87cc66c3be6 100644 --- a/core/lib/Drupal/Core/Access/CheckProvider.php +++ b/core/lib/Drupal/Core/Access/CheckProvider.php @@ -129,12 +129,14 @@ class CheckProvider implements CheckProviderInterface, ContainerAwareInterface { foreach ($route->getRequirements() as $key => $value) { if (isset($this->staticRequirementMap[$key])) { foreach ($this->staticRequirementMap[$key] as $service_id) { + $this->loadCheck($service_id); $checks[] = $service_id; } } } // Finally, see if any dynamic access checkers apply. foreach ($this->dynamicRequirementMap as $service_id) { + $this->loadCheck($service_id); if ($this->checks[$service_id]->applies($route)) { $checks[] = $service_id; } @@ -147,22 +149,8 @@ class CheckProvider implements CheckProviderInterface, ContainerAwareInterface { * Compiles a mapping of requirement keys to access checker service IDs. */ protected function loadDynamicRequirementMap() { - if (isset($this->dynamicRequirementMap)) { - return; - } - - // Set them here, so we can use the isset() check above. - $this->dynamicRequirementMap = []; - - foreach ($this->checkIds as $service_id) { - if (empty($this->checks[$service_id])) { - $this->loadCheck($service_id); - } - - // Add the service ID to an array that will be iterated over. - if ($this->checks[$service_id] instanceof AccessCheckInterface) { - $this->dynamicRequirementMap[] = $service_id; - } + if (!isset($this->dynamicRequirementMap)) { + $this->dynamicRequirementMap = $this->container->getParameter('dynamic_access_check_services'); } } diff --git a/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterAccessChecksPass.php b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterAccessChecksPass.php index f636fb023f3..046001a282d 100644 --- a/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterAccessChecksPass.php +++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterAccessChecksPass.php @@ -2,6 +2,7 @@ namespace Drupal\Core\DependencyInjection\Compiler; +use Drupal\Core\Access\AccessCheckInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; @@ -17,6 +18,7 @@ class RegisterAccessChecksPass implements CompilerPassInterface { if (!$container->hasDefinition('access_manager')) { return; } + $dynamic_access_check_services = []; // Add services tagged 'access_check' to the access_manager service. $access_manager = $container->getDefinition('access_manager.check_provider'); foreach ($container->findTaggedServiceIds('access_check') as $id => $attributes) { @@ -35,7 +37,15 @@ class RegisterAccessChecksPass implements CompilerPassInterface { } } $access_manager->addMethodCall('addCheckService', [$id, $method, $applies, $needs_incoming_request]); + + // Collect dynamic access checker services. + $class = $container->getDefinition($id)->getClass(); + if (in_array(AccessCheckInterface::class, class_implements($class), TRUE)) { + $dynamic_access_check_services[] = $id; + } } + + $container->setParameter('dynamic_access_check_services', $dynamic_access_check_services); } } diff --git a/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php b/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php index 6aa46bb2b10..2ebcd370ccd 100644 --- a/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php +++ b/core/tests/Drupal/Tests/Core/Access/AccessManagerTest.php @@ -97,6 +97,7 @@ class AccessManagerTest extends UnitTestCase { $this->container = new ContainerBuilder(); $cache_contexts_manager = $this->prophesize(CacheContextsManager::class)->reveal(); $this->container->set('cache_contexts_manager', $cache_contexts_manager); + $this->container->setParameter('dynamic_access_check_services', []); \Drupal::setContainer($this->container); $this->routeCollection = new RouteCollection(); @@ -162,6 +163,7 @@ class AccessManagerTest extends UnitTestCase { // Setup the dynamic access checker. $access_check = $this->createMock('Drupal\Tests\Core\Access\TestAccessCheckInterface'); $this->container->set('test_access', $access_check); + $this->container->setParameter('dynamic_access_check_services', ['test_access']); $this->checkProvider->addCheckService('test_access', 'access'); $route = new Route('/test-path', [], ['_foo' => '1', '_bar' => '1']); @@ -398,6 +400,7 @@ class AccessManagerTest extends UnitTestCase { ->will($this->returnValue(AccessResult::forbidden())); $this->container->set('test_access', $access_check); + $this->container->setParameter('dynamic_access_check_services', ['test_access']); $this->checkProvider->addCheckService('test_access', 'access'); $this->checkProvider->setChecks($this->routeCollection); @@ -446,6 +449,7 @@ class AccessManagerTest extends UnitTestCase { ->will($this->returnValue(AccessResult::forbidden())); $this->container->set('test_access', $access_check); + $this->container->setParameter('dynamic_access_check_services', ['test_access']); $this->checkProvider->addCheckService('test_access', 'access'); $this->checkProvider->setChecks($this->routeCollection);