From 594c849d8d7f404df835e213476085e4ea0eea61 Mon Sep 17 00:00:00 2001
From: webchick <webchick@24967.no-reply.drupal.org>
Date: Wed, 1 May 2013 21:09:48 -0700
Subject: [PATCH] Issue #1938960 by tim.plunkett, dawehner: Fixed
_menu_translate() doesn't care about new routes.
---
core/includes/menu.inc | 16 +++-
.../Drupal/Core/Access/DefaultAccessCheck.php | 2 +-
.../system/Tests/Menu/MenuTranslateTest.php | 53 +++++++++++++
.../system/Tests/Menu/TreeAccessTest.php | 4 +-
core/modules/system/system.install | 18 +++++
.../tests/modules/menu_test/menu_test.module | 5 ++
.../modules/menu_test/menu_test.routing.yml | 8 +-
.../Core/Access/DefaultAccessCheckTest.php | 75 +++++++++++++++++++
8 files changed, 176 insertions(+), 5 deletions(-)
create mode 100644 core/modules/system/lib/Drupal/system/Tests/Menu/MenuTranslateTest.php
create mode 100644 core/tests/Drupal/Tests/Core/Access/DefaultAccessCheckTest.php
diff --git a/core/includes/menu.inc b/core/includes/menu.inc
index 6b62bf878de4..7065218a007e 100644
--- a/core/includes/menu.inc
+++ b/core/includes/menu.inc
@@ -766,7 +766,18 @@ function _menu_translate(&$router_item, $map, $to_arg = FALSE) {
$router_item['tab_root_href'] = implode('/', $tab_root_map);
$router_item['tab_parent_href'] = implode('/', $tab_parent_map);
$router_item['options'] = array();
- _menu_check_access($router_item, $map);
+ if (!empty($router_item['route_name'])) {
+ $route_provider = Drupal::getContainer()->get('router.route_provider');
+ $route = $route_provider->getRouteByName($router_item['route_name']);
+ $request = Request::create('/' . $router_item['href']);
+ $request->attributes->set('system_path', $router_item['href']);
+ $request->attributes->add(Drupal::service('router')->matchRequest($request));
+ $router_item['access'] = Drupal::service('access_manager')->check($route, $request);
+ }
+ else {
+ // @todo: Remove once all routes are converted.
+ _menu_check_access($router_item, $map);
+ }
// For performance, don't localize an item the user can't access.
if ($router_item['access']) {
@@ -3166,6 +3177,7 @@ function _menu_router_build($callbacks, $save = FALSE) {
'file' => '',
'file path' => '',
'include file' => '',
+ 'route_name' => '',
);
// Calculate out the file to be included for each callback, if any.
@@ -3227,6 +3239,7 @@ function _menu_router_save($menu, $masks) {
'position',
'weight',
'include_file',
+ 'route_name',
));
$num_records = 0;
@@ -3258,6 +3271,7 @@ function _menu_router_save($menu, $masks) {
'position' => $item['position'],
'weight' => $item['weight'],
'include_file' => $item['include file'],
+ 'route_name' => $item['route_name'],
));
// Execute in batches to avoid the memory overhead of all of those records
diff --git a/core/lib/Drupal/Core/Access/DefaultAccessCheck.php b/core/lib/Drupal/Core/Access/DefaultAccessCheck.php
index 31c6da389625..074dfd0baa01 100644
--- a/core/lib/Drupal/Core/Access/DefaultAccessCheck.php
+++ b/core/lib/Drupal/Core/Access/DefaultAccessCheck.php
@@ -26,6 +26,6 @@ public function applies(Route $route) {
* Implements AccessCheckInterface::access().
*/
public function access(Route $route, Request $request) {
- return (bool) $route->getRequirement('_access');
+ return $route->getRequirement('_access') === 'TRUE';
}
}
diff --git a/core/modules/system/lib/Drupal/system/Tests/Menu/MenuTranslateTest.php b/core/modules/system/lib/Drupal/system/Tests/Menu/MenuTranslateTest.php
new file mode 100644
index 000000000000..33aa51ada658
--- /dev/null
+++ b/core/modules/system/lib/Drupal/system/Tests/Menu/MenuTranslateTest.php
@@ -0,0 +1,53 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\system\Tests\Menu\MenuTranslateTest.
+ */
+
+namespace Drupal\system\Tests\Menu;
+
+use Drupal\simpletest\WebTestBase;
+
+/**
+ * Defines a test class which tests the _menu_translate method.
+ *
+ * @see _menu_translate().
+ */
+class MenuTranslateTest extends WebTestBase {
+
+ /**
+ * Modules to enable.
+ *
+ * @var array
+ */
+ public static $modules = array('menu_test');
+
+ public static function getInfo() {
+ return array(
+ 'name' => 'menu_translate',
+ 'description' => 'Tests the _menu_translate() method.',
+ 'group' => 'Menu',
+ );
+ }
+
+ /**
+ * Tests _menu_translate().
+ */
+ public function testMenuTranslate() {
+ // Check for access to a restricted local task from a default local task.
+ $this->drupalGet('foo/asdf');
+ $this->assertResponse(200);
+ $this->assertLinkByHref('foo/asdf');
+ $this->assertLinkByHref('foo/asdf/b');
+ $this->assertNoLinkByHref('foo/asdf/c');
+
+ // Attempt to access a restricted local task.
+ $this->drupalGet('foo/asdf/c');
+ $this->assertResponse(403);
+ $this->assertNoLinkByHref('foo/asdf');
+ $this->assertNoLinkByHref('foo/asdf/b');
+ $this->assertNoLinkByHref('foo/asdf/c');
+ }
+
+}
diff --git a/core/modules/system/lib/Drupal/system/Tests/Menu/TreeAccessTest.php b/core/modules/system/lib/Drupal/system/Tests/Menu/TreeAccessTest.php
index 91d1a39bb943..4dafee64d21b 100644
--- a/core/modules/system/lib/Drupal/system/Tests/Menu/TreeAccessTest.php
+++ b/core/modules/system/lib/Drupal/system/Tests/Menu/TreeAccessTest.php
@@ -67,7 +67,7 @@ protected function getTestRouteCollection() {
'_controller' => '\Drupal\menu_test\TestController::test'
),
array(
- '_access' => '1'
+ '_access' => 'TRUE'
)
));
$route_collection->add('menu_test_2', new Route('/menu_test/test_2',
@@ -75,7 +75,7 @@ protected function getTestRouteCollection() {
'_controller' => '\Drupal\menu_test\TestController::test'
),
array(
- '_access' => '0'
+ '_access' => 'FALSE'
)
));
$this->routeCollection = $route_collection;
diff --git a/core/modules/system/system.install b/core/modules/system/system.install
index e23bf8e0d6ed..ccde9147bb1e 100644
--- a/core/modules/system/system.install
+++ b/core/modules/system/system.install
@@ -858,6 +858,11 @@ function system_schema() {
'type' => 'text',
'size' => 'medium',
),
+ 'route_name' => array(
+ 'description' => 'The machine name of a defined Symfony Route this menu item represents.',
+ 'type' => 'varchar',
+ 'length' => 255,
+ ),
),
'indexes' => array(
'fit' => array('fit'),
@@ -2082,6 +2087,19 @@ function system_update_8053() {
db_drop_table('cache_form');
}
+/**
+ * Adds route_name column to the menu_router table.
+ */
+function system_update_8054() {
+ $spec = array(
+ 'description' => 'The machine name of a defined Symfony Route this menu item represents.',
+ 'type' => 'varchar',
+ 'length' => 255,
+ );
+
+ db_add_field('menu_router', 'route_name', $spec);
+}
+
/**
* @} End of "defgroup updates-7.x-to-8.x".
* The next series of updates should start at 9000.
diff --git a/core/modules/system/tests/modules/menu_test/menu_test.module b/core/modules/system/tests/modules/menu_test/menu_test.module
index 8a1aac48cf6a..695c2160bfba 100644
--- a/core/modules/system/tests/modules/menu_test/menu_test.module
+++ b/core/modules/system/tests/modules/menu_test/menu_test.module
@@ -407,6 +407,11 @@ function menu_test_menu() {
// Controller-based local task.
$items['foo/%/b'] = array(
'title' => 'Local task B',
+ 'route_name' => 'menu_router_test2',
+ 'type' => MENU_LOCAL_TASK,
+ );
+ $items['foo/%/c'] = array(
+ 'title' => 'Local task C',
'route_name' => 'menu_router_test3',
'type' => MENU_LOCAL_TASK,
);
diff --git a/core/modules/system/tests/modules/menu_test/menu_test.routing.yml b/core/modules/system/tests/modules/menu_test/menu_test.routing.yml
index 9ed262c6270f..12c5ad1ec6ca 100644
--- a/core/modules/system/tests/modules/menu_test/menu_test.routing.yml
+++ b/core/modules/system/tests/modules/menu_test/menu_test.routing.yml
@@ -4,9 +4,15 @@ menu_router_test1:
_content: '\Drupal\menu_test\TestControllers::test1'
requirements:
_access: 'TRUE'
-menu_router_test3:
+menu_router_test2:
pattern: '/foo/{bar}/b'
defaults:
_content: '\Drupal\menu_test\TestControllers::test2'
requirements:
_access: 'TRUE'
+menu_router_test3:
+ pattern: '/foo/{bar}/c'
+ defaults:
+ _content: '\Drupal\menu_test\TestControllers::test2'
+ requirements:
+ _access: 'FALSE'
diff --git a/core/tests/Drupal/Tests/Core/Access/DefaultAccessCheckTest.php b/core/tests/Drupal/Tests/Core/Access/DefaultAccessCheckTest.php
new file mode 100644
index 000000000000..d96825c004cf
--- /dev/null
+++ b/core/tests/Drupal/Tests/Core/Access/DefaultAccessCheckTest.php
@@ -0,0 +1,75 @@
+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Tests\Core\Access\DefaultAccessCheckTest.
+ */
+
+namespace Drupal\Tests\Core\Access;
+
+use Drupal\Core\Access\DefaultAccessCheck;
+use Drupal\Tests\UnitTestCase;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\Routing\Route;
+
+/**
+ * Defines a test to check the default access checker.
+ *
+ * @see \Drupal\Core\Access\DefaultAccessCheck
+ */
+class DefaultAccessCheckTest extends UnitTestCase {
+
+ /**
+ * The access checker to test.
+ *
+ * @var \Drupal\Core\Access\DefaultAccessCheck
+ */
+ protected $accessChecker;
+
+ public static function getInfo() {
+ return array(
+ 'name' => 'DefaultAccessCheck access checker',
+ 'description' => 'Tests the DefaultAccessCheck class.',
+ 'group' => 'Routing',
+ );
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function setUp() {
+ parent::setUp();
+
+ $this->accessChecker = new DefaultAccessCheck();
+ }
+
+
+ /**
+ * Test the applies method.
+ */
+ public function testApplies() {
+ $route = new Route('/test-route');
+ $this->assertFalse($this->accessChecker->applies($route), 'Access checker applied even no _access was defined as requirement.');
+
+ $route->addRequirements(array('_access' => 'FALSE'));
+ $this->assertTrue($this->accessChecker->applies($route), 'Access checker applied even a _access was defined as requirement.');
+
+ $route->addRequirements(array('_access' => 'TRUE'));
+ $this->assertTrue($this->accessChecker->applies($route), 'Access checker applied even a _access was defined as requirement.');
+ }
+
+ /**
+ * Test the access method.
+ */
+ public function testAccess() {
+ $route = new Route('/test-route');
+ $request = new Request(array());
+
+ $route->addRequirements(array('_access' => 'FALSE'));
+ $this->assertFalse($this->accessChecker->access($route, $request));
+
+ $route->addRequirements(array('_access' => 'TRUE'));
+ $this->assertTrue($this->accessChecker->access($route, $request));
+ }
+
+}
--
GitLab