Issue #2075889 by pwolanin, kgoel, alexpott, catch, mpdonadio, YesCT, himanshu-dixit, dawehner, xjm, johnshortess, Crell, jhodgdon, Wim Leers, mradcliffe, attiks, tstoeckler, webchick: Make Drupal handle incoming paths in a case-insensitive fashion for routing

8.4.x
Nathaniel Catchpole 2017-02-28 11:49:30 +00:00
parent 1fb7dc9c23
commit b1242b2928
14 changed files with 516 additions and 17 deletions

View File

@ -2,6 +2,7 @@
namespace Drupal\Core\Routing;
use Drupal\Component\Utility\Unicode;
use Symfony\Component\Routing\CompiledRoute as SymfonyCompiledRoute;
/**
@ -65,7 +66,10 @@ class CompiledRoute extends SymfonyCompiledRoute {
parent::__construct($staticPrefix, $regex, $tokens, $pathVariables, $hostRegex, $hostTokens, $hostVariables, $variables);
$this->fit = $fit;
$this->patternOutline = $pattern_outline;
// Support case-insensitive route matching by ensuring the pattern outline
// is lowercase.
// @see \Drupal\Core\Routing\RouteProvider::getRoutesByPath()
$this->patternOutline = Unicode::strtolower($pattern_outline);
$this->numParts = $num_parts;
}

View File

@ -46,8 +46,14 @@ class RouteCompiler extends SymfonyRouteCompiler implements RouteCompilerInterfa
$fit,
$pattern_outline,
$num_parts,
// These are the Symfony compiled parts.
$symfony_compiled->getStaticPrefix(),
// 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.
'',
$symfony_compiled->getRegex(),
$symfony_compiled->getTokens(),
$symfony_compiled->getPathVariables(),
@ -55,14 +61,14 @@ class RouteCompiler extends SymfonyRouteCompiler implements RouteCompilerInterfa
$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.

View File

@ -2,6 +2,7 @@
namespace Drupal\Core\Routing;
use Drupal\Component\Utility\Unicode;
use Drupal\Core\Cache\Cache;
use Drupal\Core\Cache\CacheBackendInterface;
use Drupal\Core\Cache\CacheTagsInvalidatorInterface;
@ -139,7 +140,9 @@ class RouteProvider implements PreloadableRouteProviderInterface, PagedRouteProv
*
* @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,20 @@ class RouteProvider implements PreloadableRouteProviderInterface, PagedRouteProv
* 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().
// @see \Drupal\Core\Routing\CompiledRoute::__construct()
$parts = preg_split('@/+@', Unicode::strtolower($path), NULL, PREG_SPLIT_NO_EMPTY);
$collection = new RouteCollection();
@ -347,7 +355,8 @@ class RouteProvider implements PreloadableRouteProviderInterface, PagedRouteProv
$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) {

View File

@ -18,7 +18,9 @@ interface RouteProviderInterface extends RouteProviderBaseInterface {
* 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);

View File

@ -159,6 +159,96 @@ class Router extends UrlMatcher implements RequestMatcherInterface, RouterInterf
: new ResourceNotFoundException(sprintf('No routes found for "%s".', $this->currentPath->getPath()));
}
/**
* Tries to match a URL with a set of routes.
*
* @param string $pathinfo
* The path info to be parsed
* @param \Symfony\Component\Routing\RouteCollection $routes
* The set of routes.
*
* @return array|null
* An array of parameters. NULL when there is no match.
*/
protected function matchCollection($pathinfo, RouteCollection $routes) {
// Try a case-sensitive match.
$match = $this->doMatchCollection($pathinfo, $routes, TRUE);
// Try a case-insensitive match.
if ($match === NULL && $routes->count() > 0) {
$match = $this->doMatchCollection($pathinfo, $routes, FALSE);
}
return $match;
}
/**
* Tries to match a URL with a set of routes.
*
* This code is very similar to Symfony's UrlMatcher::matchCollection() but it
* supports case-insensitive matching. The static prefix optimization is
* removed as this duplicates work done by the query in
* RouteProvider::getRoutesByPath().
*
* @param string $pathinfo
* The path info to be parsed
* @param \Symfony\Component\Routing\RouteCollection $routes
* The set of routes.
* @param bool $case_sensitive
* Determines if the match should be case-sensitive of not.
*
* @return array|null
* An array of parameters. NULL when there is no match.
*
* @see \Symfony\Component\Routing\Matcher\UrlMatcher::matchCollection()
* @see \Drupal\Core\Routing\RouteProvider::getRoutesByPath()
*/
protected function doMatchCollection($pathinfo, RouteCollection $routes, $case_sensitive) {
foreach ($routes as $name => $route) {
$compiledRoute = $route->compile();
// Set the regex to use UTF-8.
$regex = $compiledRoute->getRegex() . 'u';
if (!$case_sensitive) {
$regex = $regex . 'i';
}
if (!preg_match($regex, $pathinfo, $matches)) {
continue;
}
$hostMatches = array();
if ($compiledRoute->getHostRegex() && !preg_match($compiledRoute->getHostRegex(), $this->context->getHost(), $hostMatches)) {
$routes->remove($name);
continue;
}
// Check HTTP method requirement.
if ($requiredMethods = $route->getMethods()) {
// HEAD and GET are equivalent as per RFC.
if ('HEAD' === $method = $this->context->getMethod()) {
$method = 'GET';
}
if (!in_array($method, $requiredMethods)) {
$this->allow = array_merge($this->allow, $requiredMethods);
$routes->remove($name);
continue;
}
}
$status = $this->handleRouteRequirements($pathinfo, $name, $route);
if (self::ROUTE_MATCH === $status[0]) {
return $status[1];
}
if (self::REQUIREMENT_MISMATCH === $status[0]) {
$routes->remove($name);
continue;
}
return $this->getAttributes($route, $name, array_replace($matches, $hostMatches));
}
}
/**
* Returns a collection of potential matching routes for a request.
*

View File

@ -43,10 +43,16 @@
* by the machine name of the module that defines the route, or the name of
* a subsystem.
* - The 'path' line gives the URL path of the route (relative to the site's
* base URL). Note: The path in Drupal is treated case insensitive so
* /example and /EXAmplE should return the same page.
* @todo Fix https://www.drupal.org/node/2075889 to actually get this
* behaviour.
* base URL). Generally, paths in Drupal are treated as case-insensitive,
* which overrides the default Symfony behavior. Specifically:
* - If different routes are defined for /example and /EXAmplE, the exact
* match is respected.
* - If there is no exact match, the route falls back to a case-insensitive
* match, so /example and /EXAmplE will return the same page.
* Relying on case-sensitive path matching is not recommended because it
* negatively affects user experience, and path aliases do not support case-
* sensitive matches. The case-sensitive exact match is currently supported
* only for backwards compatibility and may be deprecated in a later release.
* - The 'defaults' section tells how to build the main content of the route,
* and can also give other information, such as the page title and additional
* arguments for the route controller method. There are several possibilities

View File

@ -100,6 +100,34 @@ class RouterTest extends WebTestBase {
$this->assertFalse(isset($headers['x-drupal-cache-tags']));
}
/**
* 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.
$this->drupalGet('router-test/case-sensitive-duplicate-path3');
$this->assertResponse(200);
$this->assertRaw('router_test.case_sensitive_duplicate1');
// While case-insensitive matching works, exact matches are preferred.
$this->drupalGet('router-test/case-sensitive-Duplicate-PATH3');
$this->assertResponse(200);
$this->assertRaw('router_test.case_sensitive_duplicate2');
// Test that case-insensitive matching works, falling back to the first
// route defined.
$this->drupalGet('router-test/case-sensitive-Duplicate-Path3');
$this->assertResponse(200);
$this->assertRaw('router_test.case_sensitive_duplicate1');
}
/**
* Confirms that placeholders in paths work correctly.
*/

