[AOT] Fixing the problems with ProblemDetails (#45646)

JamesNK · web-flow · commit 3de17af87705 · 2022-12-20T22:42:13.000+08:00
diff --git 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<string, object?> _extensions = new Dictionary<string, object?>(StringComparer.Ordinal);
+
     /// <summary>
     /// 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.
     /// </remarks>
     [JsonExtensionData]
-    public IDictionary<string, object?> Extensions { get; } = new Dictionary<string, object?>(StringComparer.Ordinal);
+    public IDictionary<string, object?> 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
@@ -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
@@ -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
@@ -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<ErrorContext, Task> _exceptionHandler;
     private static readonly MediaTypeHeaderValue _textHtmlMediaType = new MediaTypeHeaderValue("text/html");
+    private readonly ExtensionsExceptionJsonContext _serializationContext;
     private readonly IProblemDetailsService? _problemDetailsService;
 
     /// <summary>
@@ -45,6 +49,7 @@ internal class DeveloperExceptionPageMiddlewareImpl
     /// <param name="hostingEnvironment"></param>
     /// <param name="diagnosticSource">The <see cref="DiagnosticSource"/> used for writing diagnostic messages.</param>
     /// <param name="filters">The list of registered <see cref="IDeveloperPageExceptionFilter"/>.</param>
+    /// <param name="jsonOptions">The <see cref="JsonOptions"/> used for serialization.</param>
     /// <param name="problemDetailsService">The <see cref="IProblemDetailsService"/> used for writing <see cref="ProblemDetails"/> messages.</param>
     public DeveloperExceptionPageMiddlewareImpl(
         RequestDelegate next,
@@ -53,6 +58,7 @@ public DeveloperExceptionPageMiddlewareImpl(
         IWebHostEnvironment hostingEnvironment,
         DiagnosticSource diagnosticSource,
         IEnumerable<IDeveloperPageExceptionFilter> filters,
+        IOptions<JsonOptions>? jsonOptions = null,
         IProblemDetailsService? problemDetailsService = null)
     {
         if (next == null)
@@ -77,15 +83,22 @@ 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;
             _exceptionHandler = errorContext => filter.HandleExceptionAsync(errorContext, nextFilter);
         }
     }
 
+    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));
+    }
+
     /// <summary>
     /// Process an individual request.
     /// </summary>
@@ -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<IRouteValuesFeature>()?.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<IRouteValuesFeature>()?.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
@@ -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
@@ -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
diff --git a/src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs b/src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs
