From a6ce917d599290e63fd66c6dcc4e1d433be8ed0b Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Tue, 11 Nov 2025 17:37:35 +0100 Subject: [PATCH] Update to .NET 10.x packages --- Directory.Packages.props | 71 +++-- .../CustomizableJsonStringEnumConverter.cs | 125 --------- src/ModelContextProtocol.Core/Diagnostics.cs | 8 +- .../McpJsonUtilities.cs | 2 +- .../Protocol/ContextInclusion.cs | 2 +- .../Protocol/LoggingLevel.cs | 2 +- .../Protocol/Role.cs | 2 +- .../Protocol/ElicitationTypedTests.cs | 2 +- .../Protocol/UnknownPropertiesTests.cs | 258 ++++++++++++++++++ 9 files changed, 298 insertions(+), 174 deletions(-) delete mode 100644 src/ModelContextProtocol.Core/CustomizableJsonStringEnumConverter.cs create mode 100644 tests/ModelContextProtocol.Tests/Protocol/UnknownPropertiesTests.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index c058881f7..50d5ca624 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -1,72 +1,69 @@ true - 9.0.10 - 10.0.0-rc.2.25502.107 - 9.10.2 + 8.0.22 + 9.0.11 + 10.0.0 - + + + + + + + + + + + + + + - - - - - - - + + + + + - + - - - - + + - - - + - - - - - - - - - - - + runtime; build; native; contentfiles; analyzers; buildtransitive all - - - - - - - + + + + + + + diff --git a/src/ModelContextProtocol.Core/CustomizableJsonStringEnumConverter.cs b/src/ModelContextProtocol.Core/CustomizableJsonStringEnumConverter.cs deleted file mode 100644 index 617b33359..000000000 --- a/src/ModelContextProtocol.Core/CustomizableJsonStringEnumConverter.cs +++ /dev/null @@ -1,125 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.ComponentModel; -using System.Diagnostics.CodeAnalysis; -#if !NET9_0_OR_GREATER -using System.Reflection; -#endif -using System.Text.Json; -using System.Text.Json.Serialization; -#if !NET9_0_OR_GREATER -using ModelContextProtocol; -#endif - -// NOTE: -// This is a workaround for lack of System.Text.Json's JsonStringEnumConverter -// 9.x support for JsonStringEnumMemberNameAttribute. Once all builds use the System.Text.Json 9.x -// version, this whole file can be removed. Note that the type is public so that external source -// generators can use it, so removing it is a potential breaking change. - -namespace ModelContextProtocol -{ - /// - /// A JSON converter for enums that allows customizing the serialized string value of enum members - /// using the . - /// - /// The enum type to convert. - /// - /// This is a temporary workaround for lack of System.Text.Json's JsonStringEnumConverter<T> - /// 9.x support for custom enum member naming. It will be replaced by the built-in functionality - /// once .NET 9 is fully adopted. - /// - [EditorBrowsable(EditorBrowsableState.Never)] - public sealed class CustomizableJsonStringEnumConverter<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] TEnum> : - JsonStringEnumConverter where TEnum : struct, Enum - { -#if !NET9_0_OR_GREATER - /// - /// Initializes a new instance of the class. - /// - /// - /// The converter automatically detects any enum members decorated with - /// and uses those values during serialization and deserialization. - /// - public CustomizableJsonStringEnumConverter() : - base(namingPolicy: ResolveNamingPolicy()) - { - } - - private static JsonNamingPolicy? ResolveNamingPolicy() - { - var map = typeof(TEnum).GetFields(BindingFlags.Public | BindingFlags.Static) - .Select(f => (f.Name, AttributeName: f.GetCustomAttribute()?.Name)) - .Where(pair => pair.AttributeName != null) - .ToDictionary(pair => pair.Name, pair => pair.AttributeName); - - return map.Count > 0 ? new EnumMemberNamingPolicy(map!) : null; - } - - private sealed class EnumMemberNamingPolicy(Dictionary map) : JsonNamingPolicy - { - public override string ConvertName(string name) => - map.TryGetValue(name, out string? newName) ? - newName : - name; - } -#endif - } - - /// - /// A JSON converter for enums that allows customizing the serialized string value of enum members - /// using the . - /// - /// - /// This is a temporary workaround for lack of System.Text.Json's JsonStringEnumConverter<T> - /// 9.x support for custom enum member naming. It will be replaced by the built-in functionality - /// once .NET 9 is fully adopted. - /// - [EditorBrowsable(EditorBrowsableState.Never)] - [RequiresUnreferencedCode("Requires unreferenced code to instantiate the generic enum converter.")] - [RequiresDynamicCode("Requires dynamic code to instantiate the generic enum converter.")] - public sealed class CustomizableJsonStringEnumConverter : JsonConverterFactory - { - /// - public override bool CanConvert(Type typeToConvert) => typeToConvert.IsEnum; - /// - public override JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions options) - { - Type converterType = typeof(CustomizableJsonStringEnumConverter<>).MakeGenericType(typeToConvert)!; - var factory = (JsonConverterFactory)Activator.CreateInstance(converterType)!; - return factory.CreateConverter(typeToConvert, options); - } - } -} - -#if !NET9_0_OR_GREATER -namespace System.Text.Json.Serialization -{ - /// - /// Determines the custom string value that should be used when serializing an enum member using JSON. - /// - /// - /// This attribute is a temporary workaround for lack of System.Text.Json's support for custom enum member naming - /// in versions prior to .NET 9. It works together with - /// to provide customized string representations of enum values during JSON serialization and deserialization. - /// - [AttributeUsage(AttributeTargets.Field, AllowMultiple = false)] - internal sealed class JsonStringEnumMemberNameAttribute : Attribute - { - /// - /// Creates new attribute instance with a specified enum member name. - /// - /// The name to apply to the current enum member when serialized to JSON. - public JsonStringEnumMemberNameAttribute(string name) - { - Name = name; - } - - /// - /// Gets the custom JSON name of the enum member. - /// - public string Name { get; } - } -} -#endif \ No newline at end of file diff --git a/src/ModelContextProtocol.Core/Diagnostics.cs b/src/ModelContextProtocol.Core/Diagnostics.cs index 284d9088e..bed648868 100644 --- a/src/ModelContextProtocol.Core/Diagnostics.cs +++ b/src/ModelContextProtocol.Core/Diagnostics.cs @@ -13,13 +13,8 @@ internal static class Diagnostics internal static Meter Meter { get; } = new("Experimental.ModelContextProtocol"); internal static Histogram CreateDurationHistogram(string name, string description, bool longBuckets) => - Meter.CreateHistogram(name, "s", description -#if NET9_0_OR_GREATER - , advice: longBuckets ? LongSecondsBucketBoundaries : ShortSecondsBucketBoundaries -#endif - ); + Meter.CreateHistogram(name, "s", description, advice: longBuckets ? LongSecondsBucketBoundaries : ShortSecondsBucketBoundaries); -#if NET9_0_OR_GREATER /// /// Follows boundaries from http.server.request.duration/http.client.request.duration /// @@ -36,7 +31,6 @@ internal static Histogram CreateDurationHistogram(string name, string de { HistogramBucketBoundaries = [0.01, 0.02, 0.05, 0.1, 0.2, 0.5, 1, 2, 5, 10, 30, 60, 120, 300], }; -#endif internal static ActivityContext ExtractActivityContext(this DistributedContextPropagator propagator, JsonRpcMessage message) { diff --git a/src/ModelContextProtocol.Core/McpJsonUtilities.cs b/src/ModelContextProtocol.Core/McpJsonUtilities.cs index 8bc9e21b0..3d08bd82e 100644 --- a/src/ModelContextProtocol.Core/McpJsonUtilities.cs +++ b/src/ModelContextProtocol.Core/McpJsonUtilities.cs @@ -47,7 +47,7 @@ private static JsonSerializerOptions CreateDefaultOptions() // Add a converter for user-defined enums, if reflection is enabled by default. if (JsonSerializer.IsReflectionEnabledByDefault) { - options.Converters.Add(new CustomizableJsonStringEnumConverter()); + options.Converters.Add(new JsonStringEnumConverter()); } options.MakeReadOnly(); diff --git a/src/ModelContextProtocol.Core/Protocol/ContextInclusion.cs b/src/ModelContextProtocol.Core/Protocol/ContextInclusion.cs index f6ec267ba..8894a3b3a 100644 --- a/src/ModelContextProtocol.Core/Protocol/ContextInclusion.cs +++ b/src/ModelContextProtocol.Core/Protocol/ContextInclusion.cs @@ -8,7 +8,7 @@ namespace ModelContextProtocol.Protocol; /// /// See the schema for details. /// -[JsonConverter(typeof(CustomizableJsonStringEnumConverter))] +[JsonConverter(typeof(JsonStringEnumConverter))] public enum ContextInclusion { /// diff --git a/src/ModelContextProtocol.Core/Protocol/LoggingLevel.cs b/src/ModelContextProtocol.Core/Protocol/LoggingLevel.cs index 55d0b6ea7..93c8a9a67 100644 --- a/src/ModelContextProtocol.Core/Protocol/LoggingLevel.cs +++ b/src/ModelContextProtocol.Core/Protocol/LoggingLevel.cs @@ -8,7 +8,7 @@ namespace ModelContextProtocol.Protocol; /// /// These map to syslog message severities, as specified in RFC-5424. /// -[JsonConverter(typeof(CustomizableJsonStringEnumConverter))] +[JsonConverter(typeof(JsonStringEnumConverter))] public enum LoggingLevel { /// Detailed debug information, typically only valuable to developers. diff --git a/src/ModelContextProtocol.Core/Protocol/Role.cs b/src/ModelContextProtocol.Core/Protocol/Role.cs index d633ce4c2..43bc26708 100644 --- a/src/ModelContextProtocol.Core/Protocol/Role.cs +++ b/src/ModelContextProtocol.Core/Protocol/Role.cs @@ -5,7 +5,7 @@ namespace ModelContextProtocol.Protocol; /// /// Represents the type of role in the Model Context Protocol conversation. /// -[JsonConverter(typeof(CustomizableJsonStringEnumConverter))] +[JsonConverter(typeof(JsonStringEnumConverter))] public enum Role { /// diff --git a/tests/ModelContextProtocol.Tests/Protocol/ElicitationTypedTests.cs b/tests/ModelContextProtocol.Tests/Protocol/ElicitationTypedTests.cs index b5cfec76c..b4afb1af3 100644 --- a/tests/ModelContextProtocol.Tests/Protocol/ElicitationTypedTests.cs +++ b/tests/ModelContextProtocol.Tests/Protocol/ElicitationTypedTests.cs @@ -309,7 +309,7 @@ public async Task Elicit_Typed_With_NonObject_Generic_Type_Throws() Assert.Contains(typeof(string).FullName!, ex.Message); } - [JsonConverter(typeof(CustomizableJsonStringEnumConverter))] + [JsonConverter(typeof(JsonStringEnumConverter))] public enum SampleRole { diff --git a/tests/ModelContextProtocol.Tests/Protocol/UnknownPropertiesTests.cs b/tests/ModelContextProtocol.Tests/Protocol/UnknownPropertiesTests.cs new file mode 100644 index 000000000..fd3117de2 --- /dev/null +++ b/tests/ModelContextProtocol.Tests/Protocol/UnknownPropertiesTests.cs @@ -0,0 +1,258 @@ +using System.Text.Json; +using ModelContextProtocol.Protocol; + +namespace ModelContextProtocol.Tests.Protocol; + +/// +/// Tests that custom JSON converters properly handle unknown additional properties +/// for forward compatibility with future protocol versions. +/// +public class UnknownPropertiesTests +{ + [Fact] + public void ContentBlock_DeserializationWithUnknownProperty_SkipsProperty() + { + // Arrange - JSON with unknown "unknownField" property + const string Json = """ + { + "type": "text", + "text": "Hello, world!", + "unknownField": "should be skipped" + } + """; + + // Act + var deserialized = JsonSerializer.Deserialize(Json, McpJsonUtilities.DefaultOptions); + + // Assert + Assert.NotNull(deserialized); + var textBlock = Assert.IsType(deserialized); + Assert.Equal("Hello, world!", textBlock.Text); + } + + [Fact] + public void ContentBlock_DeserializationWithStructuredContentInContent_SkipsProperty() + { + // Arrange - This was the actual bug case: structuredContent incorrectly placed + // inside a ContentBlock instead of at CallToolResult level + const string Json = """ + { + "type": "text", + "text": "Result text", + "structuredContent": { + "data": "this should be ignored" + } + } + """; + + // Act - Should not throw + var deserialized = JsonSerializer.Deserialize(Json, McpJsonUtilities.DefaultOptions); + + // Assert + Assert.NotNull(deserialized); + var textBlock = Assert.IsType(deserialized); + Assert.Equal("Result text", textBlock.Text); + } + + [Fact] + public void ContentBlock_DeserializationWithMultipleUnknownProperties_SkipsAll() + { + // Arrange - JSON with multiple unknown properties + const string Json = """ + { + "type": "image", + "data": "base64data", + "mimeType": "image/png", + "futureProperty1": "value1", + "futureProperty2": {"nested": "object"}, + "futureProperty3": [1, 2, 3] + } + """; + + // Act + var deserialized = JsonSerializer.Deserialize(Json, McpJsonUtilities.DefaultOptions); + + // Assert + Assert.NotNull(deserialized); + var imageBlock = Assert.IsType(deserialized); + Assert.Equal("base64data", imageBlock.Data); + Assert.Equal("image/png", imageBlock.MimeType); + } + + [Fact] + public void Reference_DeserializationWithUnknownProperty_SkipsProperty() + { + // Arrange - JSON with unknown "metadata" property + const string Json = """ + { + "type": "ref/prompt", + "name": "test-prompt", + "metadata": { + "version": "2.0", + "author": "future" + } + } + """; + + // Act + var deserialized = JsonSerializer.Deserialize(Json, McpJsonUtilities.DefaultOptions); + + // Assert + Assert.NotNull(deserialized); + var promptRef = Assert.IsType(deserialized); + Assert.Equal("test-prompt", promptRef.Name); + } + + [Fact] + public void Reference_DeserializationWithMultipleUnknownProperties_SkipsAll() + { + // Arrange + const string Json = """ + { + "type": "ref/resource", + "uri": "file:///test.txt", + "extraField1": "ignored", + "extraField2": 123, + "extraField3": true + } + """; + + // Act + var deserialized = JsonSerializer.Deserialize(Json, McpJsonUtilities.DefaultOptions); + + // Assert + Assert.NotNull(deserialized); + var resourceRef = Assert.IsType(deserialized); + Assert.Equal("file:///test.txt", resourceRef.Uri); + } + + [Fact] + public void ResourceContents_DeserializationWithUnknownProperty_SkipsProperty() + { + // Arrange + const string Json = """ + { + "uri": "file:///document.txt", + "text": "File contents here", + "mimeType": "text/plain", + "futureEncoding": "utf-16", + "futureChecksum": "abc123" + } + """; + + // Act + var deserialized = JsonSerializer.Deserialize(Json, McpJsonUtilities.DefaultOptions); + + // Assert + Assert.NotNull(deserialized); + var textResource = Assert.IsType(deserialized); + Assert.Equal("file:///document.txt", textResource.Uri); + Assert.Equal("File contents here", textResource.Text); + Assert.Equal("text/plain", textResource.MimeType); + } + + [Fact] + public void ProgressNotificationParams_DeserializationWithUnknownProperty_SkipsProperty() + { + // Arrange + const string Json = """ + { + "progressToken": "token123", + "progress": 50, + "newProgressFormat": "percentage", + "estimatedTimeRemaining": 30 + } + """; + + // Act + var deserialized = JsonSerializer.Deserialize(Json, McpJsonUtilities.DefaultOptions); + + // Assert + Assert.NotNull(deserialized); + Assert.Equal("token123", deserialized.ProgressToken.ToString()); + Assert.Equal(50.0f, deserialized.Progress.Progress); + } + + [Fact] + public void PrimitiveSchemaDefinition_DeserializationWithUnknownProperty_SkipsProperty() + { + // Arrange + const string Json = """ + { + "type": "string", + "description": "A test string", + "futureValidation": "regex:.*", + "futureMaxLength": 100 + } + """; + + // Act + var deserialized = JsonSerializer.Deserialize(Json, McpJsonUtilities.DefaultOptions); + + // Assert + Assert.NotNull(deserialized); + var stringSchema = Assert.IsType(deserialized); + Assert.Equal("A test string", stringSchema.Description); + } + + [Fact] + public void CallToolResult_WithContentBlockContainingUnknownProperties_Succeeds() + { + // Arrange - Simulates the real-world bug scenario: a malformed response where + // structuredContent was incorrectly nested inside content blocks + const string Json = """ + { + "content": [ + { + "type": "text", + "text": "Tool executed successfully", + "structuredContent": { + "result": "this was incorrectly placed here" + } + } + ], + "isError": false + } + """; + + // Act - Should not throw an exception + var deserialized = JsonSerializer.Deserialize(Json, McpJsonUtilities.DefaultOptions); + + // Assert + Assert.NotNull(deserialized); + Assert.Single(deserialized.Content); + var textBlock = Assert.IsType(deserialized.Content[0]); + Assert.Equal("Tool executed successfully", textBlock.Text); + Assert.False(deserialized.IsError); + } + + [Fact] + public void CallToolResult_WithStructuredContentAtCorrectLevel_PreservesProperty() + { + // Arrange - Correct placement of structuredContent at CallToolResult level + const string Json = """ + { + "content": [ + { + "type": "text", + "text": "Tool executed" + } + ], + "structuredContent": { + "result": "correctly placed here", + "value": 42 + }, + "isError": false + } + """; + + // Act + var deserialized = JsonSerializer.Deserialize(Json, McpJsonUtilities.DefaultOptions); + + // Assert + Assert.NotNull(deserialized); + Assert.NotNull(deserialized.StructuredContent); + Assert.Equal("correctly placed here", deserialized.StructuredContent["result"]?.ToString()); + Assert.Equal(42, (int?)deserialized.StructuredContent["value"]); + } +}