diff --git a/core/includes/menu.inc b/core/includes/menu.inc index 006730e51ecfdccff8d4ef972c5ed025ce76e1b7..d7195711f35a16090e677e1abf7235902e210486 100644 --- a/core/includes/menu.inc +++ b/core/includes/menu.inc @@ -5,6 +5,8 @@ * API for the Drupal menu system. */ +use Symfony\Component\HttpFoundation\Request; + use Drupal\Component\Utility\NestedArray; use Drupal\Core\Cache\CacheBackendInterface; use Drupal\Core\Template\Attribute; @@ -896,7 +898,11 @@ function _menu_link_translate(&$item, $translate = FALSE) { } // menu_tree_check_access() may set this ahead of time for links to nodes. if (!isset($item['access'])) { - if (!empty($item['load_functions']) && !_menu_load_objects($item, $map)) { + if ($route = $item->getRoute()) { + $request = Request::create('/' . $item['path']); + $item['access'] = drupal_container()->get('access_manager')->check($route, $request); + } + elseif (!empty($item['load_functions']) && !_menu_load_objects($item, $map)) { // An error occurred loading an object. $item['access'] = FALSE; return FALSE; @@ -2642,6 +2648,17 @@ function menu_router_build() { $router_items = call_user_func($module . '_menu'); if (isset($router_items) && is_array($router_items)) { foreach (array_keys($router_items) as $path) { + // If the menu item references a route, normalize the route information + // into the old structure. Note that routes are keyed by name, not path, + // so the path of the route takes precedence. + if (isset($router_items[$path]['route_name'])) { + $router_item = $router_items[$path]; + $router_item = _menu_router_merge_route($router_item, $path); + $new_path = $router_item['path']; + unset($router_items[$path]); + $router_items[$new_path] = $router_item; + $path = $new_path; + } $router_items[$path]['module'] = $module; } $callbacks = array_merge($callbacks, $router_items); @@ -2655,6 +2672,33 @@ function menu_router_build() { return array($menu, $masks); } +/** + * Merges a legacy menu router item with its referenced route. + * + * @param array $router_item + * The router item to modify. + * @param string $path + * The path this router item is for. + * + * @return array + * The modified router item, including the path pattern from the route in the + * 'path' key. + */ +function _menu_router_merge_route(array $router_item, $path) { + $router_item['path'] = $path; + + $route_provider = drupal_container()->get('router.route_provider'); + + $route = $route_provider->getRouteByName($router_item['route_name']); + $router_item['path'] = trim($route->getPattern(), '/'); + + $router_item['page callback'] = 'USES_ROUTE'; + $router_item['access callback'] = TRUE; + + + return $router_item; +} + /** * Stores the menu router if we have it in memory. */ diff --git a/core/includes/path.inc b/core/includes/path.inc index 6e2c19e2dc9ff4e251522b756999d1a1f6a8a291..182c0c84a2d4bb50a7429d91a46c638f83e59e94 100644 --- a/core/includes/path.inc +++ b/core/includes/path.inc @@ -9,6 +9,8 @@ * "drupal_bootstrap(DRUPAL_BOOTSTRAP_FULL);". */ +use Symfony\Component\HttpFoundation\Request; + /** * Check if the current page is the front page. * @@ -209,11 +211,44 @@ function drupal_valid_path($path, $dynamic_allowed = FALSE) { $item['options'] = ''; _menu_link_translate($item); } + if (empty($item['access'])) { + // Nothing was found in the old routing system, so try the new one. + $item = _drupal_valid_path_new_router($path); + } } else { $item = menu_get_item($path); + if (empty($item['access'])) { + // Nothing was found in the old routing system, so try the new one. + $item = _drupal_valid_path_new_router($path); + } } $menu_admin = FALSE; return $item && $item['access']; } +/** + * Temporary helper function to check a path in the new routing system. + * + * @param string $path + * The path string as expected by drupal_valid_path(). + * + * @return array|NULL + * An array containing 'access' => TRUE or NULL for paths that were not found + * or the user has no access to. + */ +function _drupal_valid_path_new_router($path) { + $request = Request::create('/' . $path); + $request->attributes->set('system_path', $path); + try { + $dc = drupal_container(); + $route = $dc->get('router.dynamic')->matchRequest($request); + if (!empty($route)) { + $dc->get('access_manager')->check($route['_route_object'], $request); + } + return array('access' => TRUE); + } + catch (Exception $e) { + drupal_set_message($e->getMessage(), 'menu', WATCHDOG_ERROR); + } +} diff --git a/core/lib/Drupal/Core/Access/AccessManager.php b/core/lib/Drupal/Core/Access/AccessManager.php index 533b1312f4a700a9bd29d567d41e415d3e1237a1..94dba60e3652e149a6c151643e4fd4dff47aa1cb 100644 --- a/core/lib/Drupal/Core/Access/AccessManager.php +++ b/core/lib/Drupal/Core/Access/AccessManager.php @@ -31,23 +31,6 @@ class AccessManager extends ContainerAware { */ protected $checks; - /** - * The request object. - * - * @var \Symfony\Component\HttpFoundation\Request - */ - protected $request; - - /** - * Constructs a new AccessManager. - * - * @param \Symfony\Component\HttpFoundation\Request $request - * The request object. - */ - public function __construct(Request $request) { - $this->request = $request; - } - /** * Registers a new AccessCheck by service ID. * @@ -106,11 +89,13 @@ protected function applies(Route $route) { * * @param \Symfony\Component\Routing\Route $route * The route to check access to. + * @param \Symfony\Commponent\HttpFoundation\Request $request + * The incoming request object. * * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException * If any access check denies access or none explicitly approve. */ - public function check(Route $route) { + public function check(Route $route, Request $request) { $access = FALSE; $checks = $route->getOption('_access_checks') ?: array(); @@ -120,7 +105,7 @@ public function check(Route $route) { $this->loadCheck($service_id); } - $service_access = $this->checks[$service_id]->access($route, $this->request); + $service_access = $this->checks[$service_id]->access($route, $request); if ($service_access === FALSE) { // A check has denied access, no need to continue checking. $access = FALSE; @@ -132,10 +117,7 @@ public function check(Route $route) { } } - // Access has been denied or not explicily approved. - if (!$access) { - throw new AccessDeniedHttpException(); - } + return $access; } /** diff --git a/core/lib/Drupal/Core/CoreBundle.php b/core/lib/Drupal/Core/CoreBundle.php index 88b25bf26903555aa3b1fd16484d171a0292ee94..ef8a1e7b54c8ea6f808ea62a48f40693d05ff624 100644 --- a/core/lib/Drupal/Core/CoreBundle.php +++ b/core/lib/Drupal/Core/CoreBundle.php @@ -256,7 +256,6 @@ public function build(ContainerBuilder $container) { $container->register('legacy_access_subscriber', 'Drupal\Core\EventSubscriber\LegacyAccessSubscriber') ->addTag('event_subscriber'); $container->register('access_manager', 'Drupal\Core\Access\AccessManager') - ->addArgument(new Reference('request')) ->addMethodCall('setContainer', array(new Reference('service_container'))); $container->register('access_subscriber', 'Drupal\Core\EventSubscriber\AccessSubscriber') ->addArgument(new Reference('access_manager')) diff --git a/core/lib/Drupal/Core/EventSubscriber/AccessSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/AccessSubscriber.php index 420780a3c058b34c0ffce60cc894132b60755cb0..6f18839269d11d860a5f3c6a644ee1485a9825a3 100644 --- a/core/lib/Drupal/Core/EventSubscriber/AccessSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/AccessSubscriber.php @@ -11,6 +11,7 @@ use Symfony\Component\HttpKernel\KernelEvents; use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\EventDispatcher\EventSubscriberInterface; +use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Drupal\Core\Routing\RoutingEvents; use Drupal\Core\Access\AccessManager; use Drupal\Core\Routing\RouteBuildEvent; @@ -45,7 +46,10 @@ public function onKernelRequestAccessCheck(GetResponseEvent $event) { return; } - $this->accessManager->check($request->attributes->get(RouteObjectInterface::ROUTE_OBJECT)); + $access = $this->accessManager->check($request->attributes->get(RouteObjectInterface::ROUTE_OBJECT), $request); + if (!$access) { + throw new AccessDeniedHttpException(); + } } /** diff --git a/core/lib/Drupal/Core/LegacyUrlMatcher.php b/core/lib/Drupal/Core/LegacyUrlMatcher.php index 7098b412ee6685be0aa821ce83fa7f84cd98b5b9..80d6ae80b6079a0ee59a965fc9a59cbe3b6e2dc4 100644 --- a/core/lib/Drupal/Core/LegacyUrlMatcher.php +++ b/core/lib/Drupal/Core/LegacyUrlMatcher.php @@ -158,7 +158,7 @@ protected function convertDrupalItem($router_item) { // A few menu items have a fake page callback temporarily. Skip those, // we aren't going to route them. - if ($router_item['page_callback'] == 'NOT_USED') { + if ($router_item['page_callback'] == 'USES_ROUTE') { throw new ResourceNotFoundException(); } diff --git a/core/lib/Drupal/Core/Routing/RouteProvider.php b/core/lib/Drupal/Core/Routing/RouteProvider.php index 1927c1c31e75809ca34e7b8fd262788c525c52d0..8892866f35b71d6769bbbf4bfbd9cd67f01e60e3 100644 --- a/core/lib/Drupal/Core/Routing/RouteProvider.php +++ b/core/lib/Drupal/Core/Routing/RouteProvider.php @@ -166,14 +166,20 @@ public function getRouteByName($name, $parameters = array()) { */ public function getRoutesByNames($names, $parameters = array()) { + if (empty($names)) { + throw new \InvalidArgumentException('You must specify the route names to load'); + } + $routes_to_load = array_diff($names, array_keys($this->routes)); - $result = $this->connection->query('SELECT name, route FROM {' . $this->connection->escapeTable($this->tableName) . '} WHERE name IN (:names)', array(':names' => $routes_to_load)); - $routes = $result->fetchAllKeyed(); + if ($routes_to_load) { + $result = $this->connection->query('SELECT name, route FROM {' . $this->connection->escapeTable($this->tableName) . '} WHERE name IN (:names)', array(':names' => $routes_to_load)); + $routes = $result->fetchAllKeyed(); - $return = array(); - foreach ($routes as $name => $route) { - $this->routes[$name] = unserialize($route); + $return = array(); + foreach ($routes as $name => $route) { + $this->routes[$name] = unserialize($route); + } } return array_intersect_key($this->routes, array_flip($names)); diff --git a/core/modules/book/book.module b/core/modules/book/book.module index 81b0c9948bd5fe742606cd94b090c01f8d035321..c316278586d604c0dcf03d2f6712d1a6ddd0c169 100644 --- a/core/modules/book/book.module +++ b/core/modules/book/book.module @@ -1025,7 +1025,7 @@ function template_preprocess_book_navigation(&$variables) { } if ($book_link['plid'] && $parent = book_link_load($book_link['plid'])) { - $parent_href = url($parent['href']); + $parent_href = url($parent['link_path']); drupal_add_html_head_link(array('rel' => 'up', 'href' => $parent_href)); $variables['parent_url'] = $parent_href; $variables['parent_title'] = check_plain($parent['title']); @@ -1279,14 +1279,7 @@ function book_node_type_update($type) { * {book} table. FALSE if there is an error. */ function book_link_load($mlid) { - if ($item = db_query("SELECT * FROM {menu_links} ml INNER JOIN {book} b ON b.mlid = ml.mlid LEFT JOIN {menu_router} m ON m.path = ml.router_path WHERE ml.mlid = :mlid", array( - ':mlid' => $mlid, - ))->fetchAssoc()) { - _menu_link_translate($item); - return $item; - } - - return FALSE; + return entity_load('menu_link', $mlid); } /** diff --git a/core/modules/menu/menu.module b/core/modules/menu/menu.module index c300ed61c0c7ce1a5c77e52d4114f5f16f78a3c5..46d6ead3160389eb1469974300b3887e7af0ad68 100644 --- a/core/modules/menu/menu.module +++ b/core/modules/menu/menu.module @@ -194,6 +194,7 @@ function menu_theme() { * Add a link for each custom menu. */ function menu_enable() { + drupal_container()->get('router.builder')->rebuild(); menu_router_rebuild(); $system_link = entity_load_multiple_by_properties('menu_link', array('link_path' => 'admin/structure/menu', 'module' => 'system')); $system_link = reset($system_link); diff --git a/core/modules/menu_link/lib/Drupal/menu_link/MenuLinkStorageController.php b/core/modules/menu_link/lib/Drupal/menu_link/MenuLinkStorageController.php index 865b8ed4ad646e1c5482cd9d4e8be64d5718685a..5180b3ee4443c38ae03f5c426a4dae06efcff76f 100644 --- a/core/modules/menu_link/lib/Drupal/menu_link/MenuLinkStorageController.php +++ b/core/modules/menu_link/lib/Drupal/menu_link/MenuLinkStorageController.php @@ -62,11 +62,27 @@ protected function buildQuery($ids, $revision_id = FALSE) { * control the entity load hooks. */ protected function attachLoad(&$menu_links, $load_revision = FALSE) { + $routes = array(); + foreach ($menu_links as &$menu_link) { $menu_link->options = unserialize($menu_link->options); // Use the weight property from the menu link. $menu_link->router_item['weight'] = $menu_link->weight; + + // For all links that have an associated route, load the route object now + // and save it on the object. That way we avoid a select N+1 problem later. + if ($menu_link->route_name) { + $routes[$menu_link->id()] = $menu_link->route_name; + } + } + + // Now mass-load any routes needed and associate them. + if ($routes) { + $route_objects = drupal_container()->get('router.route_provider')->getRoutesByNames($routes); + foreach ($routes as $entity_id => $route) { + $menu_links[$entity_id]->setRouteObject($route_objects[$route]); + } } parent::attachLoad($menu_links, $load_revision); diff --git a/core/modules/menu_link/lib/Drupal/menu_link/Plugin/Core/Entity/MenuLink.php b/core/modules/menu_link/lib/Drupal/menu_link/Plugin/Core/Entity/MenuLink.php index 110a1e39acc2ce864cea0cd5cc71c9d13b2490bc..ce3911d1301b00f8eedbb7f4eaf69a276919ad0d 100644 --- a/core/modules/menu_link/lib/Drupal/menu_link/Plugin/Core/Entity/MenuLink.php +++ b/core/modules/menu_link/lib/Drupal/menu_link/Plugin/Core/Entity/MenuLink.php @@ -7,6 +7,8 @@ namespace Drupal\menu_link\Plugin\Core\Entity; +use Symfony\Component\Routing\Route; + use Drupal\Component\Annotation\Plugin; use Drupal\Core\Annotation\Translation; use Drupal\Core\Entity\ContentEntityInterface; @@ -232,6 +234,20 @@ class MenuLink extends Entity implements \ArrayAccess, ContentEntityInterface { */ public $updated = 0; + /** + * The name of the route associated with this menu link, if any. + * + * @var string + */ + public $route_name; + + /** + * The route object associated with this menu link, if any. + * + * @var \Symfony\Component\Routing\Route + */ + protected $routeObject; + /** * Overrides Entity::id(). */ @@ -248,6 +264,36 @@ public function createDuplicate() { return $duplicate; } + /** + * Returns the Route object associated with this link, if any. + * + * @return \Symfony\Component\Routing\Route|null + * The route object for this menu link, or NULL if there isn't one. + */ + public function getRoute() { + if (!$this->route_name) { + return NULL; + } + if (!($this->routeObject instanceof Route)) { + $route_provider = drupal_container()->get('router.route_provider'); + $this->routeObject = $route_provider->getRouteByName($this->route_name); + } + return $this->routeObject; + } + + /** + * Sets the route object for this link. + * + * This should only be called by MenuLinkStorageController when loading + * the link object. Calling it at other times could result in unpredictable + * behavior. + * + * @param \Symfony\Component\Routing\Route $route + */ + public function setRouteObject(Route $route) { + $this->routeObject = $route; + } + /** * Resets a system-defined menu link. * diff --git a/core/modules/menu_link/menu_link.install b/core/modules/menu_link/menu_link.install index b8a471d2785e49eebffbc2f97e25ef574e803ca7..1d806794d7b5a8f40bdedaa3778ed63510d16141 100644 --- a/core/modules/menu_link/menu_link.install +++ b/core/modules/menu_link/menu_link.install @@ -197,6 +197,11 @@ function menu_link_schema() { 'default' => 0, 'size' => 'small', ), + 'route_name' => array( + 'description' => 'The machine name of a defined Symfony Route this menu item represents.', + 'type' => 'varchar', + 'length' => 255, + ), ), 'indexes' => array( 'path_menu' => array(array('link_path', 128), 'menu_name'), diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/RouterPermissionTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/RouterPermissionTest.php index 5cb4e15e6c0b3542ed507dbbb65bd23828225a51..30591648154a00095037140805e0317bade9fb49 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/RouterPermissionTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/RouterPermissionTest.php @@ -33,7 +33,6 @@ public static function getInfo() { * Tests permission requirements on routes. */ public function testPermissionAccess() { - $this->drupalGet('router_test/test7'); $this->assertResponse(403, "Access denied for a route where we don't have a permission"); diff --git a/core/modules/system/system.install b/core/modules/system/system.install index 58dc47936484472317388f4333666d0cb7e6c5ec..882a049f7de52acad72eabccaa6d9ca6f1a6f3b0 100644 --- a/core/modules/system/system.install +++ b/core/modules/system/system.install @@ -2181,6 +2181,19 @@ function system_update_8050() { )); } +/** + * Adds route_name column to the menu_links table. + */ +function system_update_8051() { + $spec = array( + 'description' => 'The machine name of a defined Symfony Route this menu item represents.', + 'type' => 'varchar', + 'length' => 255, + ); + + db_add_field('menu_links', '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/user/user.module b/core/modules/user/user.module index ab5af81cc3223a368f23851e9f2d7ff5ce535ada..bdf5862739c85b3c68bcfa6649fb305cb9df1507 100644 --- a/core/modules/user/user.module +++ b/core/modules/user/user.module @@ -875,29 +875,6 @@ function user_register_access() { * Implements hook_menu(). */ function user_menu() { - // @todo Remove once drupal_valid_path() is fixed to find and access check - // paths managed by the new routing system: http://drupal.org/node/1793520. - $items['user/autocomplete'] = array( - 'title' => 'User autocomplete', - // _menu_router_build() denies access to paths without a page callback. - 'page callback' => 'NOT_USED', - 'access callback' => 'user_access', - 'access arguments' => array('access user profiles'), - 'type' => MENU_CALLBACK, - 'file' => 'user.pages.inc', - ); - - $items['user/autocomplete/anonymous'] = array( - 'title' => 'User autocomplete including anonymous', - // _menu_router_build() denies access to paths without a page callback. - 'page callback' => 'NOT_USED', - 'page arguments' => array(TRUE), - 'access callback' => 'user_access', - 'access arguments' => array('access user profiles'), - 'type' => MENU_CALLBACK, - 'file' => 'user.pages.inc', - ); - // Registration and login pages. $items['user'] = array( 'title' => 'User account', @@ -923,13 +900,7 @@ function user_menu() { $items['user/register'] = array( 'title' => 'Create new account', 'type' => MENU_LOCAL_TASK, - // @todo This route is now declared in user.routing.yml, so the below are - // only needed for drupal_valid_path(). Without a non-empty (but not - // necessarily valid) page callback, _menu_router_build() overrides the - // access callback to 0. Remove once drupal_valid_path works with the new - // routing system: http://drupal.org/node/1793520. - 'page callback' => 'NOT_USED', - 'access callback' => 'user_register_access', + 'route_name' => 'user_register', ); $items['user/password'] = array(