diff --git a/core/lib/Drupal/Core/Routing/Router.php b/core/lib/Drupal/Core/Routing/Router.php index ee2dc28acd041f38549f84c272d61a874fab2cb3..c6c5dc0aa7662d805ee1688f0c80adc8f800d017 100644 --- a/core/lib/Drupal/Core/Routing/Router.php +++ b/core/lib/Drupal/Core/Routing/Router.php @@ -125,6 +125,7 @@ public function matchRequest(Request $request) { throw new ResourceNotFoundException(sprintf('No routes found for "%s".', $this->currentPath->getPath())); } $collection = $this->applyRouteFilters($collection, $request); + $collection = $this->applyFitOrder($collection); if ($ret = $this->matchCollection(rawurldecode($this->currentPath->getPath($request)), $collection)) { return $this->applyRouteEnhancers($ret, $request); @@ -286,6 +287,44 @@ protected function applyRouteFilters(RouteCollection $collection, Request $reque return $collection; } + /** + * Reapplies the fit order to a RouteCollection object. + * + * Route filters can reorder route collections. For example, routes with an + * explicit _format requirement will be preferred. This can result in a less + * fit route being used. For example, as a result of filtering /user/% comes + * before /user/login. In order to not break this fundamental property of + * routes, we need to reapply the fit order. We also need to ensure that order + * within each group of the same fit is preserved. + * + * @param \Symfony\Component\Routing\RouteCollection $collection + * The route collection. + * + * @return \Symfony\Component\Routing\RouteCollection + * The reordered route collection. + */ + protected function applyFitOrder(RouteCollection $collection) { + $buckets = []; + // Sort all the routes by fit descending. + foreach ($collection->all() as $name => $route) { + $fit = $route->compile()->getFit(); + $buckets += [$fit => []]; + $buckets[$fit][] = [$name, $route]; + } + krsort($buckets); + + $flattened = array_reduce($buckets, 'array_merge', []); + + // Add them back onto a new route collection. + $collection = new RouteCollection(); + foreach ($flattened as $pair) { + $name = $pair[0]; + $route = $pair[1]; + $collection->add($name, $route); + } + return $collection; + } + /** * {@inheritdoc} */ diff --git a/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_user_path.yml b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_user_path.yml new file mode 100644 index 0000000000000000000000000000000000000000..a5e629f06c0df7ce9f4c3b619115b8b11797aa80 --- /dev/null +++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_user_path.yml @@ -0,0 +1,158 @@ +langcode: en +status: true +dependencies: + module: + - user +id: test_user_path +label: 'user break' +module: views +description: '' +tag: '' +base_table: users_field_data +base_field: uid +core: 8.x +display: + default: + display_plugin: default + id: default + display_title: Master + position: 0 + display_options: + access: + type: perm + options: + perm: 'access user profiles' + cache: + type: tag + options: { } + query: + type: views_query + options: + disable_sql_rewrite: false + distinct: false + replica: false + query_comment: '' + query_tags: { } + exposed_form: + type: basic + options: + submit_button: Toepassen + reset_button: false + reset_button_label: Reset + exposed_sorts_label: 'Sorteren op' + expose_sort_order: true + sort_asc_label: Oplopend + sort_desc_label: Aflopend + pager: + type: mini + options: + items_per_page: 10 + offset: 0 + id: 0 + total_pages: null + expose: + items_per_page: false + items_per_page_label: 'Items per pagina' + items_per_page_options: '5, 10, 25, 50' + items_per_page_options_all: false + items_per_page_options_all_label: '- Alle -' + offset: false + offset_label: Startpunt + tags: + previous: ‹‹ + next: ›› + style: + type: default + row: + type: fields + fields: + name: + id: name + table: users_field_data + field: name + entity_type: user + entity_field: name + label: '' + alter: + alter_text: false + make_link: false + absolute: false + trim: false + word_boundary: false + ellipsis: false + strip_tags: false + html: false + hide_empty: false + empty_zero: false + plugin_id: field + relationship: none + group_type: group + admin_label: '' + exclude: false + element_type: '' + element_class: '' + element_label_type: '' + element_label_class: '' + element_label_colon: true + element_wrapper_type: '' + element_wrapper_class: '' + element_default_classes: true + empty: '' + hide_alter_empty: true + click_sort_column: value + type: user_name + settings: { } + group_column: value + group_columns: { } + group_rows: true + delta_limit: 0 + delta_offset: 0 + delta_reversed: false + delta_first_last: false + multi_type: separator + separator: ', ' + field_api_classes: false + filters: + status: + value: '1' + table: users_field_data + field: status + plugin_id: boolean + entity_type: user + entity_field: status + id: status + expose: + operator: '' + group: 1 + sorts: { } + title: 'user break' + header: { } + footer: { } + empty: { } + relationships: { } + arguments: { } + display_extenders: { } + cache_metadata: + max-age: -1 + contexts: + - 'languages:language_content' + - 'languages:language_interface' + - url.query_args + - user.permissions + tags: { } + page_1: + display_plugin: page + id: page_1 + display_title: Page + position: 1 + display_options: + display_extenders: { } + path: user/% + cache_metadata: + max-age: -1 + contexts: + - 'languages:language_content' + - 'languages:language_interface' + - url.query_args + - user.permissions + tags: { } diff --git a/core/modules/views/tests/src/Functional/UserPathTest.php b/core/modules/views/tests/src/Functional/UserPathTest.php new file mode 100644 index 0000000000000000000000000000000000000000..27a0f27a9bbaa500bf4c9e6528e632fc4bef7ed4 --- /dev/null +++ b/core/modules/views/tests/src/Functional/UserPathTest.php @@ -0,0 +1,32 @@ +drupalGet('user/login'); + $this->assertSession()->statusCodeEquals(200); + } + +} diff --git a/core/tests/Drupal/Tests/Core/Routing/RouterTest.php b/core/tests/Drupal/Tests/Core/Routing/RouterTest.php new file mode 100644 index 0000000000000000000000000000000000000000..d6ba1d43fb0a4970ec375c302a1921234ad91967 --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Routing/RouterTest.php @@ -0,0 +1,60 @@ +prophesize(RouteProviderInterface::class); + + $route_collection = new RouteCollection(); + + $route = new Route('/user/{user}'); + $route->setOption('compiler_class', RouteCompiler::class); + $route_collection->add('user_view', $route); + + $route = new Route('/user/login'); + $route->setOption('compiler_class', RouteCompiler::class); + $route_collection->add('user_login', $route); + + $route_provider->getRouteCollectionForRequest(Argument::any()) + ->willReturn($route_collection); + + $url_generator = $this->prophesize(UrlGeneratorInterface::class); + $current_path_stack = $this->prophesize(CurrentPathStack::class); + $router = new Router($route_provider->reveal(), $current_path_stack->reveal(), $url_generator->reveal()); + + $request_context = $this->prophesize(RequestContext::class); + $request_context->getScheme()->willReturn('http'); + $router->setContext($request_context->reveal()); + + $current_path_stack->getPath(Argument::any())->willReturn('/user/1'); + $result = $router->match('/user/1'); + + $this->assertEquals('user_view', $result['_route']); + + $current_path_stack->getPath(Argument::any())->willReturn('/user/login'); + $result = $router->match('/user/login'); + + $this->assertEquals('user_login', $result['_route']); + } + +}