From d954ddafcaf8a00ffebd05b69321d9e973ff880a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 31 Jul 2025 22:09:50 +0000 Subject: [PATCH 1/4] Initial plan From d81fbeebd0d326666747dcc0d0a8b539bbe495ff Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 31 Jul 2025 22:28:48 +0000 Subject: [PATCH 2/4] Implement trim-safe default value fallback with try-catch mechanism Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com> --- .../src/Extensions/JsonNodeSchemaExtensions.cs | 13 ++++++++++++- .../OpenApiSchemaService.ParameterSchemas.cs | 3 +++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/OpenApi/src/Extensions/JsonNodeSchemaExtensions.cs b/src/OpenApi/src/Extensions/JsonNodeSchemaExtensions.cs index fc282118ef08..ed090d6d08e8 100644 --- a/src/OpenApi/src/Extensions/JsonNodeSchemaExtensions.cs +++ b/src/OpenApi/src/Extensions/JsonNodeSchemaExtensions.cs @@ -183,7 +183,18 @@ internal static void ApplyDefaultValue(this JsonNode schema, object? defaultValu } else { - schema[OpenApiSchemaKeywords.DefaultKeyword] = JsonSerializer.SerializeToNode(defaultValue, jsonTypeInfo); + try + { + schema[OpenApiSchemaKeywords.DefaultKeyword] = JsonSerializer.SerializeToNode(defaultValue, jsonTypeInfo); + } + catch (InvalidCastException) + { + // Fallback to string representation when there's a type mismatch + var stringValue = defaultValue is IFormattable formattable + ? formattable.ToString(null, CultureInfo.InvariantCulture) + : defaultValue.ToString(); + schema[OpenApiSchemaKeywords.DefaultKeyword] = JsonValue.Create(stringValue); + } } } diff --git a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.ParameterSchemas.cs b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.ParameterSchemas.cs index 7929b27f0eca..489f3a63034d 100644 --- a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.ParameterSchemas.cs +++ b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.ParameterSchemas.cs @@ -175,6 +175,9 @@ await VerifyOpenApiDocument(builder, document => [([DefaultValue("foo")] string? id) => { }, (JsonNode defaultValue) => Assert.Equal("foo", defaultValue.GetValue())], [([DefaultValue(null)] TaskStatus? status) => { }, (JsonNode defaultValue) => Assert.True(defaultValue is null)], [([DefaultValue(TaskStatus.Canceled)] TaskStatus? status) => { }, (JsonNode defaultValue) => Assert.Equal(6, defaultValue.GetValue())], + // F# scenarios where type mismatch causes InvalidCastException and fallback to string is used + [([DefaultValue(10)] ulong id) => { }, (JsonNode defaultValue) => Assert.Equal("10", defaultValue.GetValue())], + [([DefaultValue(10u)] ulong id) => { }, (JsonNode defaultValue) => Assert.Equal("10", defaultValue.GetValue())], ]; [Theory] From 4e363d39851d441c5318177c3f1e46a465097e2b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 31 Jul 2025 23:41:21 +0000 Subject: [PATCH 3/4] Implement logging-based solution instead of string fallback Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com> --- .../Extensions/JsonNodeSchemaExtensions.cs | 21 ++--- .../Extensions/OpenApiLoggingExtensions.cs | 12 +++ .../Services/Schemas/OpenApiSchemaService.cs | 8 +- .../OpenApiDocumentServiceTestsBase.cs | 8 +- .../OpenApiSchemaService.ParameterSchemas.cs | 90 ++++++++++++++++++- 5 files changed, 121 insertions(+), 18 deletions(-) create mode 100644 src/OpenApi/src/Extensions/OpenApiLoggingExtensions.cs diff --git a/src/OpenApi/src/Extensions/JsonNodeSchemaExtensions.cs b/src/OpenApi/src/Extensions/JsonNodeSchemaExtensions.cs index ed090d6d08e8..b563dd070256 100644 --- a/src/OpenApi/src/Extensions/JsonNodeSchemaExtensions.cs +++ b/src/OpenApi/src/Extensions/JsonNodeSchemaExtensions.cs @@ -15,6 +15,7 @@ using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Constraints; +using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.OpenApi; @@ -170,7 +171,8 @@ internal static void ApplyValidationAttributes(this JsonNode schema, IEnumerable /// The produced by the underlying schema generator. /// An object representing the associated with the default value. /// The associated with the target type. - internal static void ApplyDefaultValue(this JsonNode schema, object? defaultValue, JsonTypeInfo? jsonTypeInfo) + /// The logger to use for warning messages when default value type mismatches occur. + internal static void ApplyDefaultValue(this JsonNode schema, object? defaultValue, JsonTypeInfo? jsonTypeInfo, ILogger? logger = null) { if (jsonTypeInfo is null) { @@ -187,13 +189,11 @@ internal static void ApplyDefaultValue(this JsonNode schema, object? defaultValu { schema[OpenApiSchemaKeywords.DefaultKeyword] = JsonSerializer.SerializeToNode(defaultValue, jsonTypeInfo); } - catch (InvalidCastException) + catch (Exception ex) when (ex is InvalidCastException or NotSupportedException or InvalidOperationException) { - // Fallback to string representation when there's a type mismatch - var stringValue = defaultValue is IFormattable formattable - ? formattable.ToString(null, CultureInfo.InvariantCulture) - : defaultValue.ToString(); - schema[OpenApiSchemaKeywords.DefaultKeyword] = JsonValue.Create(stringValue); + // Log warning when there's a type mismatch that prevents serialization + logger?.DefaultValueTypeMismatch(defaultValue.GetType().Name, jsonTypeInfo.Type.Name); + // Do not apply the default value when there's a type mismatch } } } @@ -316,7 +316,8 @@ internal static void ApplyRouteConstraints(this JsonNode schema, IEnumerableThe produced by the underlying schema generator. /// The associated with the . /// The associated with the . - internal static void ApplyParameterInfo(this JsonNode schema, ApiParameterDescription parameterDescription, JsonTypeInfo? jsonTypeInfo) + /// The logger to use for warning messages when default value type mismatches occur. + internal static void ApplyParameterInfo(this JsonNode schema, ApiParameterDescription parameterDescription, JsonTypeInfo? jsonTypeInfo, ILogger? logger = null) { // This is special handling for parameters that are not bound from the body but represented in a complex type. // For example: @@ -344,11 +345,11 @@ internal static void ApplyParameterInfo(this JsonNode schema, ApiParameterDescri { if (parameterInfo.HasDefaultValue) { - schema.ApplyDefaultValue(parameterInfo.DefaultValue, jsonTypeInfo); + schema.ApplyDefaultValue(parameterInfo.DefaultValue, jsonTypeInfo, logger); } else if (parameterInfo.GetCustomAttributes().LastOrDefault() is { } defaultValueAttribute) { - schema.ApplyDefaultValue(defaultValueAttribute.Value, jsonTypeInfo); + schema.ApplyDefaultValue(defaultValueAttribute.Value, jsonTypeInfo, logger); } if (parameterInfo.GetCustomAttributes() is { } validationAttributes) diff --git a/src/OpenApi/src/Extensions/OpenApiLoggingExtensions.cs b/src/OpenApi/src/Extensions/OpenApiLoggingExtensions.cs new file mode 100644 index 000000000000..516b39b97c5f --- /dev/null +++ b/src/OpenApi/src/Extensions/OpenApiLoggingExtensions.cs @@ -0,0 +1,12 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNetCore.OpenApi; + +internal static partial class OpenApiLoggingExtensions +{ + [LoggerMessage(1, LogLevel.Warning, "Failed to apply default value for parameter due to type mismatch. Default value type: '{DefaultValueType}', Parameter type: '{ParameterType}'. Default value will be omitted from the OpenAPI schema.", EventName = "DefaultValueTypeMismatch")] + public static partial void DefaultValueTypeMismatch(this ILogger logger, string defaultValueType, string parameterType); +} \ No newline at end of file diff --git a/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs b/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs index 10737cbd5f20..3b5798494c36 100644 --- a/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs +++ b/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs @@ -16,6 +16,7 @@ using Microsoft.AspNetCore.Mvc.ApiExplorer; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.OpenApi; @@ -28,7 +29,8 @@ namespace Microsoft.AspNetCore.OpenApi; internal sealed class OpenApiSchemaService( [ServiceKey] string documentName, IOptions jsonOptions, - IOptionsMonitor optionsMonitor) + IOptionsMonitor optionsMonitor, + ILogger logger) { private readonly ConcurrentDictionary _schemaIdCache = new(); private readonly OpenApiJsonSchemaContext _jsonSchemaContext = new(new(jsonOptions.Value.SerializerOptions)); @@ -105,7 +107,7 @@ internal sealed class OpenApiSchemaService( } if (attributeProvider.GetCustomAttributes(inherit: false).OfType().LastOrDefault() is DefaultValueAttribute defaultValueAttribute) { - schema.ApplyDefaultValue(defaultValueAttribute.Value, context.TypeInfo); + schema.ApplyDefaultValue(defaultValueAttribute.Value, context.TypeInfo, logger); } if (attributeProvider.GetCustomAttributes(inherit: false).OfType().LastOrDefault() is DescriptionAttribute descriptionAttribute) { @@ -125,7 +127,7 @@ internal async Task GetOrCreateUnresolvedSchemaAsync(OpenApiDocum var schemaAsJsonObject = CreateSchema(key); if (parameterDescription is not null) { - schemaAsJsonObject.ApplyParameterInfo(parameterDescription, _jsonSerializerOptions.GetTypeInfo(type)); + schemaAsJsonObject.ApplyParameterInfo(parameterDescription, _jsonSerializerOptions.GetTypeInfo(type), logger); } // Use _jsonSchemaContext constructed from _jsonSerializerOptions to respect shared config set by end-user, // particularly in the case of maxDepth. diff --git a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiDocumentServiceTestsBase.cs b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiDocumentServiceTestsBase.cs index f18197120b86..840256f4422c 100644 --- a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiDocumentServiceTestsBase.cs +++ b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiDocumentServiceTestsBase.cs @@ -20,6 +20,8 @@ using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Constraints; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using Microsoft.Net.Http.Headers; using Moq; @@ -88,7 +90,8 @@ internal static OpenApiDocumentService CreateDocumentService(IEndpointRouteBuild // Set strict number handling by default to make integer type checks more straightforward jsonOptions.SerializerOptions.NumberHandling = System.Text.Json.Serialization.JsonNumberHandling.Strict; - var schemaService = new OpenApiSchemaService("Test", Options.Create(jsonOptions), openApiOptions.Object); + var logger = builder.ServiceProvider.GetService>() ?? NullLogger.Instance; + var schemaService = new OpenApiSchemaService("Test", Options.Create(jsonOptions), openApiOptions.Object, logger); ((TestServiceProvider)builder.ServiceProvider).TestSchemaService = schemaService; var documentService = new OpenApiDocumentService("Test", apiDescriptionGroupCollectionProvider, hostEnvironment, openApiOptions.Object, builder.ServiceProvider, new OpenApiTestServer()); ((TestServiceProvider)builder.ServiceProvider).TestDocumentService = documentService; @@ -134,7 +137,8 @@ internal static OpenApiDocumentService CreateDocumentService(IEndpointRouteBuild defaultJsonOptions.SerializerOptions.NumberHandling = System.Text.Json.Serialization.JsonNumberHandling.Strict; var jsonOptions = builder.ServiceProvider.GetService>() ?? Options.Create(defaultJsonOptions); - var schemaService = new OpenApiSchemaService("Test", jsonOptions, options.Object); + var logger = builder.ServiceProvider.GetService>() ?? NullLogger.Instance; + var schemaService = new OpenApiSchemaService("Test", jsonOptions, options.Object, logger); ((TestServiceProvider)builder.ServiceProvider).TestSchemaService = schemaService; var documentService = new OpenApiDocumentService("Test", apiDescriptionGroupCollectionProvider, hostEnvironment, options.Object, builder.ServiceProvider, new OpenApiTestServer()); ((TestServiceProvider)builder.ServiceProvider).TestDocumentService = documentService; diff --git a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.ParameterSchemas.cs b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.ParameterSchemas.cs index 489f3a63034d..a6258d00e4d2 100644 --- a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.ParameterSchemas.cs +++ b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiSchemaService/OpenApiSchemaService.ParameterSchemas.cs @@ -14,6 +14,7 @@ using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; public partial class OpenApiSchemaServiceTests : OpenApiDocumentServiceTestBase { @@ -175,9 +176,6 @@ await VerifyOpenApiDocument(builder, document => [([DefaultValue("foo")] string? id) => { }, (JsonNode defaultValue) => Assert.Equal("foo", defaultValue.GetValue())], [([DefaultValue(null)] TaskStatus? status) => { }, (JsonNode defaultValue) => Assert.True(defaultValue is null)], [([DefaultValue(TaskStatus.Canceled)] TaskStatus? status) => { }, (JsonNode defaultValue) => Assert.Equal(6, defaultValue.GetValue())], - // F# scenarios where type mismatch causes InvalidCastException and fallback to string is used - [([DefaultValue(10)] ulong id) => { }, (JsonNode defaultValue) => Assert.Equal("10", defaultValue.GetValue())], - [([DefaultValue(10u)] ulong id) => { }, (JsonNode defaultValue) => Assert.Equal("10", defaultValue.GetValue())], ]; [Theory] @@ -565,6 +563,50 @@ await VerifyOpenApiDocument(builder, document => }); } + public static object[][] RouteParametersWithDefaultValueTypeMismatch => + [ + // F# scenarios where type mismatch causes InvalidCastException and logging should occur + [([DefaultValue(10)] ulong id) => { }, (Action)((logMessages) => + { + Assert.Single(logMessages); + Assert.Contains("Failed to apply default value for parameter due to type mismatch", logMessages[0]); + Assert.Contains("Default value type: 'Int32', Parameter type: 'UInt64'", logMessages[0]); + })], + [([DefaultValue(10u)] ulong id) => { }, (Action)((logMessages) => + { + Assert.Single(logMessages); + Assert.Contains("Failed to apply default value for parameter due to type mismatch", logMessages[0]); + Assert.Contains("Default value type: 'UInt32', Parameter type: 'UInt64'", logMessages[0]); + })], + ]; + + [Theory] + [MemberData(nameof(RouteParametersWithDefaultValueTypeMismatch))] + public async Task GetOpenApiParameters_LogsWarningForDefaultValueTypeMismatch(Delegate requestHandler, Action assertLogMessages) + { + // Arrange + var logMessages = new List(); + var serviceCollection = new ServiceCollection(); + serviceCollection.AddSingleton(new TestLoggerProvider(logMessages)); + serviceCollection.AddLogging(); + + var builder = CreateBuilder(serviceCollection); + builder.MapGet("/api/{id}", requestHandler); + + // Act & Assert + await VerifyOpenApiDocument(builder, document => + { + var operation = document.Paths["/api/{id}"].Operations[HttpMethod.Get]; + var parameter = Assert.Single(operation.Parameters); + + // Verify that no default value is set when there's a type mismatch + Assert.Null(parameter.Schema.Default); + + // Verify the warning log was emitted + assertLogMessages(logMessages.ToArray()); + }); + } + public struct CustomType { } public class CustomTypeConverter : JsonConverter @@ -812,4 +854,46 @@ public override void Write(Utf8JsonWriter writer, EnumArrayType value, JsonSeria writer.WriteEndObject(); } } + + public class TestLoggerProvider : ILoggerProvider + { + private readonly List _logMessages; + + public TestLoggerProvider(List logMessages) + { + _logMessages = logMessages; + } + + public ILogger CreateLogger(string categoryName) + { + return new TestLogger(_logMessages); + } + + public void Dispose() { } + } + + public class TestLogger : ILogger + { + private readonly List _logMessages; + + public TestLogger(List logMessages) + { + _logMessages = logMessages; + } + + public IDisposable BeginScope(TState state) => NullScope.Instance; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func formatter) + { + _logMessages.Add(formatter(state, exception)); + } + + private class NullScope : IDisposable + { + public static NullScope Instance { get; } = new(); + public void Dispose() { } + } + } } From d44f96f3842599d5085dad5c84e1ae4f2de5af1c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 31 Jul 2025 23:46:56 +0000 Subject: [PATCH 4/4] Fix OpenApiSchemaService DI registration by making logger optional Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com> --- src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs b/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs index 3b5798494c36..8abae3195383 100644 --- a/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs +++ b/src/OpenApi/src/Services/Schemas/OpenApiSchemaService.cs @@ -30,7 +30,7 @@ internal sealed class OpenApiSchemaService( [ServiceKey] string documentName, IOptions jsonOptions, IOptionsMonitor optionsMonitor, - ILogger logger) + ILogger? logger = null) { private readonly ConcurrentDictionary _schemaIdCache = new(); private readonly OpenApiJsonSchemaContext _jsonSchemaContext = new(new(jsonOptions.Value.SerializerOptions));