Skip to content

Ambigious routing after update to stable version #148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
xakep139 opened this issue Jun 21, 2017 · 3 comments · Fixed by #149
Closed

Ambigious routing after update to stable version #148

xakep139 opened this issue Jun 21, 2017 · 3 comments · Fixed by #149
Assignees

Comments

@xakep139
Copy link

xakep139 commented Jun 21, 2017

I've created a simple project to repro this bug: https://github.com/xakep139/simpleAspNetCoreApplication
If you request /api/1.0/values/123/abcdef and then /api/1.0/values/123/versions it goes into GetVersion() action in both request.
But if change order of requests it works as expected - GetVersions() and then GetVersion()

@qazq
Copy link

qazq commented Jun 21, 2017

I got the same issue, but change the order is not work for me. It seems related to request sequence. If request /api/1.0/values/123/versions and then api/1.0/values/123/abcdef, it work as expect.

@commonsensesoftware
Copy link
Collaborator

Thanks for the repro and behavior description. I found the issue immediately. In short, it's due to an optimization in the action selector. Since ASP.NET Core might call the action selector multiple times, all the results have to be tallied and then a final decision is made at the end once all possible candidates have been considered. This could negatively hurt performance if this was the case for every, single request. An optimization exists that tracks whether an action has been matched all the way through the pipeline at least once. Once it has, the action selector can short-circuit and return the match without considering other candidates. What's missing is a check to determine whether there are high precedence candidates when this happens.

In this scenario:

  • /api/1.0/values/123/abcdef is lower precedence because versions is a constant segment
  • however, /api/1.0/values/123/abcdef is requested first and the match results are cached
  • when /api/1.0/values/123/versions is requested, it hasn't been matched before, so all candidates are considered
  • /api/1.0/values/123/{versionId} overlaps with the route template and will match as a possible candidate because the literal value "versions" is a suite value for {versionId}
    • since the result for this candidate has been cached, the process short-circuits and matches the wrong action

This should be a relatively simple fix. I'll get it out ASAP. I suspect that this is also the cause for #147.

@xakep139
Copy link
Author

Thanks for this qiuck fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants