Change route and query fallback semantics (#35317)

* Change route and query fallback semantics
- If the list of route parameters is empty then simple parameters are from query only.
- If the list of route parameters is null then fallback from route to query.
- Added tests.

Author: davidfowl

src/Http/Http.Extensions/src/RequestDelegateFactory.cs

@@ -247,11 +247,22 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext
             }
             else if (parameter.ParameterType == typeof(string) || TryParseMethodCache.HasTryParseMethod(parameter))
             {
-                // We're in the fallback case and we have a parameter and route parameter match so don't fallback
-                // to query string in this case
-                if (factoryContext.RouteParameters is { } routeParams && routeParams.Contains(parameter.Name, StringComparer.OrdinalIgnoreCase))
+                // 1. We bind from route values only, if route parameters are non-null and the parameter name is in that set.
+                // 2. We bind from query only, if route parameters are non-null and the parameter name is NOT in that set.
+                // 3. Otherwise, we fallback to route or query if route parameters is null (it means we don't know what route parameters are defined). This case only happens
+                // when RDF.Create is manually invoked.
+                if (factoryContext.RouteParameters is { } routeParams)
                 {
-                    return BindParameterFromProperty(parameter, RouteValuesExpr, parameter.Name, factoryContext);
+                    if (routeParams.Contains(parameter.Name, StringComparer.OrdinalIgnoreCase))
+                    {
+                        // We're in the fallback case and we have a parameter and route parameter match so don't fallback
+                        // to query string in this case
+                        return BindParameterFromProperty(parameter, RouteValuesExpr, parameter.Name, factoryContext);
+                    }
+                    else
+                    {
+                        return BindParameterFromProperty(parameter, QueryExpr, parameter.Name, factoryContext);
+                    }
                 }
 
                 return BindParameterFromRouteValueOrQueryString(parameter, parameter.Name, factoryContext);

src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs

@@ -248,6 +248,62 @@ public async Task SpecifiedRouteParametersDoNotFallbackToQueryString()
             Assert.Null(httpContext.Items["input"]);
         }
 
+        [Fact]
+        public async Task SpecifiedQueryParametersDoNotFallbackToRouteValues()
+        {
+            var httpContext = new DefaultHttpContext();
+
+            var requestDelegate = RequestDelegateFactory.Create((int? id, HttpContext httpContext) =>
+            {
+                if (id is not null)
+                {
+                    httpContext.Items["input"] = id;
+                }
+            },
+            new() { RouteParameterNames = new string[] { } });
+
+            httpContext.Request.Query = new QueryCollection(new Dictionary<string, StringValues>
+            {
+                ["id"] = "41"
+            });
+            httpContext.Request.RouteValues = new()
+            {
+                ["id"] = "42"
+            };
+
+            await requestDelegate(httpContext);
+
+            Assert.Equal(41, httpContext.Items["input"]);
+        }
+
+        [Fact]
+        public async Task NullRouteParametersPrefersRouteOverQueryString()
+        {
+            var httpContext = new DefaultHttpContext();
+
+            var requestDelegate = RequestDelegateFactory.Create((int? id, HttpContext httpContext) =>
+            {
+                if (id is not null)
+                {
+                    httpContext.Items["input"] = id;
+                }
+            },
+            new() { RouteParameterNames = null });
+
+            httpContext.Request.Query = new QueryCollection(new Dictionary<string, StringValues>
+            {
+                ["id"] = "41"
+            });
+            httpContext.Request.RouteValues = new()
+            {
+                ["id"] = "42"
+            };
+
+            await requestDelegate(httpContext);
+
+            Assert.Equal(42, httpContext.Items["input"]);
+        }
+
         [Fact]
         public async Task CreatingDelegateWithInstanceMethodInfoCreatesInstancePerCall()
         {
@@ -825,7 +881,7 @@ void TestAction([FromBody] Todo todo)
             httpContext.Request.Body = new IOExceptionThrowingRequestBodyStream(invalidDataException);
             httpContext.Features.Set<IHttpRequestBodyDetectionFeature>(new RequestBodyDetectionFeature(true));
             httpContext.Features.Set<IHttpRequestLifetimeFeature>(new TestHttpRequestLifetimeFeature());
-            
+
             httpContext.RequestServices = serviceCollection.BuildServiceProvider();
 
             var requestDelegate = RequestDelegateFactory.Create(TestAction);
@@ -1516,7 +1572,10 @@ public async Task RequestDelegateHandlesRouteParamOptionality(Delegate @delegate
             serviceCollection.AddSingleton(LoggerFactory);
             httpContext.RequestServices = serviceCollection.BuildServiceProvider();
 
-            var requestDelegate = RequestDelegateFactory.Create(@delegate);
+            var requestDelegate = RequestDelegateFactory.Create(@delegate, new()
+            {
+                RouteParameterNames = routeParam is not null ? new[] { paramName } : Array.Empty<string>()
+            });
 
             await requestDelegate(httpContext);
 
@@ -1567,7 +1626,7 @@ public async Task RequestDelegateHandlesBodyParamOptionality(Delegate @delegate,
             var httpContext = new DefaultHttpContext();
             var responseBodyStream = new MemoryStream();
             httpContext.Response.Body = responseBodyStream;
-            
+
             if (hasBody)
             {
                 var todo = new Todo() { Name = "Default Todo" };
@@ -1699,7 +1758,7 @@ public async Task AllowEmptyOverridesOptionality(Delegate @delegate, bool allows
             await requestDelegate(httpContext);
 
             var logs = TestSink.Writes.ToArray();
-            
+
             if (!allowsEmptyRequest)
             {
                 Assert.Equal(400, httpContext.Response.StatusCode);

src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs

@@ -141,6 +141,46 @@ public async Task MapGetWithRouteParameter_BuildsEndpointWithRouteSpecificBindin
             Assert.Null(httpContext.Items["input"]);
         }
 
+        [Fact]
+        public async Task MapGetWithoutRouteParameter_BuildsEndpointWithQuerySpecificBinding()
+        {
+            var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvdier()));
+            _ = builder.MapGet("/", (int? id, HttpContext httpContext) =>
+            {
+                if (id is not null)
+                {
+                    httpContext.Items["input"] = id;
+                }
+            });
+
+            var dataSource = GetBuilderEndpointDataSource(builder);
+            // Trigger Endpoint build by calling getter.
+            var endpoint = Assert.Single(dataSource.Endpoints);
+
+            var methodMetadata = endpoint.Metadata.GetMetadata<IHttpMethodMetadata>();
+            Assert.NotNull(methodMetadata);
+            var method = Assert.Single(methodMetadata!.HttpMethods);
+            Assert.Equal("GET", method);
+
+            var routeEndpointBuilder = GetRouteEndpointBuilder(builder);
+            Assert.Equal("/ HTTP: GET", routeEndpointBuilder.DisplayName);
+            Assert.Equal("/", routeEndpointBuilder.RoutePattern.RawText);
+
+            // Assert that we don't fallback to the route values
+            var httpContext = new DefaultHttpContext();
+
+            httpContext.Request.Query = new QueryCollection(new Dictionary<string, StringValues>()
+            {
+                ["id"] = "41"
+            });
+            httpContext.Request.RouteValues = new();
+            httpContext.Request.RouteValues["id"] = "42";
+
+            await endpoint.RequestDelegate!(httpContext);
+
+            Assert.Equal(41, httpContext.Items["input"]);
+        }
+
         [Theory]
         [MemberData(nameof(MapMethods))]
         public async Task MapVerbWithExplicitRouteParameterIsCaseInsensitive(Action<IEndpointRouteBuilder, string, Delegate> map, string expectedMethod)