Skip to content

ApiVersionMatcherPolicy doesn't check candidate validity. #600

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
jtkech opened this issue Jan 31, 2020 · 1 comment
Closed

ApiVersionMatcherPolicy doesn't check candidate validity. #600

jtkech opened this issue Jan 31, 2020 · 1 comment
Assignees
Milestone

Comments

@jtkech
Copy link

jtkech commented Jan 31, 2020

In OrchardCore we don't use AddApiVersioning() by default but someone did and experienced a NRE in ApiVersionMatcherPolicy. We already had the exact same issue with PageLoaderMatcherPolicy that we use by default, see here the related issue we opened and whose fix has been merged.

Here the description of this related issue where you can replace PageLoaderMatcherPolicy with ApiVersionMatcherPolicy.

If a DynamicRouteValueTransformer, mapped to a given pattern, doesn't always return some route values related to a valid endpoint, when this is the case for a given path which also matches a razor page, PageLoaderMatcherPolicy fails line 71 because of a null endpoint.

The issue was that the policy was not checking the endpoint validity as done e.g in DynamicPageEndpointMatcherPolicy, but not here in ApiVersionMatcherPolicy.

    ...
    for (var i = 0; i < candidates.Count; i++)
    {
        // Additional checking.
        if (!candidates.IsValidCandidate(i))
        {
            continue;
        }
    ...
@commonsensesoftware
Copy link
Collaborator

With diving into further, I'm just going to assume that this is a bug because the implementation is based off of examples and guidance from the ASP.NET Core team. Thanks for reporting it. I'll work on getting a fix out.

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

No branches or pull requests

2 participants