Fix non-parameter route constraints not called with endpoint routing for 2.2 (#6587)

Author: JamesNK

eng/PatchConfig.props

@@ -31,6 +31,7 @@ Later on, this will be checked using this condition:
       Microsoft.AspNetCore.AspNetCoreModuleV2;
       Microsoft.AspNetCore.Authentication.Google;
       Microsoft.AspNetCore.Http;
+      Microsoft.AspNetCore.Mvc.Core;
       Microsoft.AspNetCore.Server.IIS;
       java:signalr;
     </PackagesInPatch>

src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcEndpointDataSource.cs

@@ -224,6 +224,15 @@ private int CreateEndpoints(
             var newPathSegments = routePattern.PathSegments.ToList();
             var hasLinkGenerationEndpoint = false;
 
+            // This is required because we create modified copies of the route pattern using its segments
+            // A segment with a parameter will automatically include its policies
+            // Non-parameter policies need to be manually included
+            var nonParameterPolicyValues = routePattern.ParameterPolicies
+                .Where(p => routePattern.GetParameter(p.Key ?? string.Empty) == null && p.Value.Count > 0 && p.Value.First().ParameterPolicy != null) // Only GetParameter is required. Extra is for safety
+                .Select(p => new KeyValuePair<string, object>(p.Key, p.Value.First().ParameterPolicy)) // Can only pass a single non-parameter to RouteParameter
+                .ToArray();
+            var nonParameterPolicies = RouteValueDictionary.FromArray(nonParameterPolicyValues);
+
             // Create a mutable copy
             var nonInlineDefaultsCopy = nonInlineDefaults != null
                 ? new RouteValueDictionary(nonInlineDefaults)
@@ -259,6 +268,7 @@ private int CreateEndpoints(
                             resolvedRouteValues,
                             name,
                             GetPattern(ref patternStringBuilder, newPathSegments),
+                            nonParameterPolicies,
                             newPathSegments,
                             nonInlineDefaultsCopy,
                             routeOrder++,
@@ -277,6 +287,7 @@ private int CreateEndpoints(
                         resolvedRouteValues,
                         name,
                         GetPattern(ref patternStringBuilder, subPathSegments),
+                        nonParameterPolicies,
                         subPathSegments,
                         nonInlineDefaultsCopy,
                         routeOrder++,
@@ -294,6 +305,7 @@ private int CreateEndpoints(
                 resolvedRouteValues,
                 name,
                 GetPattern(ref patternStringBuilder, newPathSegments),
+                nonParameterPolicies,
                 newPathSegments,
                 nonInlineDefaultsCopy,
                 routeOrder++,
@@ -531,6 +543,7 @@ private RouteEndpoint CreateEndpoint(
             IDictionary<string, string> actionRouteValues,
             string routeName,
             string patternRawText,
+            object nonParameterPolicies,
             IEnumerable<RoutePatternPathSegment> segments,
             object nonInlineDefaults,
             int order,
@@ -561,7 +574,7 @@ private RouteEndpoint CreateEndpoint(
 
             var endpoint = new RouteEndpoint(
                 requestDelegate,
-                RoutePatternFactory.Pattern(patternRawText, defaults, parameterPolicies: null, segments),
+                RoutePatternFactory.Pattern(patternRawText, defaults, nonParameterPolicies, segments),
                 order,
                 metadataCollection,
                 action.DisplayName);

src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MvcEndpointDataSourceTests.cs

@@ -14,6 +14,7 @@
 using Microsoft.AspNetCore.Mvc.Infrastructure;
 using Microsoft.AspNetCore.Mvc.Routing;
 using Microsoft.AspNetCore.Routing;
+using Microsoft.AspNetCore.Routing.Constraints;
 using Microsoft.AspNetCore.Routing.Matching;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Options;
@@ -311,6 +312,109 @@ public void Endpoints_SingleAction_ConventionalRoute_ContainsParameterWithNullRe
             Assert.Empty(endpoints);
         }
 
+        [Fact]
+        public void Endpoints_SingleAction_ConventionalRoute_ContainsNonParameterConstraint()
+        {
+            // Arrange
+            var actionDescriptorCollection = GetActionDescriptorCollection(
+                new { controller = "TestController", action = "TestAction", page = (string)null });
+            var dataSource = CreateMvcEndpointDataSource(actionDescriptorCollection);
+            dataSource.ConventionalEndpointInfos.Add(CreateEndpointInfo(
+                string.Empty,
+                "{controller}/{action}/{id:range(0, 100)}",
+                new RouteValueDictionary(new { action = "TestAction" }),
+                new RouteValueDictionary(new { controller = "TestController", nonParameter = new CustomConstraint(), id = new IntRouteConstraint() })));
+
+            // Act
+            var endpoints = dataSource.Endpoints;
+
+            // Assert
+            var endpoint = Assert.IsType<RouteEndpoint>(Assert.Single(endpoints));
+
+            var routePattern = endpoint.RoutePattern;
+
+            Assert.Equal("TestController/TestAction/{id::range(0, 100)}", routePattern.RawText);
+            Assert.Collection(routePattern.ParameterPolicies.OrderBy(p => p.Key),
+                p =>
+                {
+                    Assert.Equal("id", p.Key);
+                    Assert.Collection(p.Value,
+                        c => Assert.IsType<IntRouteConstraint>(c.ParameterPolicy),
+                        c => Assert.Equal("range(0, 100)", c.Content));
+                },
+                p =>
+                {
+                    Assert.Equal("nonParameter", p.Key);
+                    Assert.IsType<CustomConstraint>(p.Value.Single().ParameterPolicy);
+                });
+        }
+
+        [Fact]
+        public void Endpoints_SingleAction_ConventionalRouteWithOptional_ContainsNonParameterConstraint()
+        {
+            // Arrange
+            var actionDescriptorCollection = GetActionDescriptorCollection(
+                new { controller = "TestController", action = "TestAction", page = (string)null });
+            var dataSource = CreateMvcEndpointDataSource(actionDescriptorCollection);
+            dataSource.ConventionalEndpointInfos.Add(CreateEndpointInfo(
+                string.Empty,
+                "{controller}/{action}/{id?}",
+                new RouteValueDictionary(new { action = "TestAction" }),
+                new RouteValueDictionary(new { controller = "TestController", nonParameter = new CustomConstraint(), id = new IntRouteConstraint() })));
+
+            // Act
+            var endpoints = dataSource.Endpoints;
+
+            // Assert
+            var endpoint1 = Assert.IsType<RouteEndpoint>(endpoints[0]);
+            var routePattern1 = endpoint1.RoutePattern;
+            Assert.Equal("TestController/{action=TestAction}/{id:?}", routePattern1.RawText);
+            Assert.Collection(routePattern1.ParameterPolicies.OrderBy(p => p.Key),
+                p =>
+                {
+                    Assert.Equal("id", p.Key);
+                    Assert.IsType<IntRouteConstraint>(p.Value.Single().ParameterPolicy);
+                },
+                p =>
+                {
+                    Assert.Equal("nonParameter", p.Key);
+                    Assert.IsType<CustomConstraint>(p.Value.Single().ParameterPolicy);
+                });
+
+            var endpoint2 = Assert.IsType<RouteEndpoint>(endpoints[1]);
+            var routePattern2 = endpoint2.RoutePattern;
+            Assert.Equal("TestController", routePattern2.RawText);
+            Assert.Collection(routePattern2.ParameterPolicies.OrderBy(p => p.Key),
+                p =>
+                {
+                    Assert.Equal("nonParameter", p.Key);
+                    Assert.IsType<CustomConstraint>(p.Value.Single().ParameterPolicy);
+                });
+
+            var endpoint3 = Assert.IsType<RouteEndpoint>(endpoints[2]);
+            var routePattern3 = endpoint3.RoutePattern;
+            Assert.Equal("TestController/TestAction/{id:?}", routePattern3.RawText);
+            Assert.Collection(routePattern3.ParameterPolicies.OrderBy(p => p.Key),
+                p =>
+                {
+                    Assert.Equal("id", p.Key);
+                    Assert.IsType<IntRouteConstraint>(p.Value.Single().ParameterPolicy);
+                },
+                p =>
+                {
+                    Assert.Equal("nonParameter", p.Key);
+                    Assert.IsType<CustomConstraint>(p.Value.Single().ParameterPolicy);
+                });
+        }
+
+        private class CustomConstraint : IRouteConstraint
+        {
+            public bool Match(HttpContext httpContext, IRouter route, string routeKey, RouteValueDictionary values, RouteDirection routeDirection)
+            {
+                throw new NotImplementedException();
+            }
+        }
+
         [Fact]
         public void Endpoints_SingleAction_AttributeRoute_ContainsParameterWithNullRequiredRouteValue()
         {

src/Mvc/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RoutingTestsBase.cs

@@ -26,6 +26,32 @@ private static void ConfigureWebHostBuilder(IWebHostBuilder builder) =>
 
         public HttpClient Client { get; }
 
+        [Fact]
+        public async Task ConventionalRoutedAction_RouteHasNonParameterConstraint_RouteConstraintRun_Allowed()
+        {
+            // Arrange & Act
+            var response = await Client.GetAsync("http://localhost/NonParameterConstraintRoute/NonParameterConstraint/Index?allowed=true");
+
+            // Assert
+            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+
+            var body = await response.Content.ReadAsStringAsync();
+            var result = JsonConvert.DeserializeObject<RoutingResult>(body);
+
+            Assert.Equal("NonParameterConstraint", result.Controller);
+            Assert.Equal("Index", result.Action);
+        }
+
+        [Fact]
+        public async Task ConventionalRoutedAction_RouteHasNonParameterConstraint_RouteConstraintRun_Denied()
+        {
+            // Arrange & Act
+            var response = await Client.GetAsync("http://localhost/NonParameterConstraintRoute/NonParameterConstraint/Index?allowed=false");
+
+            // Assert
+            Assert.Equal(HttpStatusCode.NotFound, response.StatusCode);
+        }
+
         [Fact]
         public async Task ConventionalRoutedAction_RouteContainsPage_RouteNotMatched()
         {

src/Mvc/test/WebSites/RoutingWebSite/Controllers/NonParameterConstraintController.cs

@@ -0,0 +1,26 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Threading.Tasks;
+using Microsoft.AspNetCore.Mvc;
+
+namespace RoutingWebSite
+{
+    public class NonParameterConstraintController : Controller
+    {
+        private readonly TestResponseGenerator _generator;
+
+        public NonParameterConstraintController(TestResponseGenerator generator)
+        {
+            _generator = generator;
+        }
+
+        public IActionResult Index()
+        {
+            return _generator.Generate("/NonParameterConstraintRoute/NonParameterConstraint/Index");
+        }
+    }
+}

src/Mvc/test/WebSites/RoutingWebSite/QueryStringConstraint.cs

@@ -0,0 +1,16 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Routing;
+
+namespace RoutingWebSite
+{
+    public class QueryStringConstraint : IRouteConstraint
+    {
+        public bool Match(HttpContext httpContext, IRouter route, string routeKey, RouteValueDictionary values, RouteDirection routeDirection)
+        {
+            return httpContext.Request.Query["allowed"].ToString() == "true";
+        }
+    }
+}

src/Mvc/test/WebSites/RoutingWebSite/Startup.cs

@@ -49,6 +49,12 @@ public void Configure(IApplicationBuilder app)
         {
             app.UseMvc(routes =>
             {
+                routes.MapRoute(
+                    "NonParameterConstraintRoute",
+                    "NonParameterConstraintRoute/{controller}/{action}",
+                    defaults: null,
+                    constraints: new { controller = "NonParameterConstraint", nonParameter = new QueryStringConstraint() });
+
                 routes.MapRoute(
                     "DataTokensRoute",
                     "DataTokensRoute/{controller}/{action}",

src/Mvc/test/WebSites/RoutingWebSite/StartupWith21Compat.cs

@@ -54,6 +54,12 @@ public void Configure(IApplicationBuilder app)
         {
             app.UseMvc(routes =>
             {
+                routes.MapRoute(
+                    "NonParameterConstraintRoute",
+                    "NonParameterConstraintRoute/{controller}/{action}",
+                    defaults: null,
+                    constraints: new { controller = "NonParameterConstraint", nonParameter = new QueryStringConstraint() });
+
                 routes.MapRoute(
                     "DataTokensRoute",
                     "DataTokensRoute/{controller}/{action}",