core/lib/Drupal/Core/Access/RouteProcessorCsrf.php | 23 ++++--- .../OutboundRouteProcessorInterface.php | 11 +++ .../Core/RouteProcessor/RouteProcessorManager.php | 18 +++++ .../src/Plugin/Menu/MenuLinkContent.php | 42 +++++++++++- .../src/Tests/MenuLinkContentTest.php | 79 ++++++++++++++++++++++ .../Tests/Core/Access/RouteProcessorCsrfTest.php | 14 ++-- .../RouteProcessor/RouteProcessorManagerTest.php | 4 ++ 7 files changed, 173 insertions(+), 18 deletions(-) diff --git a/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php b/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php index 183c2a2..f708a00 100644 --- a/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php +++ b/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php @@ -37,17 +37,22 @@ function __construct(CsrfTokenGenerator $csrf_token) { /** * {@inheritdoc} */ + public function applies(Route $route) { + return $route->hasRequirement('_csrf_token'); + } + + /** + * {@inheritdoc} + */ public function processOutbound(Route $route, array &$parameters) { - if ($route->hasRequirement('_csrf_token')) { - $path = ltrim($route->getPath(), '/'); - // Replace the path parameters with values from the parameters array. - foreach ($parameters as $param => $value) { - $path = str_replace("{{$param}}", $value, $path); - } - // Adding this to the parameters means it will get merged into the query - // string when the route is compiled. - $parameters['token'] = $this->csrfToken->get($path); + $path = ltrim($route->getPath(), '/'); + // Replace the path parameters with values from the parameters array. + foreach ($parameters as $param => $value) { + $path = str_replace("{{$param}}", $value, $path); } + // Adding this to the parameters means it will get merged into the query + // string when the route is compiled. + $parameters['token'] = $this->csrfToken->get($path); } } diff --git a/core/lib/Drupal/Core/RouteProcessor/OutboundRouteProcessorInterface.php b/core/lib/Drupal/Core/RouteProcessor/OutboundRouteProcessorInterface.php index c9bda24..767f2c8 100644 --- a/core/lib/Drupal/Core/RouteProcessor/OutboundRouteProcessorInterface.php +++ b/core/lib/Drupal/Core/RouteProcessor/OutboundRouteProcessorInterface.php @@ -15,6 +15,17 @@ interface OutboundRouteProcessorInterface { /** + * Indicates whether this outbound route processor applies to a route. + * + * @param \Symfony\Component\Routing\Route $route + * The outbound route to process. + * + * @return bool + * True if it applies, false otherwise. + */ + public function applies(Route $route); + + /** * Processes the outbound route. * * @param \Symfony\Component\Routing\Route $route diff --git a/core/lib/Drupal/Core/RouteProcessor/RouteProcessorManager.php b/core/lib/Drupal/Core/RouteProcessor/RouteProcessorManager.php index 3dc8864..4908b54 100644 --- a/core/lib/Drupal/Core/RouteProcessor/RouteProcessorManager.php +++ b/core/lib/Drupal/Core/RouteProcessor/RouteProcessorManager.php @@ -50,7 +50,25 @@ public function addOutbound(OutboundRouteProcessorInterface $processor, $priorit /** * {@inheritdoc} */ + public function applies(Route $route) { + $processors = $this->getOutbound(); + foreach ($processors as $processor) { + if ($processor->applies($route)) { + // Return as soon as at least one outbound route processor applies. + return TRUE; + } + } + return FALSE; + } + + /** + * {@inheritdoc} + */ public function processOutbound(Route $route, array &$parameters) { + if (!$this->applies($route)) { + return; + } + $processors = $this->getOutbound(); foreach ($processors as $processor) { $processor->processOutbound($route, $parameters); diff --git a/core/modules/menu_link_content/src/Plugin/Menu/MenuLinkContent.php b/core/modules/menu_link_content/src/Plugin/Menu/MenuLinkContent.php index ea47019..2e46e3f 100644 --- a/core/modules/menu_link_content/src/Plugin/Menu/MenuLinkContent.php +++ b/core/modules/menu_link_content/src/Plugin/Menu/MenuLinkContent.php @@ -13,6 +13,8 @@ use Drupal\Core\Language\LanguageManagerInterface; use Drupal\Core\Menu\MenuLinkBase; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; +use Drupal\Core\RouteProcessor\OutboundRouteProcessorInterface; +use Drupal\Core\Routing\RouteProviderInterface; use Symfony\Component\DependencyInjection\ContainerInterface; /** @@ -54,6 +56,20 @@ class MenuLinkContent extends MenuLinkBase implements ContainerFactoryPluginInte protected $entity; /** + * The route provider. + * + * @var \Drupal\Core\Routing\RouteProviderInterface + */ + protected $routeProvider; + + /** + * The route processor manager. + * + * @var \Drupal\Core\RouteProcessor\OutboundRouteProcessorInterface + */ + protected $routeProcessorManager; + + /** * The entity manager. * * @var \Drupal\Core\Entity\EntityManagerInterface @@ -76,13 +92,17 @@ class MenuLinkContent extends MenuLinkBase implements ContainerFactoryPluginInte * The plugin_id for the plugin instance. * @param mixed $plugin_definition * The plugin implementation definition. + * @param \Drupal\Core\Routing\RouteProviderInterface $route_provider + * The route provider. + * @param \Drupal\Core\RouteProcessor\OutboundRouteProcessorInterface $route_processor_manager + * The route processor manager. * @param \Drupal\Core\Entity\EntityManagerInterface $entity_manager * The entity manager. * @param \Drupal\Core\Language\LanguageManagerInterface $language_manager * The language manager. */ - public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityManagerInterface $entity_manager, LanguageManagerInterface $language_manager) { - parent::__construct($configuration, $plugin_id, $plugin_definition); + public function __construct(array $configuration, $plugin_id, $plugin_definition, RouteProviderInterface $route_provider, OutboundRouteProcessorInterface $route_processor_manager, EntityManagerInterface $entity_manager, LanguageManagerInterface $language_manager) { + parent::__construct($configuration, $plugin_id, $plugin_definition, $route_provider, $route_processor_manager); if (!empty($this->pluginDefinition['metadata']['entity_id'])) { $entity_id = $this->pluginDefinition['metadata']['entity_id']; @@ -91,6 +111,8 @@ public function __construct(array $configuration, $plugin_id, $plugin_definition static::$entityIdsToLoad[$entity_id] = $entity_id; } + $this->routeProvider = $route_provider; + $this->routeProcessorManager = $route_processor_manager; $this->entityManager = $entity_manager; $this->languageManager = $language_manager; } @@ -103,6 +125,8 @@ public static function create(ContainerInterface $container, array $configuratio $configuration, $plugin_id, $plugin_definition, + $container->get('router.route_provider'), + $container->get('route_processor_manager'), $container->get('entity.manager'), $container->get('language_manager') ); @@ -253,6 +277,20 @@ public function isTranslatable() { /** * {@inheritdoc} */ + public function isCacheable() { + if ($this->getUrlObject()->isExternal()) { + return TRUE; + } + else { + $route = $this->routeProvider->getRouteByName($this->getUrlObject()->getRouteName()); + // If no outbound route processor applies, then this link is cacheable. + return !$this->routeProcessorManager->applies($route); + } + } + + /** + * {@inheritdoc} + */ public function deleteLink() { $this->getEntity()->delete(); } diff --git a/core/modules/menu_link_content/src/Tests/MenuLinkContentTest.php b/core/modules/menu_link_content/src/Tests/MenuLinkContentTest.php new file mode 100644 index 0000000..03daeb9 --- /dev/null +++ b/core/modules/menu_link_content/src/Tests/MenuLinkContentTest.php @@ -0,0 +1,79 @@ +installEntitySchema('menu_link_content'); + $this->installSchema('system', ['router']); + + \Drupal::service('router.builder')->rebuild(); + + Menu::create([ + 'id' => 'menu_test', + 'label' => 'Test menu', + 'description' => 'Description text', + ])->save(); + + $this->menuLinkManager = \Drupal::service('plugin.manager.menu.link'); + } + + /** + * Tests ::isCacheable(). + */ + public function testIsCacheable() { + $link1 = MenuLinkContent::create([ + 'route_name' => 'system.admin', + 'route_parameters' => [], + 'title' => $this->randomMachineName(16), + 'menu_name' => 'menu_test', + 'bundle' => 'menu_link_content', + ]); + $link1->save(); + $this->assertTrue($this->menuLinkManager->createInstance($link1->getPluginId())->isCacheable(), 'A menu link to a route without a CSRF token is cacheable.'); + + $link2 = MenuLinkContent::create([ + 'route_name' => 'system.theme_enable', + 'route_parameters' => [], + 'title' => $this->randomMachineName(16), + 'menu_name' => 'menu_test', + 'bundle' => 'menu_link_content', + ]); + $link2->save(); + $this->assertFalse($this->menuLinkManager->createInstance($link2->getPluginId())->isCacheable(), 'A menu link to a route with a CSRF token is not cacheable.'); + } + +} diff --git a/core/tests/Drupal/Tests/Core/Access/RouteProcessorCsrfTest.php b/core/tests/Drupal/Tests/Core/Access/RouteProcessorCsrfTest.php index 871ffb0..2b18da8 100644 --- a/core/tests/Drupal/Tests/Core/Access/RouteProcessorCsrfTest.php +++ b/core/tests/Drupal/Tests/Core/Access/RouteProcessorCsrfTest.php @@ -14,6 +14,7 @@ /** * @coversDefaultClass \Drupal\Core\Access\RouteProcessorCsrf * @group Access + * @group RouteProcessor */ class RouteProcessorCsrfTest extends UnitTestCase { @@ -40,18 +41,14 @@ protected function setUp() { } /** - * Tests the processOutbound() method with no _csrf_token route requirement. - */ + * Tests the processOutbound() method with no _csrf_token route requirement. + */ public function testProcessOutboundNoRequirement() { $this->csrfToken->expects($this->never()) ->method('get'); $route = new Route('/test-path'); - $parameters = array(); - - $this->processor->processOutbound($route, $parameters); - // No parameters should be added to the parameters array. - $this->assertEmpty($parameters); + $this->assertFalse($this->processor->applies($route)); } /** @@ -67,6 +64,7 @@ public function testProcessOutbound() { $route = new Route('/test-path', array(), array('_csrf_token' => 'TRUE')); $parameters = array(); + $this->assertTrue($this->processor->applies($route)); $this->processor->processOutbound($route, $parameters); // 'token' should be added to the parameters array. $this->assertArrayHasKey('token', $parameters); @@ -85,6 +83,7 @@ public function testProcessOutboundDynamicOne() { $route = new Route('/test-path/{slug}', array(), array('_csrf_token' => 'TRUE')); $parameters = array('slug' => 100); + $this->assertTrue($this->processor->applies($route)); $this->assertNull($this->processor->processOutbound($route, $parameters)); } @@ -100,6 +99,7 @@ public function testProcessOutboundDynamicTwo() { $route = new Route('{slug_1}/test-path/{slug_2}', array(), array('_csrf_token' => 'TRUE')); $parameters = array('slug_1' => 100, 'slug_2' => 'test'); + $this->assertTrue($this->processor->applies($route)); $this->assertNull($this->processor->processOutbound($route, $parameters)); } diff --git a/core/tests/Drupal/Tests/Core/RouteProcessor/RouteProcessorManagerTest.php b/core/tests/Drupal/Tests/Core/RouteProcessor/RouteProcessorManagerTest.php index eacc6d8..2875f68 100644 --- a/core/tests/Drupal/Tests/Core/RouteProcessor/RouteProcessorManagerTest.php +++ b/core/tests/Drupal/Tests/Core/RouteProcessor/RouteProcessorManagerTest.php @@ -61,6 +61,10 @@ public function testRouteProcessorManager() { */ protected function getMockProcessor($route, $parameters) { $processor = $this->getMock('Drupal\Core\RouteProcessor\OutboundRouteProcessorInterface'); + $processor->expects($this->any()) + ->method('applies') + ->with($route) + ->will($this->returnValue(TRUE)); $processor->expects($this->once()) ->method('processOutbound') ->with($route, $parameters);