From bafe66c7eb76556f1e0bc0255012c50ddcfa8834 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Thu, 5 Aug 2021 20:56:36 -0700 Subject: [PATCH] Make sure route binding isn't case sensitive - For error handling cases, we were doing a case sensitive match against route parameters instead of a case insensitive one. - Added tests --- .../src/RequestDelegateFactory.cs | 4 +- ...ctionEndpointRouteBuilderExtensionsTest.cs | 102 ++++++++++++++++++ 2 files changed, 104 insertions(+), 2 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index e658ab7caa8d..6840901b970f 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -208,7 +208,7 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext if (parameterCustomAttributes.OfType().FirstOrDefault() is { } routeAttribute) { - if (factoryContext.RouteParameters is { } routeParams && !routeParams.Contains(parameter.Name)) + if (factoryContext.RouteParameters is { } routeParams && !routeParams.Contains(parameter.Name, StringComparer.OrdinalIgnoreCase)) { throw new InvalidOperationException($"{parameter.Name} is not a route paramter."); } @@ -255,7 +255,7 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext { // 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)) + if (factoryContext.RouteParameters is { } routeParams && routeParams.Contains(parameter.Name, StringComparer.OrdinalIgnoreCase)) { return BindParameterFromProperty(parameter, RouteValuesExpr, parameter.Name, factoryContext); } diff --git a/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs index 9bfa0fa0bb71..d480406e475b 100644 --- a/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs @@ -29,6 +29,32 @@ private RouteEndpointBuilder GetRouteEndpointBuilder(IEndpointRouteBuilder endpo return Assert.IsType(Assert.Single(GetBuilderEndpointDataSource(endpointRouteBuilder).EndpointBuilders)); } + public static object[][] MapMethods + { + get + { + void MapGet(IEndpointRouteBuilder routes, string template, Delegate action) => + routes.MapGet(template, action); + + void MapPost(IEndpointRouteBuilder routes, string template, Delegate action) => + routes.MapPost(template, action); + + void MapPut(IEndpointRouteBuilder routes, string template, Delegate action) => + routes.MapPut(template, action); + + void MapDelete(IEndpointRouteBuilder routes, string template, Delegate action) => + routes.MapDelete(template, action); + + return new object[][] + { + new object[] { (Action)MapGet, "GET" }, + new object[] { (Action)MapPost, "POST" }, + new object[] { (Action)MapPut, "PUT" }, + new object[] { (Action)MapDelete, "DELETE" }, + }; + } + } + [Fact] public void MapEndpoint_PrecedenceOfMetadata_BuilderMetadataReturned() { @@ -115,6 +141,82 @@ public async Task MapGetWithRouteParameter_BuildsEndpointWithRouteSpecificBindin Assert.Null(httpContext.Items["input"]); } + [Theory] + [MemberData(nameof(MapMethods))] + public async Task MapVerbWithExplicitRouteParameterIsCaseInsensitive(Action map, string expectedMethod) + { + var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvdier())); + + map(builder, "/{ID}", ([FromRoute] 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(); + Assert.NotNull(methodMetadata); + var method = Assert.Single(methodMetadata!.HttpMethods); + Assert.Equal(expectedMethod, method); + + var routeEndpointBuilder = GetRouteEndpointBuilder(builder); + Assert.Equal($"/{{ID}} HTTP: {expectedMethod}", routeEndpointBuilder.DisplayName); + Assert.Equal($"/{{ID}}", routeEndpointBuilder.RoutePattern.RawText); + + var httpContext = new DefaultHttpContext(); + + httpContext.Request.RouteValues["id"] = "13"; + + await endpoint.RequestDelegate!(httpContext); + + Assert.Equal(13, httpContext.Items["input"]); + } + + [Theory] + [MemberData(nameof(MapMethods))] + public async Task MapVerbWithRouteParameterDoesNotFallbackToQuery(Action map, string expectedMethod) + { + var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvdier())); + + map(builder, "/{ID}", (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(); + Assert.NotNull(methodMetadata); + var method = Assert.Single(methodMetadata!.HttpMethods); + Assert.Equal(expectedMethod, method); + + var routeEndpointBuilder = GetRouteEndpointBuilder(builder); + Assert.Equal($"/{{ID}} HTTP: {expectedMethod}", routeEndpointBuilder.DisplayName); + Assert.Equal($"/{{ID}}", routeEndpointBuilder.RoutePattern.RawText); + + // Assert that we don't fallback to the query string + var httpContext = new DefaultHttpContext(); + + httpContext.Request.Query = new QueryCollection(new Dictionary + { + ["id"] = "42" + }); + + await endpoint.RequestDelegate!(httpContext); + + Assert.Null(httpContext.Items["input"]); + } + [Fact] public void MapGetWithRouteParameter_ThrowsIfRouteParameterDoesNotExist() {