View File

@ -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.case_sensitive_duplicate1:
path: '/router-test/case-sensitive-duplicate-path3'
defaults:
_controller: '\Drupal\router_test\TestControllers::testRouteName'
requirements:
_access: 'TRUE'
router_test.case_sensitive_duplicate2:
path: '/router-test/case-sensitive-Duplicate-PATH3'
defaults:
_controller: '\Drupal\router_test\TestControllers::testRouteName'
requirements:
_access: 'TRUE'
router_test.case_sensitive_duplicate3:
path: '/router-test/case-sensitive-duplicate-path3'
defaults:
_controller: '\Drupal\router_test\TestControllers::testRouteName'
requirements:
_access: 'TRUE'

View File

@ -6,6 +6,7 @@ use Drupal\Core\Cache\CacheableResponse;
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 @@ class TestControllers {
];
}
public function testRouteName(Request $request) {
return [
'#markup' => $request->attributes->get(RouteObjectInterface::ROUTE_NAME),
];
}
/**
* Throws an exception.
*

View File

@ -312,6 +312,21 @@ class SystemTestController extends ControllerBase {
return 'Bar.' . $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.
*

View File

@ -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'

View File

@ -0,0 +1,113 @@
<?php
namespace Drupal\FunctionalTests\Routing;
use Drupal\Tests\BrowserTestBase;
/**
* Tests incoming path case insensitivity.
*
* @group routing
*/
class CaseInsensitivePathTest extends BrowserTestBase {
/**
* {@inheritdoc}
*/
public static $modules = ['system', 'views', 'node', 'system_test'];
/**
* {@inheritdoc}
*/
protected function setUp() {
parent::setUp();
\Drupal::state()->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/');
}
}

View File

@ -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 @@ class RouteProviderTest extends KernelTestBase {
}
/**
* 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();

View File

@ -139,6 +139,74 @@ class RoutingFixtures {
return $collection;
}
/**
* 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 Hebrew 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.
*