diff --git a/src/Http/Http.Abstractions/src/ProblemDetails/ProblemDetails.cs b/src/Http/Http.Abstractions/src/ProblemDetails/ProblemDetails.cs index 2d01289cdf19..582154c971a8 100644 --- a/src/Http/Http.Abstractions/src/ProblemDetails/ProblemDetails.cs +++ b/src/Http/Http.Abstractions/src/ProblemDetails/ProblemDetails.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Diagnostics.CodeAnalysis; using System.Text.Json.Serialization; using Microsoft.AspNetCore.Http; @@ -12,6 +13,8 @@ namespace Microsoft.AspNetCore.Mvc; [JsonConverter(typeof(ProblemDetailsJsonConverter))] public class ProblemDetails { + private readonly IDictionary _extensions = new Dictionary(StringComparer.Ordinal); + /// /// A URI reference [RFC3986] that identifies the problem type. This specification encourages that, when /// dereferenced, it provide human-readable documentation for the problem type @@ -59,5 +62,10 @@ public class ProblemDetails /// In particular, complex types or collection types may not round-trip to the original type when using the built-in JSON or XML formatters. /// [JsonExtensionData] - public IDictionary Extensions { get; } = new Dictionary(StringComparer.Ordinal); + public IDictionary Extensions + { + [RequiresUnreferencedCode("JSON serialization and deserialization of ProblemDetails.Extensions might require types that cannot be statically analyzed.")] + [RequiresDynamicCode("JSON serialization and deserialization of ProblemDetails.Extensions might require types that cannot be statically analyzed.")] + get => _extensions; + } } diff --git a/src/Http/Http.Abstractions/test/HttpValidationProblemDetailsJsonConverterTest.cs b/src/Http/Http.Abstractions/test/HttpValidationProblemDetailsJsonConverterTest.cs index 06ca5f330c07..838bcd836243 100644 --- a/src/Http/Http.Abstractions/test/HttpValidationProblemDetailsJsonConverterTest.cs +++ b/src/Http/Http.Abstractions/test/HttpValidationProblemDetailsJsonConverterTest.cs @@ -11,6 +11,40 @@ public class HttpValidationProblemDetailsJsonConverterTest { private static JsonSerializerOptions JsonSerializerOptions => new JsonOptions().SerializerOptions; + [Fact] + public void Write_Works() + { + var converter = new HttpValidationProblemDetailsJsonConverter(); + var problemDetails = new HttpValidationProblemDetails(); + + problemDetails.Type = "https://tools.ietf.org/html/rfc9110#section-15.5.5"; + problemDetails.Title = "Not found"; + problemDetails.Status = 404; + problemDetails.Detail = "Product not found"; + problemDetails.Instance = "http://example.com/products/14"; + problemDetails.Extensions["traceId"] = "|37dd3dd5-4a9619f953c40a16."; + problemDetails.Errors.Add("key0", new[] { "error0" }); + problemDetails.Errors.Add("key1", new[] { "error1", "error2" }); + + var ms = new MemoryStream(); + var writer = new Utf8JsonWriter(ms); + converter.Write(writer, problemDetails, JsonSerializerOptions); + writer.Flush(); + + ms.Seek(0, SeekOrigin.Begin); + var document = JsonDocument.Parse(ms); + Assert.Equal(problemDetails.Type, document.RootElement.GetProperty("type").GetString()); + Assert.Equal(problemDetails.Title, document.RootElement.GetProperty("title").GetString()); + Assert.Equal(problemDetails.Status, document.RootElement.GetProperty("status").GetInt32()); + Assert.Equal(problemDetails.Detail, document.RootElement.GetProperty("detail").GetString()); + Assert.Equal(problemDetails.Instance, document.RootElement.GetProperty("instance").GetString()); + Assert.Equal((string)problemDetails.Extensions["traceId"]!, document.RootElement.GetProperty("traceId").GetString()); + var errorsElement = document.RootElement.GetProperty("errors"); + Assert.Equal("error0", errorsElement.GetProperty("key0")[0].GetString()); + Assert.Equal("error1", errorsElement.GetProperty("key1")[0].GetString()); + Assert.Equal("error2", errorsElement.GetProperty("key1")[1].GetString()); + } + [Fact] public void Read_Works() { diff --git a/src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs b/src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs index 719926b951e4..e3cd65db380e 100644 --- a/src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs +++ b/src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; +using System.Text.Json; using System.Text.Json.Serialization; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Options; @@ -43,20 +45,20 @@ public bool CanWrite(ProblemDetailsContext context) } [UnconditionalSuppressMessage("Trimming", "IL2026", - Justification = "JSON serialization of ProblemDetails.Extensions might require types that cannot be statically analyzed and we need to fallback" + - "to reflection-based. The ProblemDetailsConverter is marked as RequiresUnreferencedCode already.")] + Justification = "JSON serialization of ProblemDetails.Extensions might require types that cannot be statically analyzed. The property is annotated with a warning")] [UnconditionalSuppressMessage("Trimming", "IL3050", - Justification = "JSON serialization of ProblemDetails.Extensions might require types that cannot be statically analyzed and we need to fallback" + - "to reflection-based. The ProblemDetailsConverter is marked as RequiresDynamicCode already.")] + Justification = "JSON serialization of ProblemDetails.Extensions might require types that cannot be statically analyzed. The property is annotated with a warning")] public ValueTask WriteAsync(ProblemDetailsContext context) { var httpContext = context.HttpContext; ProblemDetailsDefaults.Apply(context.ProblemDetails, httpContext.Response.StatusCode); _options.CustomizeProblemDetails?.Invoke(context); - if (context.ProblemDetails.Extensions is { Count: 0 }) + // Use source generation serialization in two scenarios: + // 1. There are no extensions. Source generation is faster and works well with trimming. + // 2. Native AOT. In this case only the data types specified on ProblemDetailsJsonContext will work. + if (context.ProblemDetails.Extensions is { Count: 0 } || !RuntimeFeature.IsDynamicCodeSupported) { - // We can use the source generation in this case return new ValueTask(httpContext.Response.WriteAsJsonAsync( context.ProblemDetails, ProblemDetailsJsonContext.Default.ProblemDetails, @@ -69,7 +71,22 @@ public ValueTask WriteAsync(ProblemDetailsContext context) contentType: "application/problem+json")); } + // Additional values are specified on JsonSerializerContext to support some values for extensions. + // For example, the DeveloperExceptionMiddleware serializes its complex type to JsonElement, which problem details then needs to serialize. [JsonSerializable(typeof(ProblemDetails))] + [JsonSerializable(typeof(JsonElement))] + [JsonSerializable(typeof(string))] + [JsonSerializable(typeof(decimal))] + [JsonSerializable(typeof(float))] + [JsonSerializable(typeof(double))] + [JsonSerializable(typeof(int))] + [JsonSerializable(typeof(long))] + [JsonSerializable(typeof(Guid))] + [JsonSerializable(typeof(Uri))] + [JsonSerializable(typeof(TimeSpan))] + [JsonSerializable(typeof(DateTime))] + [JsonSerializable(typeof(DateTimeOffset))] internal sealed partial class ProblemDetailsJsonContext : JsonSerializerContext - { } + { + } } diff --git a/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs b/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs index a3bb30343b05..1d0b4b33d979 100644 --- a/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs +++ b/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs @@ -5,11 +5,14 @@ using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Text; +using System.Text.Json; +using System.Text.Json.Serialization; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Diagnostics.RazorViews; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Http.Json; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.FileProviders; @@ -34,6 +37,7 @@ internal class DeveloperExceptionPageMiddlewareImpl private readonly ExceptionDetailsProvider _exceptionDetailsProvider; private readonly Func _exceptionHandler; private static readonly MediaTypeHeaderValue _textHtmlMediaType = new MediaTypeHeaderValue("text/html"); + private readonly ExtensionsExceptionJsonContext _serializationContext; private readonly IProblemDetailsService? _problemDetailsService; /// @@ -45,6 +49,7 @@ internal class DeveloperExceptionPageMiddlewareImpl /// /// The used for writing diagnostic messages. /// The list of registered . + /// The used for serialization. /// The used for writing messages. public DeveloperExceptionPageMiddlewareImpl( RequestDelegate next, @@ -53,6 +58,7 @@ public DeveloperExceptionPageMiddlewareImpl( IWebHostEnvironment hostingEnvironment, DiagnosticSource diagnosticSource, IEnumerable filters, + IOptions? jsonOptions = null, IProblemDetailsService? problemDetailsService = null) { if (next == null) @@ -77,8 +83,8 @@ public DeveloperExceptionPageMiddlewareImpl( _diagnosticSource = diagnosticSource; _exceptionDetailsProvider = new ExceptionDetailsProvider(_fileProvider, _logger, _options.SourceCodeLineCount); _exceptionHandler = DisplayException; + _serializationContext = CreateSerializationContext(jsonOptions?.Value); _problemDetailsService = problemDetailsService; - foreach (var filter in filters.Reverse()) { var nextFilter = _exceptionHandler; @@ -86,6 +92,13 @@ public DeveloperExceptionPageMiddlewareImpl( } } + private static ExtensionsExceptionJsonContext CreateSerializationContext(JsonOptions? jsonOptions) + { + // Create context from configured options to get settings such as PropertyNamePolicy and DictionaryKeyPolicy. + jsonOptions ??= new JsonOptions(); + return new ExtensionsExceptionJsonContext(new JsonSerializerOptions(jsonOptions.SerializerOptions)); + } + /// /// Process an individual request. /// @@ -172,21 +185,7 @@ private async Task DisplayExceptionContent(ErrorContext errorContext) if (_problemDetailsService != null) { - var problemDetails = new ProblemDetails - { - Title = TypeNameHelper.GetTypeDisplayName(errorContext.Exception.GetType()), - Detail = errorContext.Exception.Message, - Status = httpContext.Response.StatusCode - }; - - problemDetails.Extensions["exception"] = new - { - Details = errorContext.Exception.ToString(), - Headers = httpContext.Request.Headers, - Path = httpContext.Request.Path.ToString(), - Endpoint = httpContext.GetEndpoint()?.ToString(), - RouteValues = httpContext.Features.Get()?.RouteValues, - }; + var problemDetails = CreateProblemDetails(errorContext, httpContext); await _problemDetailsService.WriteAsync(new() { @@ -214,6 +213,31 @@ await _problemDetailsService.WriteAsync(new() } } + [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "Values set on ProblemDetails.Extensions are supported by the default writer.")] + [UnconditionalSuppressMessage("AOT", "IL3050", Justification = "Values set on ProblemDetails.Extensions are supported by the default writer.")] + private ProblemDetails CreateProblemDetails(ErrorContext errorContext, HttpContext httpContext) + { + var problemDetails = new ProblemDetails + { + Title = TypeNameHelper.GetTypeDisplayName(errorContext.Exception.GetType()), + Detail = errorContext.Exception.Message, + Status = httpContext.Response.StatusCode + }; + + // Problem details source gen serialization doesn't know about IHeaderDictionary or RouteValueDictionary. + // Serialize payload to a JsonElement here. Problem details serialization can write JsonElement in extensions dictionary. + problemDetails.Extensions["exception"] = JsonSerializer.SerializeToElement(new ExceptionExtensionData + ( + Details: errorContext.Exception.ToString(), + Headers: httpContext.Request.Headers, + Path: httpContext.Request.Path.ToString(), + Endpoint: httpContext.GetEndpoint()?.ToString(), + RouteValues: httpContext.Features.Get()?.RouteValues + ), _serializationContext.ExceptionExtensionData); + + return problemDetails; + } + private Task DisplayCompilationException( HttpContext context, ICompilationException compilationException) @@ -328,3 +352,10 @@ private Task DisplayRuntimeException(HttpContext context, Exception ex) return errorPage.ExecuteAsync(context); } } + +internal record ExceptionExtensionData(string Details, IHeaderDictionary Headers, string Path, string? Endpoint, RouteValueDictionary? RouteValues); + +[JsonSerializable(typeof(ExceptionExtensionData))] +internal sealed partial class ExtensionsExceptionJsonContext : JsonSerializerContext +{ +} diff --git a/src/Middleware/Diagnostics/test/FunctionalTests/DeveloperExceptionPageSampleTest.cs b/src/Middleware/Diagnostics/test/FunctionalTests/DeveloperExceptionPageSampleTest.cs index 25770a6d51a1..ba9a28039657 100644 --- a/src/Middleware/Diagnostics/test/FunctionalTests/DeveloperExceptionPageSampleTest.cs +++ b/src/Middleware/Diagnostics/test/FunctionalTests/DeveloperExceptionPageSampleTest.cs @@ -6,6 +6,7 @@ using System.Net.Http.Headers; using Microsoft.AspNetCore.Mvc; using System.Net.Http.Json; +using System.Text.Json; namespace Microsoft.AspNetCore.Diagnostics.FunctionalTests; @@ -49,5 +50,14 @@ public async Task DeveloperExceptionPage_ShowsProblemDetails_WhenHtmlNotAccepted Assert.NotNull(body); Assert.Equal(500, body.Status); Assert.Contains("Demonstration exception", body.Detail); + + var exceptionNode = (JsonElement)body.Extensions["exception"]; + Assert.Contains("System.Exception: Demonstration exception.", exceptionNode.GetProperty("details").GetString()); + Assert.Equal("application/json", exceptionNode.GetProperty("headers").GetProperty("Accept")[0].GetString()); + Assert.Equal("localhost", exceptionNode.GetProperty("headers").GetProperty("Host")[0].GetString()); + Assert.Equal("/", exceptionNode.GetProperty("path").GetString()); + Assert.Equal("Endpoint display name", exceptionNode.GetProperty("endpoint").GetString()); + Assert.Equal("Value1", exceptionNode.GetProperty("routeValues").GetProperty("routeValue1").GetString()); + Assert.Equal("Value2", exceptionNode.GetProperty("routeValues").GetProperty("routeValue2").GetString()); } } diff --git a/src/Mvc/Mvc.Core/test/Infrastructure/ValidationProblemDetailsJsonConverterTest.cs b/src/Mvc/Mvc.Core/test/Infrastructure/ValidationProblemDetailsJsonConverterTest.cs index b7b877bf38f0..df3ef0d94cd3 100644 --- a/src/Mvc/Mvc.Core/test/Infrastructure/ValidationProblemDetailsJsonConverterTest.cs +++ b/src/Mvc/Mvc.Core/test/Infrastructure/ValidationProblemDetailsJsonConverterTest.cs @@ -148,7 +148,7 @@ public void WriteWorks() using Utf8JsonWriter writer = new(stream); // Act - converter.Write(writer, problemDetails, null); + converter.Write(writer, problemDetails, new JsonSerializerOptions()); writer.Flush(); var json = Encoding.UTF8.GetString(stream.ToArray()); diff --git a/src/Shared/ProblemDetails/HttpValidationProblemDetailsJsonConverter.cs b/src/Shared/ProblemDetails/HttpValidationProblemDetailsJsonConverter.cs index f1f3f9d2b736..d903bf99c34d 100644 --- a/src/Shared/ProblemDetails/HttpValidationProblemDetailsJsonConverter.cs +++ b/src/Shared/ProblemDetails/HttpValidationProblemDetailsJsonConverter.cs @@ -1,28 +1,21 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Diagnostics.CodeAnalysis; using System.Text.Json; using System.Text.Json.Serialization; namespace Microsoft.AspNetCore.Http; -internal sealed partial class HttpValidationProblemDetailsJsonConverter : JsonConverter +internal sealed class HttpValidationProblemDetailsJsonConverter : JsonConverter { private static readonly JsonEncodedText Errors = JsonEncodedText.Encode("errors"); - [RequiresUnreferencedCode("JSON serialization and deserialization of ProblemDetails.Extensions might require types that cannot be statically analyzed.")] - [RequiresDynamicCode("JSON serialization and deserialization of ProblemDetails.Extensions might require types that cannot be statically analyzed.")] - [UnconditionalSuppressMessage("Trimming", "IL2046:'RequiresUnreferencedCodeAttribute' annotations must match across all interface implementations or overrides.", Justification = "")] - [UnconditionalSuppressMessage("AOT", "IL3051:'RequiresDynamicCodeAttribute' annotations must match across all interface implementations or overrides.", Justification = "")] public override HttpValidationProblemDetails Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { var problemDetails = new HttpValidationProblemDetails(); return ReadProblemDetails(ref reader, options, problemDetails); } - [RequiresUnreferencedCode("JSON serialization and deserialization of ProblemDetails.Extensions might require types that cannot be statically analyzed.")] - [RequiresDynamicCode("JSON serialization and deserialization of ProblemDetails.Extensions might require types that cannot be statically analyzed.")] public static HttpValidationProblemDetails ReadProblemDetails(ref Utf8JsonReader reader, JsonSerializerOptions options, HttpValidationProblemDetails problemDetails) { if (reader.TokenType != JsonTokenType.StartObject) @@ -34,15 +27,7 @@ public static HttpValidationProblemDetails ReadProblemDetails(ref Utf8JsonReader { if (reader.ValueTextEquals(Errors.EncodedUtf8Bytes)) { - var context = new ErrorsJsonContext(options); - var errors = JsonSerializer.Deserialize(ref reader, context.DictionaryStringStringArray); - if (errors is not null) - { - foreach (var item in errors) - { - problemDetails.Errors[item.Key] = item.Value; - } - } + ReadErrors(ref reader, problemDetails.Errors); } else { @@ -56,34 +41,88 @@ public static HttpValidationProblemDetails ReadProblemDetails(ref Utf8JsonReader } return problemDetails; + + static void ReadErrors(ref Utf8JsonReader reader, IDictionary errors) + { + if (!reader.Read()) + { + throw new JsonException("Unexpected end when reading JSON."); + } + + switch (reader.TokenType) + { + case JsonTokenType.StartObject: + while (reader.Read() && reader.TokenType != JsonTokenType.EndObject) + { + var name = reader.GetString()!; + + if (!reader.Read()) + { + throw new JsonException("Unexpected end when reading JSON."); + } + + if (reader.TokenType == JsonTokenType.Null) + { + errors[name] = null!; + } + else + { + var values = new List(); + while (reader.Read() && reader.TokenType != JsonTokenType.EndArray) + { + values.Add(reader.GetString()!); + } + errors[name] = values.ToArray(); + } + } + break; + case JsonTokenType.Null: + return; + default: + throw new JsonException($"Unexpected token when reading errors: {reader.TokenType}"); + } + } } - [RequiresUnreferencedCode("JSON serialization and deserialization of ProblemDetails.Extensions might require types that cannot be statically analyzed.")] - [RequiresDynamicCode("JSON serialization and deserialization of ProblemDetails.Extensions might require types that cannot be statically analyzed.")] - [UnconditionalSuppressMessage("Trimming", "IL2046:'RequiresUnreferencedCodeAttribute' annotations must match across all interface implementations or overrides.", Justification = "")] - [UnconditionalSuppressMessage("AOT", "IL3051:'RequiresDynamicCodeAttribute' annotations must match across all interface implementations or overrides.", Justification = "")] public override void Write(Utf8JsonWriter writer, HttpValidationProblemDetails value, JsonSerializerOptions options) { WriteProblemDetails(writer, value, options); } - [RequiresUnreferencedCode("JSON serialization and deserialization of ProblemDetails.Extensions might require types that cannot be statically analyzed.")] - [RequiresDynamicCode("JSON serialization and deserialization of ProblemDetails.Extensions might require types that cannot be statically analyzed.")] public static void WriteProblemDetails(Utf8JsonWriter writer, HttpValidationProblemDetails value, JsonSerializerOptions options) { writer.WriteStartObject(); ProblemDetailsJsonConverter.WriteProblemDetails(writer, value, options); writer.WritePropertyName(Errors); - - var context = new ErrorsJsonContext(options); - JsonSerializer.Serialize(writer, value.Errors, context.IDictionaryStringStringArray); + WriteErrors(writer, value, options); writer.WriteEndObject(); - } - [JsonSerializable(typeof(IDictionary))] - [JsonSerializable(typeof(Dictionary))] - private sealed partial class ErrorsJsonContext : JsonSerializerContext - { } + static void WriteErrors(Utf8JsonWriter writer, HttpValidationProblemDetails value, JsonSerializerOptions options) + { + writer.WriteStartObject(); + foreach (var kvp in value.Errors) + { + var name = kvp.Key; + var errors = kvp.Value; + + writer.WritePropertyName(options.DictionaryKeyPolicy?.ConvertName(name) ?? name); + if (errors is null) + { + writer.WriteNullValue(); + } + else + { + writer.WriteStartArray(); + foreach (var error in errors) + { + writer.WriteStringValue(error); + } + writer.WriteEndArray(); + } + } + writer.WriteEndObject(); + } + } } diff --git a/src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs b/src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs index a523de3e4aad..13fa59608f2b 100644 --- a/src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs +++ b/src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs @@ -8,7 +8,7 @@ namespace Microsoft.AspNetCore.Http; -internal sealed partial class ProblemDetailsJsonConverter : JsonConverter +internal sealed class ProblemDetailsJsonConverter : JsonConverter { private static readonly JsonEncodedText Type = JsonEncodedText.Encode("type"); private static readonly JsonEncodedText Title = JsonEncodedText.Encode("title"); @@ -16,10 +16,6 @@ internal sealed partial class ProblemDetailsJsonConverter : JsonConverter