Aspnetcore: Ambigous route matches

[avin-kavish]

Describe the bug

These two routes lead to an ambiguous match on Toyota-Corolla-vehicles/2.

    [HttpGet("{make}-{query}-vehicles/{makeId:int}")]
    [HttpGet("{make}-vehicles/{makeId:int}")]

According to my understanding, they shouldn't as the two parameters are separated by a - which is a valid string literal. As per the docs, the routes match non greedily from right to left by comparing literals. the first route has two literals, the second route has only one literal, so I don't see why it would be ambiguous when Toyota-Corolla-vehicles/2 contains two literals that match the first route in the form of - and -vehicles

To Reproduce

Add these routes to an action

    [HttpGet("{make}-{query}-vehicles/{makeId:int}")]
    [HttpGet("{make}-vehicles/{makeId:int}")]

Fire a get in the format

Toyota-Corolla-vehicles/2

[huysentruitw]

Both route templates match equally well, so the ambiguity is correct IMO.

[avin-kavish]

How so? There is a literal dash between {make} and {query} in the first one, a non-greedy right to left scan should match the dash between toyota and corolla to that, shouldn't it? They both match yes, but they are by no means equal in weight. The route with the most string literals in it should be treated as more specific therefore match the query.

[huysentruitw]

Matching is a boolean operation, so it either matches or it doesn't. I don't think any _weight_ is taken into consideration, but I'm no specialist here.

See https://github.com/aspnet/Routing/blob/9cea167cfac36cf034dbb780e3f783114ef94780/src/Microsoft.AspNetCore.Routing/Patterns/RoutePatternMatcher.cs#L296

[avin-kavish]

FYI. I've already worked past it. As we all know, there's a hundred different ways to do one thing with general purpose programming languages.

But don't you think the implementation could be improved to recognise these two routes as distinct ? As far as I can see these two routes are machine recognisable as two distinct routes.

The documentation of route matching of complex segments mention that the route matches non-greedily from right to left. A non greedy match takes the least amount of characters to make a match which means {query} should grab Corolla and {make} should grab Toyota. But if {make} matches the entire literal Toyota-Corolla, (which seems to be the cause of ambiguity) it is greedy by definition and is inconsistent with the documentation mentioned here.

Complex segments (for example, [Route("/dog{token}cat")]), are processed by matching up literals from right to left in a non-greedy way. See the source code for a description. For more information, see this issue.

Maybe the documentation needs to elaborate by saying that non-greedy matching is not respected across different route definitions but only within a single definition, (which seems to be the case)

The cause of this issue is that my mental representation of an MVC framework avoids conflicts based on the greediness of the match across routes but apparently ASP Net Core doesn't, which is fine but it would be nice to be mentioned in the documentation.

[huysentruitw]

I'm not trying to defend the current implementation, the idea you have makes perfectly sense. However, I do think that it would make the route matching code more complex and probably slower (although the slowdown could be limited to figuring out the correct path in case of ambiguity). So let's wait until someone from the core team joins this conversation.

[mkArtakMSFT]

Thanks for contacting us, @avin-kavish.
@rynowak can you please have a look at this? Thanks!

[rynowak]

You can configure which one of these you want to be considered first with Order. https://docs.microsoft.com/en-us/aspnet/core/mvc/controllers/routing?view=aspnetcore-2.2#ordering-attribute-routes

[msftbot[bot]]

Thank you for contacting us. Due to no activity on this issue we're closing it in an effort to keep our backlog clean. If you believe there is a concern related to the ASP.NET Core framework, which hasn't been addressed yet, please file a new issue.