From 81c060d18029da57ef498d0ab65c87e15ce1f78c Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 17 Dec 2022 21:40:59 +0800 Subject: [PATCH 1/9] [AOT] Fixing the problems with ProblemDetails --- .../src/ProblemDetails/ProblemDetails.cs | 10 +- ...lidationProblemDetailsJsonConverterTest.cs | 34 +++++++ .../src/DefaultProblemDetailsWriter.cs | 32 ++++-- .../DeveloperExceptionPageMiddlewareImpl.cs | 31 ++++-- .../DeveloperExceptionPageSampleTest.cs | 10 ++ ...tpValidationProblemDetailsJsonConverter.cs | 99 +++++++++++++------ .../ProblemDetailsJsonConverter.cs | 17 +--- 7 files changed, 176 insertions(+), 57 deletions(-) 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..fd74c7214f41 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.Nodes; 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, @@ -70,6 +72,22 @@ public ValueTask WriteAsync(ProblemDetailsContext context) } [JsonSerializable(typeof(ProblemDetails))] + [JsonSerializable(typeof(Dictionary))] + [JsonSerializable(typeof(JsonNode))] + [JsonSerializable(typeof(JsonObject))] + [JsonSerializable(typeof(JsonArray))] + [JsonSerializable(typeof(JsonValue))] + [JsonSerializable(typeof(string))] + [JsonSerializable(typeof(decimal))] + [JsonSerializable(typeof(float))] + [JsonSerializable(typeof(double))] + [JsonSerializable(typeof(int))] + [JsonSerializable(typeof(long))] + [JsonSerializable(typeof(Guid))] + [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..087f9df7ded9 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; @@ -78,7 +81,6 @@ public DeveloperExceptionPageMiddlewareImpl( _exceptionDetailsProvider = new ExceptionDetailsProvider(_fileProvider, _logger, _options.SourceCodeLineCount); _exceptionHandler = DisplayException; _problemDetailsService = problemDetailsService; - foreach (var filter in filters.Reverse()) { var nextFilter = _exceptionHandler; @@ -179,13 +181,18 @@ private async Task DisplayExceptionContent(ErrorContext errorContext) Status = httpContext.Response.StatusCode }; - problemDetails.Extensions["exception"] = new + var headersNode = JsonSerializer.SerializeToNode(httpContext.Request.Headers, ExtensionsExceptionJsonContext.Default.IHeaderDictionary); + var routeValuesNode = httpContext.Features.Get()?.RouteValues is { } routeValues + ? JsonSerializer.SerializeToNode(routeValues, ExtensionsExceptionJsonContext.Default.RouteValueDictionary) + : null; + + problemDetails.Extensions["exception"] = new Dictionary { - Details = errorContext.Exception.ToString(), - Headers = httpContext.Request.Headers, - Path = httpContext.Request.Path.ToString(), - Endpoint = httpContext.GetEndpoint()?.ToString(), - RouteValues = httpContext.Features.Get()?.RouteValues, + ["Details"] = errorContext.Exception.ToString(), + ["Headers"] = headersNode, + ["Path"] = httpContext.Request.Path.ToString(), + ["Endpoint"] = httpContext.GetEndpoint()?.ToString(), + ["RouteValues"] = routeValuesNode, }; await _problemDetailsService.WriteAsync(new() @@ -212,6 +219,9 @@ await _problemDetailsService.WriteAsync(new() await httpContext.Response.WriteAsync(sb.ToString()); } + + static string ResolvePropertyName(string propertyName, JsonNamingPolicy? namingPolicy) => + propertyName; } private Task DisplayCompilationException( @@ -328,3 +338,10 @@ private Task DisplayRuntimeException(HttpContext context, Exception ex) return errorPage.ExecuteAsync(context); } } + +[JsonSerializable(typeof(IHeaderDictionary))] +[JsonSerializable(typeof(RouteValueDictionary))] +[JsonSerializable(typeof(string))] +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/Shared/ProblemDetails/HttpValidationProblemDetailsJsonConverter.cs b/src/Shared/ProblemDetails/HttpValidationProblemDetailsJsonConverter.cs index f1f3f9d2b736..65e1210cbea1 100644 --- a/src/Shared/ProblemDetails/HttpValidationProblemDetailsJsonConverter.cs +++ b/src/Shared/ProblemDetails/HttpValidationProblemDetailsJsonConverter.cs @@ -1,7 +1,6 @@ // 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; @@ -11,18 +10,12 @@ internal sealed partial class HttpValidationProblemDetailsJsonConverter : JsonCo { 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..d46b84689f14 100644 --- a/src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs +++ b/src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs @@ -3,6 +3,7 @@ using System.Diagnostics.CodeAnalysis; using System.Text.Json; +using System.Text.Json.Nodes; using System.Text.Json.Serialization; using Microsoft.AspNetCore.Mvc; @@ -16,10 +17,6 @@ internal sealed partial class ProblemDetailsJsonConverter : JsonConverter Date: Sat, 17 Dec 2022 21:46:37 +0800 Subject: [PATCH 2/9] Clean up --- .../src/DefaultProblemDetailsWriter.cs | 1 + .../DeveloperExceptionPageMiddlewareImpl.cs | 50 ++++++++++--------- .../ProblemDetailsJsonConverter.cs | 1 + 3 files changed, 29 insertions(+), 23 deletions(-) diff --git a/src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs b/src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs index fd74c7214f41..ea7b80a90452 100644 --- a/src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs +++ b/src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs @@ -71,6 +71,7 @@ public ValueTask WriteAsync(ProblemDetailsContext context) contentType: "application/problem+json")); } + // Additional values are specified on JsonContext to support some values for extensions. [JsonSerializable(typeof(ProblemDetails))] [JsonSerializable(typeof(Dictionary))] [JsonSerializable(typeof(JsonNode))] diff --git a/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs b/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs index 087f9df7ded9..e94b4173947e 100644 --- a/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs +++ b/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs @@ -12,7 +12,6 @@ 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; @@ -174,26 +173,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 - }; - - var headersNode = JsonSerializer.SerializeToNode(httpContext.Request.Headers, ExtensionsExceptionJsonContext.Default.IHeaderDictionary); - var routeValuesNode = httpContext.Features.Get()?.RouteValues is { } routeValues - ? JsonSerializer.SerializeToNode(routeValues, ExtensionsExceptionJsonContext.Default.RouteValueDictionary) - : null; - - problemDetails.Extensions["exception"] = new Dictionary - { - ["Details"] = errorContext.Exception.ToString(), - ["Headers"] = headersNode, - ["Path"] = httpContext.Request.Path.ToString(), - ["Endpoint"] = httpContext.GetEndpoint()?.ToString(), - ["RouteValues"] = routeValuesNode, - }; + var problemDetails = CreateProblemDetails(errorContext, httpContext); await _problemDetailsService.WriteAsync(new() { @@ -219,9 +199,33 @@ await _problemDetailsService.WriteAsync(new() await httpContext.Response.WriteAsync(sb.ToString()); } + } + + [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 static ProblemDetails CreateProblemDetails(ErrorContext errorContext, HttpContext httpContext) + { + var problemDetails = new ProblemDetails + { + Title = TypeNameHelper.GetTypeDisplayName(errorContext.Exception.GetType()), + Detail = errorContext.Exception.Message, + Status = httpContext.Response.StatusCode + }; - static string ResolvePropertyName(string propertyName, JsonNamingPolicy? namingPolicy) => - propertyName; + var headersNode = JsonSerializer.SerializeToNode(httpContext.Request.Headers, ExtensionsExceptionJsonContext.Default.IHeaderDictionary); + var routeValuesNode = httpContext.Features.Get()?.RouteValues is { } routeValues + ? JsonSerializer.SerializeToNode(routeValues, ExtensionsExceptionJsonContext.Default.RouteValueDictionary) + : null; + + problemDetails.Extensions["exception"] = new Dictionary + { + ["Details"] = errorContext.Exception.ToString(), + ["Headers"] = headersNode, + ["Path"] = httpContext.Request.Path.ToString(), + ["Endpoint"] = httpContext.GetEndpoint()?.ToString(), + ["RouteValues"] = routeValuesNode, + }; + return problemDetails; } private Task DisplayCompilationException( diff --git a/src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs b/src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs index d46b84689f14..decf5872eb15 100644 --- a/src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs +++ b/src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs @@ -131,6 +131,7 @@ internal static void WriteProblemDetails(Utf8JsonWriter writer, ProblemDetails v foreach (var kvp in value.Extensions) { writer.WritePropertyName(kvp.Key); + // When AOT is enabled, Serialize will only work with values specified on the JsonContext. JsonSerializer.Serialize(writer, kvp.Value, kvp.Value?.GetType() ?? typeof(object), options); } } From ef9a9631891f0129a1295b8e1d901a36ebea5ca1 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 17 Dec 2022 21:58:24 +0800 Subject: [PATCH 3/9] Clean up --- src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs | 3 ++- src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs b/src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs index ea7b80a90452..c3b7cdfdc9a2 100644 --- a/src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs +++ b/src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs @@ -71,7 +71,7 @@ public ValueTask WriteAsync(ProblemDetailsContext context) contentType: "application/problem+json")); } - // Additional values are specified on JsonContext to support some values for extensions. + // Additional values are specified on JsonSerializerContext to support some values for extensions. [JsonSerializable(typeof(ProblemDetails))] [JsonSerializable(typeof(Dictionary))] [JsonSerializable(typeof(JsonNode))] @@ -85,6 +85,7 @@ public ValueTask WriteAsync(ProblemDetailsContext context) [JsonSerializable(typeof(int))] [JsonSerializable(typeof(long))] [JsonSerializable(typeof(Guid))] + [JsonSerializable(typeof(Uri))] [JsonSerializable(typeof(TimeSpan))] [JsonSerializable(typeof(DateTime))] [JsonSerializable(typeof(DateTimeOffset))] diff --git a/src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs b/src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs index decf5872eb15..773dc04b1ecb 100644 --- a/src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs +++ b/src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs @@ -3,7 +3,6 @@ using System.Diagnostics.CodeAnalysis; using System.Text.Json; -using System.Text.Json.Nodes; using System.Text.Json.Serialization; using Microsoft.AspNetCore.Mvc; From 059e7311cd786ce2ff70bb8e2daac93c91efbaa0 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sun, 18 Dec 2022 07:31:08 +0800 Subject: [PATCH 4/9] Fix tests --- .../DeveloperExceptionPageMiddlewareImpl.cs | 39 ++++++++++++------- ...lidationProblemDetailsJsonConverterTest.cs | 2 +- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs b/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs index e94b4173947e..461359cdf8b1 100644 --- a/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs +++ b/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs @@ -12,6 +12,7 @@ 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; @@ -36,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; /// @@ -47,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, @@ -55,6 +58,7 @@ public DeveloperExceptionPageMiddlewareImpl( IWebHostEnvironment hostingEnvironment, DiagnosticSource diagnosticSource, IEnumerable filters, + IOptions? jsonOptions = null, IProblemDetailsService? problemDetailsService = null) { if (next == null) @@ -79,6 +83,7 @@ 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()) { @@ -87,6 +92,12 @@ public DeveloperExceptionPageMiddlewareImpl( } } + private static ExtensionsExceptionJsonContext CreateSerializationContext(JsonOptions? jsonOptions) + { + jsonOptions ??= new JsonOptions(); + return new ExtensionsExceptionJsonContext(new JsonSerializerOptions(jsonOptions.SerializerOptions)); + } + /// /// Process an individual request. /// @@ -212,19 +223,17 @@ private static ProblemDetails CreateProblemDetails(ErrorContext errorContext, Ht Status = httpContext.Response.StatusCode }; - var headersNode = JsonSerializer.SerializeToNode(httpContext.Request.Headers, ExtensionsExceptionJsonContext.Default.IHeaderDictionary); - var routeValuesNode = httpContext.Features.Get()?.RouteValues is { } routeValues - ? JsonSerializer.SerializeToNode(routeValues, ExtensionsExceptionJsonContext.Default.RouteValueDictionary) - : null; + // 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); - problemDetails.Extensions["exception"] = new Dictionary - { - ["Details"] = errorContext.Exception.ToString(), - ["Headers"] = headersNode, - ["Path"] = httpContext.Request.Path.ToString(), - ["Endpoint"] = httpContext.GetEndpoint()?.ToString(), - ["RouteValues"] = routeValuesNode, - }; return problemDetails; } @@ -343,9 +352,9 @@ private Task DisplayRuntimeException(HttpContext context, Exception ex) } } -[JsonSerializable(typeof(IHeaderDictionary))] -[JsonSerializable(typeof(RouteValueDictionary))] -[JsonSerializable(typeof(string))] +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/Mvc/Mvc.Core/test/Infrastructure/ValidationProblemDetailsJsonConverterTest.cs b/src/Mvc/Mvc.Core/test/Infrastructure/ValidationProblemDetailsJsonConverterTest.cs index b7b877bf38f0..9c35ea32b334 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 JsonSerializationOptions()); writer.Flush(); var json = Encoding.UTF8.GetString(stream.ToArray()); From 1e0fb4d1fbfd6c12f5bbc4fb8d85169e011a286e Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sun, 18 Dec 2022 07:32:20 +0800 Subject: [PATCH 5/9] Comment --- .../DeveloperExceptionPageMiddlewareImpl.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs b/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs index 461359cdf8b1..1d0b4b33d979 100644 --- a/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs +++ b/src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs @@ -94,6 +94,7 @@ 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)); } @@ -214,7 +215,7 @@ 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 static ProblemDetails CreateProblemDetails(ErrorContext errorContext, HttpContext httpContext) + private ProblemDetails CreateProblemDetails(ErrorContext errorContext, HttpContext httpContext) { var problemDetails = new ProblemDetails { From 0ca6b4766c40cb3163a2cb1e96db02f9be2a09ee Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sun, 18 Dec 2022 08:41:25 +0800 Subject: [PATCH 6/9] Update --- .../Http.Extensions/src/DefaultProblemDetailsWriter.cs | 8 +++----- .../ValidationProblemDetailsJsonConverterTest.cs | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs b/src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs index c3b7cdfdc9a2..0de74fea8a6d 100644 --- a/src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs +++ b/src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs @@ -3,6 +3,7 @@ using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; +using System.Text.Json; using System.Text.Json.Nodes; using System.Text.Json.Serialization; using Microsoft.AspNetCore.Mvc; @@ -72,12 +73,9 @@ public ValueTask WriteAsync(ProblemDetailsContext context) } // 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(Dictionary))] - [JsonSerializable(typeof(JsonNode))] - [JsonSerializable(typeof(JsonObject))] - [JsonSerializable(typeof(JsonArray))] - [JsonSerializable(typeof(JsonValue))] + [JsonSerializable(typeof(JsonElement))] [JsonSerializable(typeof(string))] [JsonSerializable(typeof(decimal))] [JsonSerializable(typeof(float))] diff --git a/src/Mvc/Mvc.Core/test/Infrastructure/ValidationProblemDetailsJsonConverterTest.cs b/src/Mvc/Mvc.Core/test/Infrastructure/ValidationProblemDetailsJsonConverterTest.cs index 9c35ea32b334..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, new JsonSerializationOptions()); + converter.Write(writer, problemDetails, new JsonSerializerOptions()); writer.Flush(); var json = Encoding.UTF8.GetString(stream.ToArray()); From 20ce35b92da00a9c20a7367ac14714066b81358b Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sun, 18 Dec 2022 08:44:37 +0800 Subject: [PATCH 7/9] Update --- src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs b/src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs index 0de74fea8a6d..e3cd65db380e 100644 --- a/src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs +++ b/src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs @@ -4,7 +4,6 @@ using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using System.Text.Json; -using System.Text.Json.Nodes; using System.Text.Json.Serialization; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Options; From 6d21d57f3593af19e4b9d83d27937c8d33e6e55e Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 20 Dec 2022 21:01:58 +0800 Subject: [PATCH 8/9] PR feedback --- .../ProblemDetailsJsonConverter.cs | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs b/src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs index 773dc04b1ecb..68ee98d73721 100644 --- a/src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs +++ b/src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs @@ -45,8 +45,6 @@ public override void Write(Utf8JsonWriter writer, ProblemDetails value, JsonSeri writer.WriteEndObject(); } - [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "ProblemDetails.Extensions is annotated to expose this warning to callers.")] - [UnconditionalSuppressMessage("AOT", "IL3050", Justification = "ProblemDetails.Extensions is annotated to expose this warning to callers.")] internal static void ReadValue(ref Utf8JsonReader reader, ProblemDetails value, JsonSerializerOptions options) { if (TryReadStringProperty(ref reader, Type, out var propertyValue)) @@ -81,7 +79,14 @@ internal static void ReadValue(ref Utf8JsonReader reader, ProblemDetails value, { var key = reader.GetString()!; reader.Read(); - value.Extensions[key] = JsonSerializer.Deserialize(ref reader, typeof(object), options); + ReadExtension(value, key, ref reader, options); + } + + [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "ProblemDetails.Extensions is annotated to expose this warning to callers.")] + [UnconditionalSuppressMessage("AOT", "IL3050", Justification = "ProblemDetails.Extensions is annotated to expose this warning to callers.")] + static void ReadExtension(ProblemDetails problemDetails, string key, ref Utf8JsonReader reader, JsonSerializerOptions options) + { + problemDetails.Extensions[key] = JsonSerializer.Deserialize(ref reader, typeof(object), options); } } @@ -98,8 +103,6 @@ internal static bool TryReadStringProperty(ref Utf8JsonReader reader, JsonEncode return true; } - [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "ProblemDetails.Extensions is annotated to expose this warning to callers.")] - [UnconditionalSuppressMessage("AOT", "IL3050", Justification = "ProblemDetails.Extensions is annotated to expose this warning to callers.")] internal static void WriteProblemDetails(Utf8JsonWriter writer, ProblemDetails value, JsonSerializerOptions options) { if (value.Type != null) @@ -127,11 +130,18 @@ internal static void WriteProblemDetails(Utf8JsonWriter writer, ProblemDetails v writer.WriteString(Instance, value.Instance); } - foreach (var kvp in value.Extensions) + WriteExtensions(value, writer, options); + + [UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "ProblemDetails.Extensions is annotated to expose this warning to callers.")] + [UnconditionalSuppressMessage("AOT", "IL3050", Justification = "ProblemDetails.Extensions is annotated to expose this warning to callers.")] + static void WriteExtensions(ProblemDetails problemDetails, Utf8JsonWriter writer, JsonSerializerOptions options) { - writer.WritePropertyName(kvp.Key); - // When AOT is enabled, Serialize will only work with values specified on the JsonContext. - JsonSerializer.Serialize(writer, kvp.Value, kvp.Value?.GetType() ?? typeof(object), options); + foreach (var kvp in problemDetails.Extensions) + { + writer.WritePropertyName(kvp.Key); + // When AOT is enabled, Serialize will only work with values specified on the JsonContext. + JsonSerializer.Serialize(writer, kvp.Value, kvp.Value?.GetType() ?? typeof(object), options); + } } } } From 5a85857de21ee9f033b83787ceb086a3b3208194 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 20 Dec 2022 21:02:15 +0800 Subject: [PATCH 9/9] PR feedback --- .../ProblemDetails/HttpValidationProblemDetailsJsonConverter.cs | 2 +- src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Shared/ProblemDetails/HttpValidationProblemDetailsJsonConverter.cs b/src/Shared/ProblemDetails/HttpValidationProblemDetailsJsonConverter.cs index 65e1210cbea1..d903bf99c34d 100644 --- a/src/Shared/ProblemDetails/HttpValidationProblemDetailsJsonConverter.cs +++ b/src/Shared/ProblemDetails/HttpValidationProblemDetailsJsonConverter.cs @@ -6,7 +6,7 @@ namespace Microsoft.AspNetCore.Http; -internal sealed partial class HttpValidationProblemDetailsJsonConverter : JsonConverter +internal sealed class HttpValidationProblemDetailsJsonConverter : JsonConverter { private static readonly JsonEncodedText Errors = JsonEncodedText.Encode("errors"); diff --git a/src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs b/src/Shared/ProblemDetails/ProblemDetailsJsonConverter.cs index 68ee98d73721..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");