View with user/% path breaks login/logout

it is also /user/register that is affected.

When I rollback to 8.5.0 and follow the steps in the IS I also get the 404, no need to go to 8.5.1 to make it break. So not sure if this is 8.5.1 related really.

We just ran into this problem, though I only found this issue by chance. By some drush magic I was able to fix it fairly quickly once identified.

Nevertheless: Setting a view to path "user/%" is something recommended and described in a lot of "HowTos" around the web, plus some modules do this by themself. Once this has happened, anyone but a very capable user with Shell Access is unable to access his site and fix the issue.

So I think the "Critical Issue" criteria "Render a site unusable and have no workaround." is fulfilled here.

I'd would say that it's normal it would break those pages, but not necessarily return a 404.

Also, user/%user is probably much safer to use so that user/logout (or anything on user/%) is not affected.

It seems indeed critical as user/% path overriding with views used to work for Drupal 8 up to now

@Swentel I tried changing the view to use user/%user and the login/logout worked again but the view stopped working as it should

So currently there is no workaround other than changing the view path

The easiest solution/workaround that I found was to alter the paths login / logout / register / password
here is small module that does this https://github.com/GiorgosK/user_routes_alter

Here is a fairly ugly test, but this illustrates the point....

@swentel I'm also not surprised they would break, I'm surprised it took till 8.5.x to break...but a regression it is....

@mixologic queued the patch for 8.4.x.

The passing 8.5.x php 7.0 result is actually against 8.4.x, the php 7.1 result is against 8.5.x

I think the source of this problem is that the way of route matching process is now "correct" since core 8.5. It includes some route filters, and one of them is Drupal\Core\Routing\RequestFormatRouteFilter which changes the ordering of matched routes by checking for the parameter _format, default set to value html.

Guess the related change record is this one #2883680: Force all route filters and route enhancers to be non-lazy

The Views module does set the _format parameter to html, while other core components do not (for example all entity routes). Here is the location where Views does this: Class Drupal\views\Plugin\views\display\Page line 93-94:

<?php
// Explicitly set HTML as the format for Page displays.
$route->setRequirement('_format', 'html');
?>

This results in the situation that routes generated by Views would mostly win on the route matching process.

Maybe the other route definitions need an explicit parameter definition of _format too, or the logic of the route filter Drupal\Core\Routing\RequestFormatRouteFilter must be changed.

Maybe #2854543: NegotiationMiddleware calls $request->setRequestFormat('html') when there is no _format request parameter, but shouldn't helps with this too, then?

@Berdir : I've tried last applying patch from your linked issue #2854543-72: NegotiationMiddleware calls $request->setRequestFormat('html') when there is no _format request parameter, but shouldn't, but user/login & logout also does not work with an active view with user/% path.

I have same problem, on my Thunder based website.
After I disabled my custom view with path "/user/%" problem was gone.
That view was provided by old Thunder distro (infinite_user).

#2969297: It is not longer possible to override /taxonomy/{}/view with a more specific path without brekaing /taxonomy/{}/edit contains some more technical analysis why this happens.

So is this issue now the main one to address the problem?

Relating #2449143: REST views specify HTML as a possible request format, so if there is a "regular" HTML view on the same path, it will serve JSON as "the root of this problem".

So is this issue now the main one to address the problem?

I guess :) #2449143: REST views specify HTML as a possible request format, so if there is a "regular" HTML view on the same path, it will serve JSON caused not just one problem.

Here is a proposal: Let's re-enforce the route fit with another route filter.
I manually tested the /taxonomy/{}/edit example, and it works for me as expected.

Turns out I was using probably php 5.6 syntax.

#21 looks reasonable to me (although I wouldn't agree the already high complexity and bloat of the existing route matching process).

Combined the fix with the test from #8 and minor cleanup.

The fix looks reasonable, but my knowledge of the routing system isn't up to speed enough to judge if this is the best way to do this.

+++ b/core/core.services.yml
@@ -958,6 +958,12 @@ services:
+  fit_filter:
+    class: Drupal\Core\Routing\FitFilter
+    tags:
+      # The fit route filter must run late: it re-enforces order by re-applying different
+      # level of fit (how specific the path was in the first place).
+      - { name: route_filter, priority: -50 }

