Refactor param binding failure handling in RequestDelegate

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<object?>), 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<object?>))
+        {
+            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);
     }
 

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<string>());
     }
 
+    [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