Aspnetcore: PageLoaderMatcherPolicy doesn't check candidate validity.

[jtkech]

Describe the bug

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.

To Reproduce

Steps to reproduce the behavior:

1. Define a DynamicRouteValueTransformer mapped to a general pattern, e.g

   routes.MapDynamicControllerRoute<AutoRouteTransformer>("/{any}/{**slug}");

but that doesn't return any route values for a given path e.g `/test`

1. Then, define a Razor Page whose route pattern is /test

2. Run the application and try to access the above razor page.

3. It fails at PageLoaderMatcherPolicy line 71.

Expected behavior

PageLoaderMatcherPolicy should not fail by checking the validity of each candidate (or a null check on the endpoint) as it is done in other matcher policies.

I did a copy of PageLoaderMatcherPolicyand registered it in place of the original one. Then, adding the following fixed the issue.

    public Task ApplyAsync(HttpContext httpContext, CandidateSet candidates)
    {
        ...
        ...
        for (var i = 0; i < candidates.Count; i++)
        {
            // Additional checking.
            if (!candidates.IsValidCandidate(i))
            {
                continue;
            }
        ...
        ...

Additional context

SDK .NET Core (reflétant tous les global.json) :
 Version:   3.0.100-preview9-014004
 Commit:    8e7ef240a5

Environnement d'exécution :
 OS Name:     Windows
 OS Version:  10.0.18362
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\3.0.100-preview9-014004\

Host (useful for support):
  Version: 3.0.0-preview9-19423-09
  Commit:  2be172345a

.NET Core SDKs installed:
  2.1.604 [C:\Program Files\dotnet\sdk]
  2.1.700 [C:\Program Files\dotnet\sdk]
  2.1.800-preview-009677 [C:\Program Files\dotnet\sdk]
  2.1.801 [C:\Program Files\dotnet\sdk]
  2.2.100 [C:\Program Files\dotnet\sdk]
  3.0.100-preview9-014004 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed:
  Microsoft.AspNetCore.All 2.1.11 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.11 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.0.0-preview5-19227-01 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.0.0-preview9.19424.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.11 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.0.0-preview5-27626-15 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.0.0-preview9-19423-09 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.0.0-preview9-19423-09 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

[arkadiuszwojcik]

Just to note, DynamicPageEndpointMatcherPolicy have this check:

if (!candidates.IsValidCandidate(i))
{
   continue;
}

in place where PageLoaderMatcherPolicy should, as proposed above by @jtkech

[mkArtakMSFT]

Thanks for contacting us, @jtkech.
@rynowak can you please look into this? Thanks!

[CoskunSunali]

I am experiencing the same issue, unfortunately. A fix would be really appreciated.

[CoskunSunali]

By the way, is there a workaround you might suggest until the issue is fixed?

[jtkech]

Right now we use a temporary policy (with the same class name) that is just a copy but with the fix, and we override the original registration.

// 'PageLoaderMatcherPolicy' doesn't check if an endpoint is a valid candidate.
// So, we replace it by a custom implementation that does it as other policies.

var descriptor = collection.FirstOrDefault(d => d.ServiceType == typeof(MatcherPolicy) &&
    d.ImplementationType?.Name == nameof(PageLoaderMatcherPolicy));

if (descriptor != null)
{
    collection.Remove(descriptor);
    collection.TryAddEnumerable(ServiceDescriptor.Singleton<MatcherPolicy, PageLoaderMatcherPolicy>());
}

[CoskunSunali]

@jtkech That helped a lot. I was wondering how to remove/replace the exact service because 1) it is internal 2) its service type has multiple registrations. Using its name is smart.

That fixed the issue even though it is a workaround.

Hope the team fixes the issue with the 3.1 release.

[pranavkm]

@mkArtakMSFT putting this back in 3.1.0-preview3. It's a fairly small fix.

[CoskunSunali]

@mkArtakMSFT putting this back in 3.1.0-preview3. It's a fairly small fix.

@pranavkm Thanks for the quick fix.