Discussed with @dawehner. We weren't sure whether this should be a filter or just part of the router that happens after the routes are filtered. I'm leaning towards it not being a filter because I don't think a site should be able to opt out of this behaviour.

Discussed a bit more with @dawehner. I think we might need to sub class RouteCollection so that our one is fit aware. This is because with the current solution the filters become unaware of the route that is going to "win". Because the order at the end of filtering might be changed. This fit-aware route collection would always add into the right spot so we no longer need to resort at the end of the chain.

I'm not sure the route collection solution will really work as good as expected, because there is a lot of code out there which does: new RouteCollection().
Subclassing the class by ourself though will not just work.

Here is a patch which moves the fit ordering to the router itself.

+++ b/core/lib/Drupal/Core/Routing/Router.php
@@ -286,6 +287,37 @@ protected function applyRouteFilters(RouteCollection $collection, Request $reque
 
+  /**
+   * Reapplies the fit order.
+   *
+   * @param \Symfony\Component\Routing\RouteCollection $collection
+   *   The route collection.
+   *
+   * @return \Symfony\Component\Routing\RouteCollection
+   *   The reordered route collection.
+   */
+  protected function applyFitOrder(RouteCollection $collection) {

If it's re-applying instead of just applying, I think we need an explanatory comment.

If it's re-applying instead of just applying, I think we need an explanatory comment.

I do agree. I tried to massively improve the documentation.

This is a generic routing problem.

I think this is a good fix for now. We have explored trying to fix this in a way that doesn't require a resorting at the end but that doesn't look like it is going to be simple so this is probably the best we can do.

I think we missing some unit testing of the new method since atm the only test added is a views test.

+++ b/core/lib/Drupal/Core/Routing/Router.php
@@ -286,6 +287,44 @@ protected function applyRouteFilters(RouteCollection $collection, Request $reque
+  /**
+   * Reapplies the fit order.
+   *
+   * Route filters apply some custom order. For example routes with an explicit
+   * _format requirement will be preferred. This could though result into routes
+   * with less fit to be preferred over others (For example /user/% comes before
+   * /user/login). In order to not break this fundamental property of routes, we
+   * need to reapply this order, while keeping the previous order stable within
+   * each group with same fit.
+   *
+   * @param \Symfony\Component\Routing\RouteCollection $collection
+   *   The route collection.
+   *
+   * @return \Symfony\Component\Routing\RouteCollection
+   *   The reordered route collection.
+   */

I think we can make this comment slightly clearer.

How about...

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

I have facing same problem so that I have used alterRoute of login and logout pages

changed PATH :
/user/login > /login
/user/password > /password
/user/logout > /logout
/user/register > /register

Please find the process for Altering existing routes and adding new routes based on dynamic ones at this page https://www.drupal.org/docs/8/api/routing-system/altering-existing-routes-and-adding-new-routes-based-on-dynamic-ones

The patch from #17 fixes the problem on our side as well for views routes like /blog/2018 (while the 2018 part is a contextual filter with validation to be numerical) that broke our feed URL at /blog/feed (where feed is no contextual filter and thus the full /blog/feed route should take priority over the one with the dynamic year argument).

Thanx for fixing this!

I used git diff --color-words to look at the suggestions from @alexpott.

On top of that I added a unit test for this specific class.

Removed version number from title.

1. +++ b/core/tests/Drupal/Tests/Core/Routing/RouterTest.php
   @@ -0,0 +1,59 @@
   +/**
   + * @coversDefaultClass \Drupal\Core\Routing\Router
   + */
   +class RouterTest extends UnitTestCase {
   

   Needs an @group annotation.

2. +++ b/core/tests/Drupal/Tests/Core/Routing/RouterTest.php
   @@ -0,0 +1,59 @@
   +    $route_collection = new RouteCollection();
   +  ¶
   +    $route = new Route('/taxonomy/{term}');
   

   The blank line here contains spaces.

Also as far as I can see the test does not fail if the patch is not applied to the Router so it's not quite covering what we hope it is.

@alexpott and @dawehner worked on this patch together as part of the DrupalNorth Unconference.

Fix a minor whitespace issue.

The new unit test now fails without the fix. Nice.

+1 one commit, been bitten this by this after an upgrade, patch works fine!

Committed and pushed 18ff78aaf0 to 8.7.x and 9c94695902 to 8.6.x. Thanks!