Issue #2959370 by dawehner, Lendude, alexpott: View with user/% path breaks login/logout

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}
    */

core/modules/views/tests/modules/views_test_config/test_views/views.view.test_user_path.yml

+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: {  }

core/modules/views/tests/src/Functional/UserPathTest.php

+<?php
+
+namespace Drupal\Tests\views\Functional;
+
+/**
+ * Tests overriding user paths using wildcards.
+ *
+ * @group views
+ */
+class UserPathTest extends ViewTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  public static $modules = ['views', 'user'];
+
+  /**
+   * The test views to use.
+   *
+   * @var array
+   */
+  public static $testViews = ['test_user_path'];
+
+  /**
+   * Tests if the login page is still available when using a wildcard path.
+   */
+  public function testUserLoginPage() {
+    $this->drupalGet('user/login');
+    $this->assertSession()->statusCodeEquals(200);
+  }
+
+}

core/tests/Drupal/Tests/Core/Routing/RouterTest.php

+<?php
+
+namespace Drupal\Tests\Core\Routing;
+
+use Drupal\Core\Path\CurrentPathStack;
+use Drupal\Core\Routing\RequestContext;
+use Drupal\Core\Routing\RouteCompiler;
+use Drupal\Core\Routing\RouteProviderInterface;
+use Drupal\Core\Routing\Router;
+use Drupal\Core\Routing\UrlGeneratorInterface;
+use Drupal\Tests\UnitTestCase;
+use Prophecy\Argument;
+use Symfony\Component\Routing\Route;
+use Symfony\Component\Routing\RouteCollection;
+
+/**
+ * @coversDefaultClass \Drupal\Core\Routing\Router
+ * @group Routing
+ */
+class RouterTest extends UnitTestCase {
+
+  /**
+   * @covers ::applyFitOrder
+   */
+  public function testMatchesWithDifferentFitOrder() {
+    $route_provider = $this->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']);
+  }
+
+}