diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index c51e7aae5a36..344966e60ef4 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -993,41 +993,31 @@ private static Expression CreateParamCheckingResponseWritingMethodCall(Type retu localVariables[factoryContext.ExtraLocals.Count] = WasParamCheckFailureExpr; - // If filters have been registered, we set the `wasParamCheckFailure` property - // but do not return from the invocation to allow the filters to run. - if (factoryContext.EndpointBuilder.FilterFactories.Count > 0) - { - // if (wasParamCheckFailure) - // { - // httpContext.Response.StatusCode = 400; - // } - // return RequestDelegateFactory.ExecuteObjectReturn(invocationPipeline.Invoke(context) as object); - var checkWasParamCheckFailureWithFilters = Expression.Block( - Expression.IfThen( - WasParamCheckFailureExpr, - Expression.Assign(StatusCodeExpr, Expression.Constant(400))), - AddResponseWritingToMethodCall(factoryContext.MethodCall!, returnType, factoryContext) - ); - - checkParamAndCallMethod[factoryContext.ParamCheckExpressions.Count] = checkWasParamCheckFailureWithFilters; + // On parameter binding failure: + // - If filters are present (pipeline built -> returnType ValueTask), run the filter pipeline so that + // filters can observe/modify the 400 response. The handler itself will be skipped by the StatusCode >= 400 + // check embedded in the pipeline (see CreateFilterPipeline), allowing filters to customize the error. + // - If no filters are present, just set 400 and short-circuit. + Expression failureBranch; + if (factoryContext.EndpointBuilder.FilterFactories.Count > 0 && returnType == typeof(ValueTask)) + { + failureBranch = Expression.Block( + Expression.Assign(StatusCodeExpr, Expression.Constant(400)), + AddResponseWritingToMethodCall(factoryContext.MethodCall!, returnType, factoryContext)); } else { - // wasParamCheckFailure ? { - // httpContext.Response.StatusCode = 400; - // return Task.CompletedTask; - // } : { - // return RequestDelegateFactory.ExecuteObjectReturn(invocationPipeline.Invoke(context) as object); - // } - var checkWasParamCheckFailure = Expression.Condition( - WasParamCheckFailureExpr, - Expression.Block( - Expression.Assign(StatusCodeExpr, Expression.Constant(400)), - CompletedTaskExpr), - AddResponseWritingToMethodCall(factoryContext.MethodCall!, returnType, factoryContext)); - checkParamAndCallMethod[factoryContext.ParamCheckExpressions.Count] = checkWasParamCheckFailure; + failureBranch = Expression.Block( + Expression.Assign(StatusCodeExpr, Expression.Constant(400)), + CompletedTaskExpr); } + var checkWasParamCheckFailure = Expression.Condition( + WasParamCheckFailureExpr, + failureBranch, + AddResponseWritingToMethodCall(factoryContext.MethodCall!, returnType, factoryContext)); + checkParamAndCallMethod[factoryContext.ParamCheckExpressions.Count] = checkWasParamCheckFailure; + return Expression.Block(localVariables, checkParamAndCallMethod); } diff --git a/src/Http/Routing/test/UnitTests/Builder/RouteHandlerEndpointRouteBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/RouteHandlerEndpointRouteBuilderExtensionsTest.cs index 5f177b2afecc..76dd9ffc0413 100644 --- a/src/Http/Routing/test/UnitTests/Builder/RouteHandlerEndpointRouteBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/RouteHandlerEndpointRouteBuilderExtensionsTest.cs @@ -1066,6 +1066,164 @@ public void FinallyOnNestedGroups_OuterGroupCanExamineInnerGroup() Assert.Equal(new[] { "added-from-endpoint-1", "added-from-endpoint-2", "added-from-inner-group", "added-from-outer-group" }, endpoint.Metadata.GetOrderedMetadata()); } + [Fact] + public async Task ParameterBindingFailure_WithEndpointFilterFactory_DoesNotExecuteHandler() + { + // Arrange + var services = new ServiceCollection().AddSingleton(LoggerFactory); + var serviceProvider = services.BuildServiceProvider(); + + var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(serviceProvider)); + var handlerExecuted = false; + + // Act - Add an endpoint filter factory that returns next as-is + builder.MapGet("/test/{id}", (Guid id) => + { + handlerExecuted = true; + return 1; + }).AddEndpointFilterFactory((_, next) => next); + + var dataSource = GetBuilderEndpointDataSource(builder); + var endpoint = Assert.Single(dataSource.Endpoints); + + var httpContext = new DefaultHttpContext + { + RequestServices = serviceProvider + }; + httpContext.Request.RouteValues["id"] = "invalid-guid"; + + // Act + await endpoint.RequestDelegate!(httpContext); + + // Assert + Assert.Equal(StatusCodes.Status400BadRequest, httpContext.Response.StatusCode); + Assert.True(httpContext.Response.Body.Length == 0, "Response body should be empty"); + Assert.False(handlerExecuted, "Handler should not have been executed when parameter binding fails"); + } + + [Fact] + public async Task ParameterBindingFailure_WithoutFilter_DoesNotExecuteHandler() + { + // Arrange + var services = new ServiceCollection().AddSingleton(LoggerFactory); + var serviceProvider = services.BuildServiceProvider(); + + var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(serviceProvider)); + var handlerExecuted = false; + + // Act + builder.MapGet("/test/{id}", (Guid id) => + { + handlerExecuted = true; + return 1; + }); + + var dataSource = GetBuilderEndpointDataSource(builder); + var endpoint = Assert.Single(dataSource.Endpoints); + + var httpContext = new DefaultHttpContext + { + RequestServices = serviceProvider + }; + httpContext.Request.RouteValues["id"] = "invalid-guid"; + + // Act + await endpoint.RequestDelegate!(httpContext); + + // Assert + Assert.Equal(StatusCodes.Status400BadRequest, httpContext.Response.StatusCode); + Assert.True(httpContext.Response.Body.Length == 0, "Response body should be empty"); + Assert.False(handlerExecuted, "Handler should not have been executed when parameter binding fails"); + } + + [Fact] + public async Task ParameterBindingFailure_WithFilterFactoryGeneratingResponse_WritesBody() + { + // Arrange + var services = new ServiceCollection().AddSingleton(LoggerFactory); + var serviceProvider = services.BuildServiceProvider(); + + var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(serviceProvider)); + var handlerExecuted = false; + + // Act - Filter explicitly generates custom response for 400 errors + builder.MapGet("/test/{id}", (Guid id) => + { + handlerExecuted = true; + return 1; + }).AddEndpointFilterFactory((_, next) => async (context) => + { + if (context.HttpContext.Response.StatusCode == StatusCodes.Status400BadRequest) + { + // Return a simple custom error message + return Results.Json(new { error = "Parameter binding failed" }, statusCode: StatusCodes.Status400BadRequest); + } + + return await next(context); + }); + + var dataSource = GetBuilderEndpointDataSource(builder); + var endpoint = Assert.Single(dataSource.Endpoints); + + var httpContext = new DefaultHttpContext + { + RequestServices = serviceProvider + }; + httpContext.Request.RouteValues["id"] = "invalid-guid"; + using var responseBody = new MemoryStream(); + httpContext.Response.Body = responseBody; + + // Act + await endpoint.RequestDelegate!(httpContext); + + // Assert + Assert.Equal(StatusCodes.Status400BadRequest, httpContext.Response.StatusCode); + Assert.False(handlerExecuted, "Handler should not have been executed when parameter binding fails"); + + // Filter generated JSON error, so body should contain JSON. + responseBody.Seek(0, SeekOrigin.Begin); + using var reader = new StreamReader(responseBody); + var body = await reader.ReadToEndAsync(); + Assert.NotEmpty(body); + Assert.Contains("Parameter binding failed", body); + } + + [Fact] + public async Task ParameterBindingFailure_WithAddValidation_DoesNotExecuteHandler() + { + // Arrange + var services = new ServiceCollection().AddSingleton(LoggerFactory); + services.AddValidation(); // Register validation filter factory + var serviceProvider = services.BuildServiceProvider(); + + var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(serviceProvider)); + var handlerExecuted = false; + + // Act - Binding failure happens BEFORE validation filter runs + builder.MapGet("/test/{id}", (Guid id) => + { + handlerExecuted = true; + return 1; + }); + + var dataSource = GetBuilderEndpointDataSource(builder); + var endpoint = Assert.Single(dataSource.Endpoints); + + var httpContext = new DefaultHttpContext + { + RequestServices = serviceProvider + }; + httpContext.Request.RouteValues["id"] = "invalid-guid"; + + // Act + await endpoint.RequestDelegate!(httpContext); + + // Assert + Assert.Equal(StatusCodes.Status400BadRequest, httpContext.Response.StatusCode); + Assert.True(httpContext.Response.Body.Length == 0, "Response body should be empty"); + Assert.False(handlerExecuted, "Handler should not have been executed when parameter binding fails"); + } + class MyService { } class ServiceAccessingEndpointFilter : IEndpointFilter