From 29168ec8e6ef0ce9c0b10a81c321ca91d890ae18 Mon Sep 17 00:00:00 2001 From: Marco Minerva Date: Thu, 22 May 2025 10:13:50 +0200 Subject: [PATCH 1/5] Enhance validation error handling in filter factory Modify validation error handling to utilize IProblemDetailsService for response writing. Fallback to default behavior if the service is unavailable. --- .../src/ValidationEndpointFilterFactory.cs | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/src/Http/Routing/src/ValidationEndpointFilterFactory.cs b/src/Http/Routing/src/ValidationEndpointFilterFactory.cs index 73a41f0f8d57..c15ebc02ee06 100644 --- a/src/Http/Routing/src/ValidationEndpointFilterFactory.cs +++ b/src/Http/Routing/src/ValidationEndpointFilterFactory.cs @@ -5,7 +5,9 @@ using System.ComponentModel.DataAnnotations; using System.Linq; +using System.Net.Mime; using System.Reflection; +using Microsoft.AspNetCore.Http.HttpResults; using Microsoft.AspNetCore.Http.Metadata; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; @@ -87,8 +89,27 @@ public static EndpointFilterDelegate Create(EndpointFilterFactoryContext context if (validateContext is { ValidationErrors.Count: > 0 }) { context.HttpContext.Response.StatusCode = StatusCodes.Status400BadRequest; - context.HttpContext.Response.ContentType = "application/problem+json"; - return await ValueTask.FromResult(new HttpValidationProblemDetails(validateContext.ValidationErrors)); + + var problemDetails = new HttpValidationProblemDetails(validateContext.ValidationErrors); + + var problemDetailsService = context.HttpContext.RequestServices.GetService(); + if (problemDetailsService is not null) + { + if (await problemDetailsService.TryWriteAsync(new() + { + HttpContext = context.HttpContext, + ProblemDetails = problemDetails + })) + { + // We need to prevent further execution, because the actual + // ProblemDetails response has already been written by ProblemDetailsService. + return await ValueTask.FromResult(EmptyHttpResult.Instance); + } + } + + // Fallback to the default implementation. + context.HttpContext.Response.ContentType = MediaTypeNames.Application.ProblemJson; + return await ValueTask.FromResult(problemDetails); } return await next(context); From 3f3131574a88bdfae1e4a81d1f97dd0520136d66 Mon Sep 17 00:00:00 2001 From: Marco Minerva Date: Thu, 22 May 2025 14:49:20 +0200 Subject: [PATCH 2/5] Add validation filter tests for ProblemDetails handling Introduces the `ValidationFilterEndpointFactoryTests` class with two unit tests. These tests validate HTTP request behavior with and without the `ProblemDetails` service registered, ensuring correct status codes, content types, and the structure of the returned `ProblemDetails` object for validation errors. --- .../ValidationFilterEndpointFactoryTests.cs | 121 ++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 src/Http/Http.Extensions/test/ValidationFilterEndpointFactoryTests.cs diff --git a/src/Http/Http.Extensions/test/ValidationFilterEndpointFactoryTests.cs b/src/Http/Http.Extensions/test/ValidationFilterEndpointFactoryTests.cs new file mode 100644 index 000000000000..4b27f30b7120 --- /dev/null +++ b/src/Http/Http.Extensions/test/ValidationFilterEndpointFactoryTests.cs @@ -0,0 +1,121 @@ +#pragma warning disable ASP0029 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed. + +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.ComponentModel.DataAnnotations; +using System.Net.Mime; +using System.Text.Json; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.InternalTesting; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.AspNetCore.Http.Extensions.Tests; + +public class ValidationFilterEndpointFactoryTests +{ + [Fact] + public async Task GetHttpValidationProblemDetailsWhenProblemDetailsServiceNotRegistered() + { + var services = new ServiceCollection(); + services.AddValidation(); + var serviceProvider = services.BuildServiceProvider(); + + var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(serviceProvider)); + + // Act - Create one endpoint with validation + builder.MapGet("test-enabled", ([Range(5, 10)] int param) => "Validation enabled here."); + + // Build the endpoints + var dataSource = Assert.Single(builder.DataSources); + var endpoints = dataSource.Endpoints; + + // Get filter factories from endpoint + var endpoint = endpoints[0]; + + var context = new DefaultHttpContext + { + RequestServices = serviceProvider + }; + + context.Request.Method = "GET"; + context.Request.QueryString = new QueryString("?param=15"); + using var ms = new MemoryStream(); + context.Response.Body = ms; + + await endpoint.RequestDelegate(context); + + // Assert + Assert.Equal(StatusCodes.Status400BadRequest, context.Response.StatusCode); + Assert.StartsWith(MediaTypeNames.Application.Json, context.Response.ContentType, StringComparison.OrdinalIgnoreCase); + + ms.Seek(0, SeekOrigin.Begin); + var problemDetails = await JsonSerializer.DeserializeAsync(ms, JsonSerializerOptions.Web); + + Assert.Equal("One or more validation errors occurred.", problemDetails.Title); + + // Check that ProblemDetails contains the errors object with 1 validation error + Assert.True(problemDetails.Extensions.TryGetValue("errors", out var errorsObj)); + var errors = Assert.IsType(errorsObj); + Assert.True(errors.EnumerateObject().Count() == 1); + } + + [Fact] + public async Task UseProblemDetailsServiceWhenAddedInServiceCollection() + { + var services = new ServiceCollection(); + services.AddValidation(); + services.AddProblemDetails(); + var serviceProvider = services.BuildServiceProvider(); + + var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(serviceProvider)); + + // Act - Create one endpoint with validation + builder.MapGet("test-enabled", ([Range(5, 10)] int param) => "Validation enabled here."); + + // Build the endpoints + var dataSource = Assert.Single(builder.DataSources); + var endpoints = dataSource.Endpoints; + + // Get filter factories from endpoint + var endpoint = endpoints[0]; + + var context = new DefaultHttpContext + { + RequestServices = serviceProvider + }; + + context.Request.Method = "GET"; + context.Request.QueryString = new QueryString("?param=15"); + using var ms = new MemoryStream(); + context.Response.Body = ms; + + await endpoint.RequestDelegate(context); + + // Assert + Assert.Equal(StatusCodes.Status400BadRequest, context.Response.StatusCode); + Assert.StartsWith(MediaTypeNames.Application.ProblemJson, context.Response.ContentType, StringComparison.OrdinalIgnoreCase); + + ms.Seek(0, SeekOrigin.Begin); + var problemDetails = await JsonSerializer.DeserializeAsync(ms, JsonSerializerOptions.Web); + + Assert.Equal("https://tools.ietf.org/html/rfc9110#section-15.5.1", problemDetails.Type); + Assert.Equal("One or more validation errors occurred.", problemDetails.Title); + Assert.Equal(StatusCodes.Status400BadRequest, problemDetails.Status); + + // Check that ProblemDetails contains the errors object with 1 validation error + Assert.True(problemDetails.Extensions.TryGetValue("errors", out var errorsObj)); + var errors = Assert.IsType(errorsObj); + Assert.True(errors.EnumerateObject().Count() == 1); + } + + private class DefaultEndpointRouteBuilder(IApplicationBuilder applicationBuilder) : IEndpointRouteBuilder + { + private IApplicationBuilder ApplicationBuilder { get; } = applicationBuilder ?? throw new ArgumentNullException(nameof(applicationBuilder)); + public IApplicationBuilder CreateApplicationBuilder() => ApplicationBuilder.New(); + public ICollection DataSources { get; } = []; + public IServiceProvider ServiceProvider => ApplicationBuilder.ApplicationServices; + } +} From 5288c3a619aa8b9c2fe42b5ca4b3887d48450c00 Mon Sep 17 00:00:00 2001 From: Marco Minerva Date: Thu, 22 May 2025 16:00:56 +0200 Subject: [PATCH 3/5] Fix a typo Rename validation test class for clarity --- .../test/ValidationFilterEndpointFactoryTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Http/Http.Extensions/test/ValidationFilterEndpointFactoryTests.cs b/src/Http/Http.Extensions/test/ValidationFilterEndpointFactoryTests.cs index 4b27f30b7120..a7888b61a724 100644 --- a/src/Http/Http.Extensions/test/ValidationFilterEndpointFactoryTests.cs +++ b/src/Http/Http.Extensions/test/ValidationFilterEndpointFactoryTests.cs @@ -14,7 +14,7 @@ namespace Microsoft.AspNetCore.Http.Extensions.Tests; -public class ValidationFilterEndpointFactoryTests +public class ValidationEndpointFilterFactoryTests { [Fact] public async Task GetHttpValidationProblemDetailsWhenProblemDetailsServiceNotRegistered() From 2e46d78af7754bed48a731e3e90530c076e67077 Mon Sep 17 00:00:00 2001 From: Marco Minerva Date: Wed, 28 May 2025 11:14:23 +0200 Subject: [PATCH 4/5] Enhance validation endpoint tests Added a new test method to validate the ProblemDetails service with a timestamp extension. --- .../ValidationFilterEndpointFactoryTests.cs | 68 ++++++++++++++++++- 1 file changed, 65 insertions(+), 3 deletions(-) diff --git a/src/Http/Http.Extensions/test/ValidationFilterEndpointFactoryTests.cs b/src/Http/Http.Extensions/test/ValidationFilterEndpointFactoryTests.cs index a7888b61a724..0d9e005ac201 100644 --- a/src/Http/Http.Extensions/test/ValidationFilterEndpointFactoryTests.cs +++ b/src/Http/Http.Extensions/test/ValidationFilterEndpointFactoryTests.cs @@ -14,7 +14,7 @@ namespace Microsoft.AspNetCore.Http.Extensions.Tests; -public class ValidationEndpointFilterFactoryTests +public class ValidationEndpointFilterFactoryTests : LoggedTest { [Fact] public async Task GetHttpValidationProblemDetailsWhenProblemDetailsServiceNotRegistered() @@ -26,7 +26,7 @@ public async Task GetHttpValidationProblemDetailsWhenProblemDetailsServiceNotReg var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(serviceProvider)); // Act - Create one endpoint with validation - builder.MapGet("test-enabled", ([Range(5, 10)] int param) => "Validation enabled here."); + builder.MapGet("validation-test", ([Range(5, 10)] int param) => "Validation enabled here."); // Build the endpoints var dataSource = Assert.Single(builder.DataSources); @@ -73,7 +73,7 @@ public async Task UseProblemDetailsServiceWhenAddedInServiceCollection() var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(serviceProvider)); // Act - Create one endpoint with validation - builder.MapGet("test-enabled", ([Range(5, 10)] int param) => "Validation enabled here."); + builder.MapGet("validation-test", ([Range(5, 10)] int param) => "Validation enabled here."); // Build the endpoints var dataSource = Assert.Single(builder.DataSources); @@ -101,6 +101,7 @@ public async Task UseProblemDetailsServiceWhenAddedInServiceCollection() ms.Seek(0, SeekOrigin.Begin); var problemDetails = await JsonSerializer.DeserializeAsync(ms, JsonSerializerOptions.Web); + // Check if the response is an actual ProblemDetails object Assert.Equal("https://tools.ietf.org/html/rfc9110#section-15.5.1", problemDetails.Type); Assert.Equal("One or more validation errors occurred.", problemDetails.Title); Assert.Equal(StatusCodes.Status400BadRequest, problemDetails.Status); @@ -111,6 +112,67 @@ public async Task UseProblemDetailsServiceWhenAddedInServiceCollection() Assert.True(errors.EnumerateObject().Count() == 1); } + [Fact] + public async Task UseProblemDetailsServiceWithCallbackWhenAddedInServiceCollection() + { + var services = new ServiceCollection(); + services.AddValidation(); + + services.AddProblemDetails(options => + { + options.CustomizeProblemDetails = context => + { + context.ProblemDetails.Extensions.Add("timestamp", DateTimeOffset.Now); + }; + }); + + var serviceProvider = services.BuildServiceProvider(); + + var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(serviceProvider)); + + // Act - Create one endpoint with validation + builder.MapGet("validation-test", ([Range(5, 10)] int param) => "Validation enabled here."); + + // Build the endpoints + var dataSource = Assert.Single(builder.DataSources); + var endpoints = dataSource.Endpoints; + + // Get filter factories from endpoint + var endpoint = endpoints[0]; + + var context = new DefaultHttpContext + { + RequestServices = serviceProvider + }; + + context.Request.Method = "GET"; + context.Request.QueryString = new QueryString("?param=15"); + using var ms = new MemoryStream(); + context.Response.Body = ms; + + await endpoint.RequestDelegate(context); + + // Assert + Assert.Equal(StatusCodes.Status400BadRequest, context.Response.StatusCode); + Assert.StartsWith(MediaTypeNames.Application.ProblemJson, context.Response.ContentType, StringComparison.OrdinalIgnoreCase); + + ms.Seek(0, SeekOrigin.Begin); + var problemDetails = await JsonSerializer.DeserializeAsync(ms, JsonSerializerOptions.Web); + + // Check if the response is an actual ProblemDetails object + Assert.Equal("https://tools.ietf.org/html/rfc9110#section-15.5.1", problemDetails.Type); + Assert.Equal("One or more validation errors occurred.", problemDetails.Title); + Assert.Equal(StatusCodes.Status400BadRequest, problemDetails.Status); + + // Check that ProblemDetails contains the errors object with 1 validation error + Assert.True(problemDetails.Extensions.TryGetValue("errors", out var errorsObj)); + var errors = Assert.IsType(errorsObj); + Assert.True(errors.EnumerateObject().Count() == 1); + + // Check that ProblemDetails customizations are applied in the response + Assert.True(problemDetails.Extensions.ContainsKey("timestamp")); + } + private class DefaultEndpointRouteBuilder(IApplicationBuilder applicationBuilder) : IEndpointRouteBuilder { private IApplicationBuilder ApplicationBuilder { get; } = applicationBuilder ?? throw new ArgumentNullException(nameof(applicationBuilder)); From 83f30b6a98570237ccc1480889e29fd88a4be1c4 Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Wed, 28 May 2025 10:31:48 -0700 Subject: [PATCH 5/5] Apply suggestions from code review --- src/Http/Routing/src/ValidationEndpointFilterFactory.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Http/Routing/src/ValidationEndpointFilterFactory.cs b/src/Http/Routing/src/ValidationEndpointFilterFactory.cs index c15ebc02ee06..e24004afacff 100644 --- a/src/Http/Routing/src/ValidationEndpointFilterFactory.cs +++ b/src/Http/Routing/src/ValidationEndpointFilterFactory.cs @@ -103,13 +103,13 @@ public static EndpointFilterDelegate Create(EndpointFilterFactoryContext context { // We need to prevent further execution, because the actual // ProblemDetails response has already been written by ProblemDetailsService. - return await ValueTask.FromResult(EmptyHttpResult.Instance); + return EmptyHttpResult.Instance; } } // Fallback to the default implementation. context.HttpContext.Response.ContentType = MediaTypeNames.Application.ProblemJson; - return await ValueTask.FromResult(problemDetails); + return problemDetails; } return await next(context);