diff --git a/core/lib/Drupal/Core/Routing/RouteCompiler.php b/core/lib/Drupal/Core/Routing/RouteCompiler.php index 937bf48..52d0041 100644 --- a/core/lib/Drupal/Core/Routing/RouteCompiler.php +++ b/core/lib/Drupal/Core/Routing/RouteCompiler.php @@ -2,6 +2,7 @@ namespace Drupal\Core\Routing; +use Drupal\Component\Utility\Unicode; use Symfony\Component\Routing\RouteCompilerInterface; use Symfony\Component\Routing\Route; use Symfony\Component\Routing\RouteCompiler as SymfonyRouteCompiler; @@ -37,7 +38,8 @@ public static function compile(Route $route) { // The Drupal-specific compiled information. $stripped_path = static::getPathWithoutDefaults($route); $fit = static::getFit($stripped_path); - $pattern_outline = static::getPatternOutline($stripped_path); + // Store a lower-case pattern outline to enable case-insensitive matching. + $pattern_outline = Unicode::strtolower(static::getPatternOutline($stripped_path)); // We count the number of parts including any optional trailing parts. This // allows the RouteProvider to filter candidate routes more efficiently. $num_parts = count(explode('/', trim($route->getPath(), '/'))); @@ -46,23 +48,30 @@ public static function compile(Route $route) { $fit, $pattern_outline, $num_parts, - // These are the Symfony compiled parts. - $symfony_compiled->getStaticPrefix(), - $symfony_compiled->getRegex(), + + // The following parameters are what Symfony uses in + // \Symfony\Component\Routing\Matcher\UrlMatcher::matchCollection(). + + // Set the static prefix to an empty string since it is redundant to + // the matching in \Drupal\Core\Routing\RouteProvider::getRoutesByPath() + // and by skipping it we more easily make the routing case insensitive. + '', + // Set the regex to use UTF-8 and be case-insensitive. + $symfony_compiled->getRegex() . 'ui', $symfony_compiled->getTokens(), $symfony_compiled->getPathVariables(), $symfony_compiled->getHostRegex(), $symfony_compiled->getHostTokens(), $symfony_compiled->getHostVariables(), $symfony_compiled->getVariables() - ); + ); } /** * Returns the pattern outline. * * The pattern outline is the path pattern but normalized so that all - * placeholders are equal strings and default values are removed. + * placeholders are the string '%'. * * @param string $path * The path for which we want the normalized outline. diff --git a/core/lib/Drupal/Core/Routing/RouteProvider.php b/core/lib/Drupal/Core/Routing/RouteProvider.php index 53ed626..4586652 100644 --- a/core/lib/Drupal/Core/Routing/RouteProvider.php +++ b/core/lib/Drupal/Core/Routing/RouteProvider.php @@ -5,6 +5,7 @@ use Drupal\Core\Cache\Cache; use Drupal\Core\Cache\CacheBackendInterface; use Drupal\Core\Cache\CacheTagsInvalidatorInterface; +use Drupal\Component\Utility\Unicode; use Drupal\Core\Path\CurrentPathStack; use Drupal\Core\PathProcessor\InboundPathProcessorInterface; use Drupal\Core\State\StateInterface; @@ -139,7 +140,9 @@ public function __construct(Connection $connection, StateInterface $state, Curre * * @return \Symfony\Component\Routing\RouteCollection with all urls that * could potentially match $request. Empty collection if nothing can - * match. + * match. The collection will be sorted from highest to lowest fit (match + * of path parts) and then in ascending order by route name for routes + * with the same fit. */ public function getRouteCollectionForRequest(Request $request) { // Cache both the system path as well as route parameters and matching @@ -317,15 +320,19 @@ public function getRoutesByPattern($pattern) { * Get all routes which match a certain pattern. * * @param string $path - * The route pattern to search for (contains % as placeholders). + * The route pattern to search for. * * @return \Symfony\Component\Routing\RouteCollection - * Returns a route collection of matching routes. + * Returns a route collection of matching routes. The collection may be + * empty and will be sorted from highest to lowest fit (match of path parts) + * and then in ascending order by route name for routes with the same fit. */ protected function getRoutesByPath($path) { // Split the path up on the slashes, ignoring multiple slashes in a row - // or leading or trailing slashes. - $parts = preg_split('@/+@', $path, NULL, PREG_SPLIT_NO_EMPTY); + // or leading or trailing slashes. Convert to lower case here so we can + // have a case insensitive match from the incoming path to the lower case + // pattern outlines from \Drupal\Core\Routing\RouteCompiler::compile(). + $parts = preg_split('@/+@', Unicode::strtolower($path), NULL, PREG_SPLIT_NO_EMPTY); $collection = new RouteCollection(); @@ -347,7 +354,8 @@ protected function getRoutesByPath($path) { $routes = []; } - // We sort by fit and name in PHP to avoid a SQL filesort. + // We sort by fit and name in PHP to avoid a SQL filesort and avoid any + // difference in the sorting behavior of SQL back-ends. usort($routes, array($this, 'routeProviderRouteCompare')); foreach ($routes as $row) { diff --git a/core/lib/Drupal/Core/Routing/RouteProviderInterface.php b/core/lib/Drupal/Core/Routing/RouteProviderInterface.php index ce07a6d..f53593a 100644 --- a/core/lib/Drupal/Core/Routing/RouteProviderInterface.php +++ b/core/lib/Drupal/Core/Routing/RouteProviderInterface.php @@ -18,7 +18,9 @@ * The route pattern to search for (contains {} as placeholders). * * @return \Symfony\Component\Routing\RouteCollection - * Returns a route collection of matching routes. + * Returns a route collection of matching routes. The collection may be + * empty and will be sorted from highest to lowest fit (match of path parts) + * and then in ascending order by route name for routes with the same fit. */ public function getRoutesByPattern($pattern); diff --git a/core/modules/system/src/Tests/Routing/RouterTest.php b/core/modules/system/src/Tests/Routing/RouterTest.php index c6aa2b8..db8a8af9 100644 --- a/core/modules/system/src/Tests/Routing/RouterTest.php +++ b/core/modules/system/src/Tests/Routing/RouterTest.php @@ -101,6 +101,29 @@ public function testFinishResponseSubscriber() { } /** + * Confirms that multiple routes with the same path do not cause an error. + */ + public function testDuplicateRoutePaths() { + // Tests two routes with exactly the same path. The route with the maximum + // fit and lowest sorting route name will match, regardless of the order the + // routes are declared. + // @see \Drupal\Core\Routing\RouteProvider::getRoutesByPath() + $this->drupalGet('router-test/duplicate-path2'); + $this->assertResponse(200); + $this->assertRaw('router_test.two_duplicate1'); + + // Tests three routes with same the path. One of the routes the path has a + // different case. The route with the maximum fit and lowest sorting route + // name will match, regardless of the order the routes are declared. + $this->drupalGet('router-test/duplicate-path3'); + $this->assertResponse(200); + $this->assertRaw('router_test.three_duplicate1'); + $this->drupalGet('router-test/Duplicate-PATH3'); + $this->assertResponse(200); + $this->assertRaw('router_test.three_duplicate1'); + } + + /** * Confirms that placeholders in paths work correctly. */ public function testControllerPlaceholders() { diff --git a/core/modules/system/tests/modules/router_test_directory/router_test.routing.yml b/core/modules/system/tests/modules/router_test_directory/router_test.routing.yml index ad2d418..b9a0958 100644 --- a/core/modules/system/tests/modules/router_test_directory/router_test.routing.yml +++ b/core/modules/system/tests/modules/router_test_directory/router_test.routing.yml @@ -212,3 +212,38 @@ router_test.hierarchy_parent_child2: _controller: '\Drupal\router_test\TestControllers::test' requirements: _access: 'TRUE' + +router_test.two_duplicate1: + path: '/router-test/duplicate-path2' + defaults: + _controller: '\Drupal\router_test\TestControllers::testRouteName' + requirements: + _access: 'TRUE' + +router_test.two_duplicate2: + path: '/router-test/duplicate-path2' + defaults: + _controller: '\Drupal\router_test\TestControllers::testRouteName' + requirements: + _access: 'TRUE' + +router_test.three_duplicate2: + path: '/router-test/duplicate-path3' + defaults: + _controller: '\Drupal\router_test\TestControllers::testRouteName' + requirements: + _access: 'TRUE' + +router_test.three_duplicate3: + path: '/router-test/Duplicate-PATH3' + defaults: + _controller: '\Drupal\router_test\TestControllers::testRouteName' + requirements: + _access: 'TRUE' + +router_test.three_duplicate1: + path: '/router-test/duplicate-path3' + defaults: + _controller: '\Drupal\router_test\TestControllers::testRouteName' + requirements: + _access: 'TRUE' diff --git a/core/modules/system/tests/modules/router_test_directory/src/TestControllers.php b/core/modules/system/tests/modules/router_test_directory/src/TestControllers.php index c3bcac7..151c140 100644 --- a/core/modules/system/tests/modules/router_test_directory/src/TestControllers.php +++ b/core/modules/system/tests/modules/router_test_directory/src/TestControllers.php @@ -6,6 +6,7 @@ use Drupal\Core\ParamConverter\ParamNotConvertedException; use Drupal\user\UserInterface; use Symfony\Cmf\Component\Routing\RouteObjectInterface; +use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Zend\Diactoros\Response\HtmlResponse; @@ -110,6 +111,12 @@ public function test25() { ]; } + public function testRouteName(Request $request) { + return [ + '#markup' => $request->attributes->get(RouteObjectInterface::ROUTE_NAME), + ]; + } + /** * Throws an exception. * diff --git a/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php b/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php index eb8709e..8ef6457 100644 --- a/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php +++ b/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php @@ -313,6 +313,21 @@ public function configureTitle($foo) { } /** + * Simple argument echo. + * + * @param string $text + * Any string for the {text} slug. + * + * @return array + * A render array. + */ + public function simpleEcho($text) { + return [ + '#plain_text' => $text, + ]; + } + + /** * Shows permission-dependent content. * * @return array diff --git a/core/modules/system/tests/modules/system_test/system_test.routing.yml b/core/modules/system/tests/modules/system_test/system_test.routing.yml index f35f546..579b511 100644 --- a/core/modules/system/tests/modules/system_test/system_test.routing.yml +++ b/core/modules/system/tests/modules/system_test/system_test.routing.yml @@ -182,3 +182,17 @@ system_test.header: _controller: '\Drupal\system_test\Controller\SystemTestController::getTestHeader' requirements: _access: 'TRUE' + +system_test.echo: + path: '/system-test/echo/{text}' + defaults: + _controller: '\Drupal\system_test\Controller\SystemTestController::simpleEcho' + requirements: + _access: 'TRUE' + +system_test.echo_utf8: + path: '/system-test/Ȅchȏ/meφΩ/{text}' + defaults: + _controller: '\Drupal\system_test\Controller\SystemTestController::simpleEcho' + requirements: + _access: 'TRUE' diff --git a/core/tests/Drupal/FunctionalTests/Routing/CaseInsensitivePathTest.php b/core/tests/Drupal/FunctionalTests/Routing/CaseInsensitivePathTest.php new file mode 100644 index 0000000..183d140 --- /dev/null +++ b/core/tests/Drupal/FunctionalTests/Routing/CaseInsensitivePathTest.php @@ -0,0 +1,113 @@ +set('system_test.module_hidden', FALSE); + $this->createContentType(['type' => 'page']); + } + + /** + * Tests mixed case paths. + */ + public function testMixedCasePaths() { + // Tests paths defined by routes from standard modules as anonymous. + $this->drupalGet('user/login'); + $this->assertSession()->statusCodeEquals(200); + $this->assertSession()->pageTextMatches('/Log in/'); + $this->drupalGet('User/Login'); + $this->assertSession()->statusCodeEquals(200); + $this->assertSession()->pageTextMatches('/Log in/'); + + // Tests paths defined by routes from the Views module. + $admin = $this->drupalCreateUser(['access administration pages', 'administer nodes', 'access content overview']); + $this->drupalLogin($admin); + + $this->drupalGet('admin/content'); + $this->assertSession()->statusCodeEquals(200); + $this->assertSession()->pageTextMatches('/Content/'); + $this->drupalGet('Admin/Content'); + $this->assertSession()->statusCodeEquals(200); + $this->assertSession()->pageTextMatches('/Content/'); + + // Tests paths with query arguments. + + // Make sure our node title doesn't exist. + $this->drupalGet('admin/content'); + $this->assertSession()->linkNotExists('FooBarBaz'); + $this->assertSession()->linkNotExists('foobarbaz'); + + // Create a node, and make sure it shows up on admin/content. + $node = $this->createNode([ + 'title' => 'FooBarBaz', + 'type' => 'page', + ]); + + $this->drupalGet('admin/content', [ + 'query' => [ + 'title' => 'FooBarBaz' + ] + ]); + + $this->assertSession()->linkExists('FooBarBaz'); + $this->assertSession()->linkByHrefExists($node->toUrl()->toString()); + + // Make sure the path is case insensitive, and query case is preserved. + + $this->drupalGet('Admin/Content', [ + 'query' => [ + 'title' => 'FooBarBaz' + ] + ]); + + $this->assertSession()->linkExists('FooBarBaz'); + $this->assertSession()->linkByHrefExists($node->toUrl()->toString()); + $this->assertSession()->fieldValueEquals('edit-title', 'FooBarBaz'); + // Check that we can access the node with a mixed case path. + $this->drupalGet('NOdE/' . $node->id()); + $this->assertSession()->statusCodeEquals(200); + $this->assertSession()->pageTextMatches('/FooBarBaz/'); + } + + /** + * Tests paths with slugs. + */ + public function testPathsWithArguments() { + $this->drupalGet('system-test/echo/foobarbaz'); + $this->assertSession()->statusCodeEquals(200); + $this->assertSession()->pageTextMatches('/foobarbaz/'); + $this->assertSession()->pageTextNotMatches('/FooBarBaz/'); + + $this->drupalGet('system-test/echo/FooBarBaz'); + $this->assertSession()->statusCodeEquals(200); + $this->assertSession()->pageTextMatches('/FooBarBaz/'); + $this->assertSession()->pageTextNotMatches('/foobarbaz/'); + + // Test utf-8 characters in the route path. + $this->drupalGet('/system-test/Ȅchȏ/meΦω/ABc123'); + $this->assertSession()->statusCodeEquals(200); + $this->assertSession()->pageTextMatches('/ABc123/'); + $this->drupalGet('/system-test/ȅchȎ/MEΦΩ/ABc123'); + $this->assertSession()->statusCodeEquals(200); + $this->assertSession()->pageTextMatches('/ABc123/'); + } + +} diff --git a/core/tests/Drupal/KernelTests/Core/Routing/RouteProviderTest.php b/core/tests/Drupal/KernelTests/Core/Routing/RouteProviderTest.php index fa135f0..f5c8047 100644 --- a/core/tests/Drupal/KernelTests/Core/Routing/RouteProviderTest.php +++ b/core/tests/Drupal/KernelTests/Core/Routing/RouteProviderTest.php @@ -7,6 +7,7 @@ namespace Drupal\KernelTests\Core\Routing; +use Drupal\Component\Utility\Unicode; use Drupal\Core\Cache\MemoryBackend; use Drupal\Core\Database\Database; use Drupal\Core\DependencyInjection\ContainerBuilder; @@ -195,7 +196,108 @@ function testOutlinePathMatch() { } /** - * Confirms that a trailing slash on the request doesn't result in a 404. + * Data provider for testMixedCasePaths() + */ + public function providerMixedCaseRoutePaths() { + return [ + ['/path/one', 'route_a'], + ['/path/two', NULL], + ['/PATH/one', 'route_a'], + ['/path/2/one', 'route_b', 'PUT'], + ['/paTH/3/one', 'route_b', 'PUT'], + // There should be no lower case of a Hebrew letter. + ['/somewhere/4/over/the/קainbow', 'route_c'], + ['/Somewhere/5/over/the/קainboW', 'route_c'], + ['/another/llama/aboUT/22', 'route_d'], + ['/another/llama/about/22', 'route_d'], + ['/place/meΦω', 'route_e', 'HEAD'], + ['/place/meφΩ', 'route_e', 'HEAD'], + ]; + } + + /** + * Confirms that we find routes using a case insensitive path match. + * + * @dataProvider providerMixedCaseRoutePaths + */ + public function testMixedCasePaths($path, $expected_route_name, $method = 'GET') { + // The case-insensitive behavior for higher UTF-8 characters depends on + // \Drupal\Component\Utility\Unicode::strtolower() using mb_strtolower() + // but kernel tests do not currently run the check that enables it. + // @todo remove this when https://www.drupal.org/node/2849669 is fixed. + Unicode::check(); + + $connection = Database::getConnection(); + $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, $this->cacheTagsInvalidator, 'test_routes'); + + $this->fixtures->createTables($connection); + + $dumper = new MatcherDumper($connection, $this->state, 'test_routes'); + $dumper->addRoutes($this->fixtures->mixedCaseRouteCollection()); + $dumper->dump(); + + $request = Request::create($path, $method); + + $routes = $provider->getRouteCollectionForRequest($request); + + if ($expected_route_name) { + $this->assertEquals(1, count($routes), 'The correct number of routes was found.'); + $this->assertNotNull($routes->get($expected_route_name), 'The first matching route was found.'); + } + else { + $this->assertEquals(0, count($routes), 'No routes matched.'); + } + } + + /** + * Data provider for testMixedCasePaths() + */ + public function providerDuplicateRoutePaths() { + // When matching routes with the same fit the route with the lowest-sorting + // name should end up first in the resulting route collection. + return [ + ['/path/one', 3, 'route_a'], + ['/PATH/one', 3, 'route_a'], + ['/path/two', 1, 'route_d'], + ['/PATH/three', 0], + ['/place/meΦω', 2, 'route_e'], + ['/placE/meφΩ', 2, 'route_e'], + ]; + } + + /** + * Confirms that we find all routes with the same path. + * + * @dataProvider providerDuplicateRoutePaths + */ + public function testDuplicateRoutePaths($path, $number, $expected_route_name = NULL) { + + // The case-insensitive behavior for higher UTF-8 characters depends on + // \Drupal\Component\Utility\Unicode::strtolower() using mb_strtolower() + // but kernel tests do not currently run the check that enables it. + // @todo remove this when https://www.drupal.org/node/2849669 is fixed. + Unicode::check(); + + $connection = Database::getConnection(); + $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, $this->cacheTagsInvalidator, 'test_routes'); + + $this->fixtures->createTables($connection); + + $dumper = new MatcherDumper($connection, $this->state, 'test_routes'); + $dumper->addRoutes($this->fixtures->duplicatePathsRouteCollection()); + $dumper->dump(); + + $request = Request::create($path); + $routes = $provider->getRouteCollectionForRequest($request); + $this->assertEquals($number, count($routes), 'The correct number of routes was found.'); + if ($expected_route_name) { + $route_name = key(current($routes)); + $this->assertEquals($expected_route_name, $route_name, 'The expected route name was found.'); + } + } + + /** + * Confirms that a trailing slash on the request does not result in a 404. */ function testOutlinePathMatchTrailingSlash() { $connection = Database::getConnection(); diff --git a/core/tests/Drupal/Tests/Core/Routing/RoutingFixtures.php b/core/tests/Drupal/Tests/Core/Routing/RoutingFixtures.php index d1826ed..0384cd0 100644 --- a/core/tests/Drupal/Tests/Core/Routing/RoutingFixtures.php +++ b/core/tests/Drupal/Tests/Core/Routing/RoutingFixtures.php @@ -140,6 +140,74 @@ public function complexRouteCollection() { } /** + * Returns a complex set of routes for testing. + * + * @return \Symfony\Component\Routing\RouteCollection + */ + public function mixedCaseRouteCollection() { + $collection = new RouteCollection(); + + $route = new Route('/path/one'); + $route->setMethods(['GET']); + $collection->add('route_a', $route); + + $route = new Route('/path/{thing}/one'); + $route->setMethods(['PUT']); + $collection->add('route_b', $route); + + // Uses Hewbrew letter QOF (U+05E7) + $route = new Route('/somewhere/{item}/over/the/קainbow'); + $route->setMethods(['GET']); + $collection->add('route_c', $route); + + $route = new Route('/another/{thing}/aboUT/{item}'); + $collection->add('route_d', $route); + + // Greek letters lower case phi (U+03C6) and lower case omega (U+03C9) + $route = new Route('/place/meφω'); + $route->setMethods(['GET', 'HEAD']); + $collection->add('route_e', $route); + + return $collection; + } + + /** + * Returns a complex set of routes for testing. + * + * @return \Symfony\Component\Routing\RouteCollection + */ + public function duplicatePathsRouteCollection() { + $collection = new RouteCollection(); + + $route = new Route('/path/one'); + $route->setMethods(['GET']); + $collection->add('route_b', $route); + + // Add the routes not in order by route name. + $route = new Route('/path/one'); + $route->setMethods(['GET']); + $collection->add('route_a', $route); + + $route = new Route('/path/one'); + $route->setMethods(['GET']); + $collection->add('route_c', $route); + + $route = new Route('/path/TWO'); + $route->setMethods(['GET']); + $collection->add('route_d', $route); + + // Greek letters lower case phi (U+03C6) and lower case omega (U+03C9) + $route = new Route('/place/meφω'); + $route->setMethods(['GET', 'HEAD']); + $collection->add('route_f', $route); + + $route = new Route('/PLACE/meφω'); + $collection->add('route_e', $route); + + return $collection; + } + + /** * Returns a Content-type restricted set of routes for testing. * * @return \Symfony\Component\Routing\RouteCollection