diff --git a/core/includes/menu.inc b/core/includes/menu.inc index ef9f7e2..269ce8d 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); + } + else if (!empty($item['load_functions']) && !_menu_load_objects($item, $map)) { // An error occurred loading an object. $item['access'] = FALSE; return FALSE; @@ -2634,6 +2640,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'])) { + $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); @@ -2648,6 +2665,32 @@ function menu_router_build() { } /** + * 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']); + $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. */ function _menu_router_cache($new_menu = NULL) { @@ -3102,6 +3145,7 @@ function _menu_router_build($callbacks) { 'file' => '', 'file path' => '', 'include file' => '', + 'route' => '', ); // Calculate out the file to be included for each callback, if any. @@ -3152,6 +3196,7 @@ function _menu_router_save($menu, $masks) { 'position', 'weight', 'include_file', + 'route', )); $num_records = 0; @@ -3183,6 +3228,7 @@ function _menu_router_save($menu, $masks) { 'position' => $item['position'], 'weight' => $item['weight'], 'include_file' => $item['include file'], + 'route' => $item['route'], )); // Execute in batches to avoid the memory overhead of all of those records diff --git a/core/includes/path.inc b/core/includes/path.inc index 6e2c19e..02134c5 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']); + } + 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 533b131..94dba60 100644 --- a/core/lib/Drupal/Core/Access/AccessManager.php +++ b/core/lib/Drupal/Core/Access/AccessManager.php @@ -32,23 +32,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. * * @param string $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 035f282..79d9eb4 100644 --- a/core/lib/Drupal/Core/CoreBundle.php +++ b/core/lib/Drupal/Core/CoreBundle.php @@ -226,7 +226,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 420780a..6f18839 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 7098b41..80d6ae8 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 1927c1c..8892866 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/menu/menu.module b/core/modules/menu/menu.module index 44723dd..09363c4 100644 --- a/core/modules/menu/menu.module +++ b/core/modules/menu/menu.module @@ -195,6 +195,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 865b8ed..aaf4190 100644 --- a/core/modules/menu_link/lib/Drupal/menu_link/MenuLinkStorageController.php +++ b/core/modules/menu_link/lib/Drupal/menu_link/MenuLinkStorageController.php @@ -324,6 +324,31 @@ public function loadModuleAdminTasks() { } /** + * Implements \Drupal\Core\Entity\EntityStorageControllerInterface::load(). + */ + public function load(array $ids = NULL) { + $entities = parent::load($ids); + + // 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. + $routes = array(); + foreach ($entities as $id => $entity) { + if ($entity->route) { + $routes[$id] = $entity->route; + } + } + + if ($routes) { + $route_objects = drupal_container()->get('router.route_provider')->getRoutesByNames($routes); + foreach ($routes as $entity_id => $route) { + $entities[$entity_id]->setRouteObject($route_objects[$route]); + } + } + + return $entities; + } + + /** * Checks and updates the 'has_children' property for the parent of a link. * * @param \Drupal\Core\Entity\EntityInterface $entity 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 bccbb67..eb7d4b1 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\Core\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 Link, if any. + * + * @var string + */ + public $route; + + /** + * The route object associated with this Link, if any. + * + * @var \Symfony\Component\Routing\Route + */ + protected $routeObject; + + /** * Overrides Entity::id(). */ public function id() { @@ -248,6 +264,36 @@ public function createDuplicate() { } /** + * Returns the Route object associated with this link, if any. + * + * @return \Symfony\Component\Routing\Route + * The route object for this Link, or NULL if there isn't one. + */ + public function getRoute() { + if (!$this->route) { + return NULL; + } + if (! $this->routeObject instanceof Route) { + $route_provider = drupal_container()->get('router.route_provider'); + $this->routeObject = $route_provider->getRouteByName($this->route); + } + 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. * * @return \Drupal\Core\Entity\EntityInterface diff --git a/core/modules/menu_link/menu_link.module b/core/modules/menu_link/menu_link.module index ed11c78..74532f4 100644 --- a/core/modules/menu_link/menu_link.module +++ b/core/modules/menu_link/menu_link.module @@ -117,7 +117,7 @@ function menu_link_delete_multiple(array $mlids, $force = FALSE, $prevent_repare * The menu link entity to be saved. */ function menu_link_save(MenuLink $menu_link) { - $menu_link->save(); + return $menu_link->save(); } /** diff --git a/core/modules/system/system.install b/core/modules/system/system.install index cfc9a4e..e033aed 100644 --- a/core/modules/system/system.install +++ b/core/modules/system/system.install @@ -955,6 +955,11 @@ function system_schema() { 'type' => 'text', 'size' => 'medium', ), + 'route' => array( + 'description' => 'The machine name of a defined Symfony Route this menu item represents.', + 'type' => 'varchar', + 'length' => 255, + ), ), 'indexes' => array( 'fit' => array('fit'), @@ -2095,6 +2100,19 @@ function system_update_8048() { } /** + * Adds route column to the menu_router table. + */ +function system_update_8049() { + $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', $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 19dc974..7b23920 100644 --- a/core/modules/user/user.module +++ b/core/modules/user/user.module @@ -870,11 +870,8 @@ function user_menu() { $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'), + 'route' => 'user_autocomplete', 'type' => MENU_CALLBACK, - 'file' => 'user.pages.inc', ); $items['user/autocomplete/anonymous'] = array( @@ -919,8 +916,7 @@ function user_menu() { // 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' => 'user_register', ); $items['user/password'] = array(