diff --git a/src/Common/EncodingUtilities.cs b/src/Common/EncodingUtilities.cs index 50127882b..a3efb8714 100644 --- a/src/Common/EncodingUtilities.cs +++ b/src/Common/EncodingUtilities.cs @@ -56,4 +56,39 @@ public static ReadOnlyMemory DecodeFromBase64Utf8(ReadOnlyMemory bas throw new FormatException("Invalid base64 data"); } } + + /// + /// Validates that is a well-formed base64 encoding (as UTF-8 bytes), + /// without retaining the decoded result. + /// + /// + /// Several protocol types expose a property that is documented to + /// hold the base64 text representation of binary data (as UTF-8 bytes), with a FromBytes factory + /// provided to construct an instance from the raw, unencoded bytes instead. Assigning raw bytes directly + /// to such a property is a common, easy mistake: the value serializes without error but produces a JSON + /// payload that is not valid base64, which only surfaces as a confusing on + /// the deserializing end. This validates eagerly, at the point of misuse, instead. + /// + /// is not valid base64-encoded UTF-8 data. + public static void ValidateBase64Utf8(ReadOnlyMemory base64Utf8, string paramName, string typeName, string factoryMethodName) + { + int maxLength = Base64.GetMaxDecodedFromUtf8Length(base64Utf8.Length); + byte[] buffer = ArrayPool.Shared.Rent(maxLength); + try + { + OperationStatus status = Base64.DecodeFromUtf8(base64Utf8.Span, buffer, out int bytesConsumed, out _); + if (status != OperationStatus.Done || bytesConsumed != base64Utf8.Length) + { + throw new ArgumentException( + $"The value must be the base64 text representation of the binary data, as UTF-8 bytes, " + + $"not the raw, unencoded bytes. Use {typeName}.{factoryMethodName}(...) to construct an " + + $"instance from raw binary data instead.", + paramName); + } + } + finally + { + ArrayPool.Shared.Return(buffer); + } + } } diff --git a/src/ModelContextProtocol.Core/Protocol/BlobResourceContents.cs b/src/ModelContextProtocol.Core/Protocol/BlobResourceContents.cs index d35325010..e540ca043 100644 --- a/src/ModelContextProtocol.Core/Protocol/BlobResourceContents.cs +++ b/src/ModelContextProtocol.Core/Protocol/BlobResourceContents.cs @@ -56,12 +56,35 @@ private BlobResourceContents(ReadOnlyMemory decodedData, string uri, strin MimeType = mimeType; } + [SetsRequiredMembers] + private BlobResourceContents(ReadOnlyMemory base64Utf8, string uri, string? mimeType, bool alreadyBase64Encoded) + { + _blob = base64Utf8; + Uri = uri; + MimeType = mimeType; + } + + /// + /// Creates a directly from base64-encoded UTF-8 bytes that are + /// already known to be valid, bypassing the validation the public setter performs. + /// + /// + /// Used by for the JSON fast path: when the wire blob value required no + /// unescaping, its raw UTF-8 bytes are exactly the base64 text as read from the document, so + /// re-validating them would be redundant. + /// + internal static BlobResourceContents FromTrustedBase64Utf8(ReadOnlyMemory base64Utf8, string uri, string? mimeType) => + new(base64Utf8, uri, mimeType, alreadyBase64Encoded: true); + /// /// Gets or sets the base64-encoded UTF-8 bytes representing the binary data of the item. /// /// - /// Setting this value will invalidate any cached value of . + /// Setting this value will invalidate any cached value of . The value must be + /// the base64 text representation of the data, as UTF-8 bytes — not the raw, unencoded bytes. To + /// construct an instance from raw binary data, use instead. /// + /// The assigned value is not valid base64-encoded UTF-8 data. [JsonPropertyName("blob")] public required ReadOnlyMemory Blob { @@ -77,6 +100,7 @@ public required ReadOnlyMemory Blob } set { + EncodingUtilities.ValidateBase64Utf8(value, nameof(Blob), nameof(BlobResourceContents), nameof(FromBytes)); _blob = value; _decodedData = null; // Invalidate cache } diff --git a/src/ModelContextProtocol.Core/Protocol/ContentBlock.cs b/src/ModelContextProtocol.Core/Protocol/ContentBlock.cs index d0b1b80ec..bd330c0dd 100644 --- a/src/ModelContextProtocol.Core/Protocol/ContentBlock.cs +++ b/src/ModelContextProtocol.Core/Protocol/ContentBlock.cs @@ -243,20 +243,16 @@ public sealed class Converter : JsonConverter "image" => decodedData is not null ? ImageContentBlock.FromBytes(decodedData.Value, mimeType ?? throw new JsonException("MIME type must be provided for 'image' type.")) : - new ImageContentBlock - { - Data = data ?? throw new JsonException("Image data must be provided for 'image' type."), - MimeType = mimeType ?? throw new JsonException("MIME type must be provided for 'image' type."), - }, + ImageContentBlock.FromTrustedBase64Utf8( + data ?? throw new JsonException("Image data must be provided for 'image' type."), + mimeType ?? throw new JsonException("MIME type must be provided for 'image' type.")), "audio" => decodedData is not null ? AudioContentBlock.FromBytes(decodedData.Value, mimeType ?? throw new JsonException("MIME type must be provided for 'audio' type.")) : - new AudioContentBlock - { - Data = data ?? throw new JsonException("Audio data must be provided for 'audio' type."), - MimeType = mimeType ?? throw new JsonException("MIME type must be provided for 'audio' type."), - }, + AudioContentBlock.FromTrustedBase64Utf8( + data ?? throw new JsonException("Audio data must be provided for 'audio' type."), + mimeType ?? throw new JsonException("MIME type must be provided for 'audio' type.")), "resource" => new EmbeddedResourceBlock { @@ -462,6 +458,25 @@ private ImageContentBlock(ReadOnlyMemory decodedData, string mimeType) MimeType = mimeType; } + [SetsRequiredMembers] + private ImageContentBlock(ReadOnlyMemory base64Utf8, string mimeType, bool alreadyBase64Encoded) + { + _data = base64Utf8; + MimeType = mimeType; + } + + /// + /// Creates an directly from base64-encoded UTF-8 bytes that are + /// already known to be valid, bypassing the validation the public setter performs. + /// + /// + /// Used by for the JSON fast path: when the wire data value + /// required no unescaping, its raw UTF-8 bytes are exactly the base64 text as read from the document, + /// so re-validating them would be redundant. + /// + internal static ImageContentBlock FromTrustedBase64Utf8(ReadOnlyMemory base64Utf8, string mimeType) => + new(base64Utf8, mimeType, alreadyBase64Encoded: true); + /// public override string Type => "image"; @@ -469,8 +484,11 @@ private ImageContentBlock(ReadOnlyMemory decodedData, string mimeType) /// Gets or sets the base64-encoded UTF-8 bytes representing the image data. /// /// - /// Setting this value will invalidate any cached value of . + /// Setting this value will invalidate any cached value of . The value must be + /// the base64 text representation of the data, as UTF-8 bytes — not the raw, unencoded bytes. To + /// construct an instance from raw binary data, use instead. /// + /// The assigned value is not valid base64-encoded UTF-8 data. [JsonPropertyName("data")] public required ReadOnlyMemory Data { @@ -486,6 +504,7 @@ public required ReadOnlyMemory Data } set { + EncodingUtilities.ValidateBase64Utf8(value, nameof(Data), nameof(ImageContentBlock), nameof(FromBytes)); _data = value; _decodedData = null; // Invalidate cache } @@ -571,6 +590,25 @@ private AudioContentBlock(ReadOnlyMemory decodedData, string mimeType) MimeType = mimeType; } + [SetsRequiredMembers] + private AudioContentBlock(ReadOnlyMemory base64Utf8, string mimeType, bool alreadyBase64Encoded) + { + _data = base64Utf8; + MimeType = mimeType; + } + + /// + /// Creates an directly from base64-encoded UTF-8 bytes that are + /// already known to be valid, bypassing the validation the public setter performs. + /// + /// + /// Used by for the JSON fast path: when the wire data value + /// required no unescaping, its raw UTF-8 bytes are exactly the base64 text as read from the document, + /// so re-validating them would be redundant. + /// + internal static AudioContentBlock FromTrustedBase64Utf8(ReadOnlyMemory base64Utf8, string mimeType) => + new(base64Utf8, mimeType, alreadyBase64Encoded: true); + /// public override string Type => "audio"; @@ -578,8 +616,11 @@ private AudioContentBlock(ReadOnlyMemory decodedData, string mimeType) /// Gets or sets the base64-encoded UTF-8 bytes representing the audio data. /// /// - /// Setting this value will invalidate any cached value of . + /// Setting this value will invalidate any cached value of . The value must be + /// the base64 text representation of the data, as UTF-8 bytes — not the raw, unencoded bytes. To + /// construct an instance from raw binary data, use instead. /// + /// The assigned value is not valid base64-encoded UTF-8 data. [JsonPropertyName("data")] public required ReadOnlyMemory Data { @@ -595,6 +636,7 @@ public required ReadOnlyMemory Data } set { + EncodingUtilities.ValidateBase64Utf8(value, nameof(Data), nameof(AudioContentBlock), nameof(FromBytes)); _data = value; _decodedData = null; // Invalidate cache } diff --git a/src/ModelContextProtocol.Core/Protocol/ResourceContents.cs b/src/ModelContextProtocol.Core/Protocol/ResourceContents.cs index 283c47d6b..940ddd52f 100644 --- a/src/ModelContextProtocol.Core/Protocol/ResourceContents.cs +++ b/src/ModelContextProtocol.Core/Protocol/ResourceContents.cs @@ -139,13 +139,9 @@ public sealed class Converter : JsonConverter if (blob is not null) { - return new BlobResourceContents - { - Uri = uri ?? string.Empty, - MimeType = mimeType, - Blob = blob.Value, - Meta = meta, - }; + var blobResource = BlobResourceContents.FromTrustedBase64Utf8(blob.Value, uri ?? string.Empty, mimeType); + blobResource.Meta = meta; + return blobResource; } if (text is not null) diff --git a/tests/ModelContextProtocol.Tests/Protocol/ContentBlockTests.cs b/tests/ModelContextProtocol.Tests/Protocol/ContentBlockTests.cs index 6e943f025..ed00d9c9d 100644 --- a/tests/ModelContextProtocol.Tests/Protocol/ContentBlockTests.cs +++ b/tests/ModelContextProtocol.Tests/Protocol/ContentBlockTests.cs @@ -231,7 +231,11 @@ public void ToolResultContentBlock_SerializationRoundTrip() Content = [ new TextContentBlock { Text = "Result data" }, - new ImageContentBlock { Data = System.Text.Encoding.UTF8.GetBytes("base64data"), MimeType = "image/png" } + new ImageContentBlock + { + Data = System.Text.Encoding.UTF8.GetBytes(Convert.ToBase64String(System.Text.Encoding.UTF8.GetBytes("base64data"))), + MimeType = "image/png", + } ], StructuredContent = JsonElement.Parse("""{"temperature":18,"condition":"cloudy"}"""), IsError = false @@ -246,7 +250,7 @@ public void ToolResultContentBlock_SerializationRoundTrip() var textBlock = Assert.IsType(result.Content[0]); Assert.Equal("Result data", textBlock.Text); var imageBlock = Assert.IsType(result.Content[1]); - Assert.Equal("base64data", System.Text.Encoding.UTF8.GetString(imageBlock.Data.ToArray())); + Assert.Equal("base64data", System.Text.Encoding.UTF8.GetString(imageBlock.DecodedData.ToArray())); Assert.Equal("image/png", imageBlock.MimeType); Assert.NotNull(result.StructuredContent); Assert.Equal(18, result.StructuredContent.Value.GetProperty("temperature").GetInt32()); @@ -292,6 +296,42 @@ public void AudioContentBlock_FromBytes_ThrowsForNullOrWhiteSpaceMimeType(string Assert.ThrowsAny(() => AudioContentBlock.FromBytes((byte[])[1, 2, 3], mimeType!)); } + [Fact] + public void ImageContentBlock_DataSetter_ThrowsForRawUnencodedBytes() + { + // Assigning raw binary directly to Data (instead of going through FromBytes) must fail + // immediately and clearly, rather than silently producing a corrupt JSON payload that only + // fails to parse on the receiving end. Regression test for + // https://github.com/modelcontextprotocol/csharp-sdk/issues/1835. + byte[] rawBytes = new byte[900_000]; + new Random(42).NextBytes(rawBytes); + + var ex = Assert.Throws(() => new ImageContentBlock { Data = rawBytes, MimeType = "image/png" }); + Assert.Equal("Data", ex.ParamName); + Assert.Contains("ImageContentBlock.FromBytes", ex.Message); + } + + [Fact] + public void AudioContentBlock_DataSetter_ThrowsForRawUnencodedBytes() + { + byte[] rawBytes = new byte[900_000]; + new Random(42).NextBytes(rawBytes); + + var ex = Assert.Throws(() => new AudioContentBlock { Data = rawBytes, MimeType = "audio/wav" }); + Assert.Equal("Data", ex.ParamName); + Assert.Contains("AudioContentBlock.FromBytes", ex.Message); + } + + [Fact] + public void ImageContentBlock_DataSetter_ThrowsForOddLengthBase64() + { + // Well-formed base64 alphabet characters are not enough on their own; the length must also + // be consistent with padding rules. + var ex = Assert.Throws(() => + new ImageContentBlock { Data = System.Text.Encoding.UTF8.GetBytes("YWJ"), MimeType = "image/png" }); + Assert.Equal("Data", ex.ParamName); + } + [Fact] public void ImageContentBlock_Deserialization_HandlesEscapedForwardSlashInBase64() { diff --git a/tests/ModelContextProtocol.Tests/Protocol/PromptMessageTests.cs b/tests/ModelContextProtocol.Tests/Protocol/PromptMessageTests.cs index b89f2a424..2246e4f44 100644 --- a/tests/ModelContextProtocol.Tests/Protocol/PromptMessageTests.cs +++ b/tests/ModelContextProtocol.Tests/Protocol/PromptMessageTests.cs @@ -29,7 +29,11 @@ public static void PromptMessage_SerializationRoundTrip_WithImageContent() var original = new PromptMessage { Role = Role.Assistant, - Content = new ImageContentBlock { Data = System.Text.Encoding.UTF8.GetBytes("base64data"), MimeType = "image/png" } + Content = new ImageContentBlock + { + Data = System.Text.Encoding.UTF8.GetBytes(Convert.ToBase64String(System.Text.Encoding.UTF8.GetBytes("base64data"))), + MimeType = "image/png", + } }; string json = JsonSerializer.Serialize(original, McpJsonUtilities.DefaultOptions); @@ -38,7 +42,7 @@ public static void PromptMessage_SerializationRoundTrip_WithImageContent() Assert.NotNull(deserialized); Assert.Equal(Role.Assistant, deserialized.Role); var imageBlock = Assert.IsType(deserialized.Content); - Assert.Equal("base64data", System.Text.Encoding.UTF8.GetString(imageBlock.Data.ToArray())); + Assert.Equal("base64data", System.Text.Encoding.UTF8.GetString(imageBlock.DecodedData.ToArray())); Assert.Equal("image/png", imageBlock.MimeType); } } diff --git a/tests/ModelContextProtocol.Tests/Protocol/ReadResourceResultTests.cs b/tests/ModelContextProtocol.Tests/Protocol/ReadResourceResultTests.cs index fadfba6ca..e6aa721c8 100644 --- a/tests/ModelContextProtocol.Tests/Protocol/ReadResourceResultTests.cs +++ b/tests/ModelContextProtocol.Tests/Protocol/ReadResourceResultTests.cs @@ -23,7 +23,7 @@ public static void ReadResourceResult_SerializationRoundTrip_PreservesAllPropert { Uri = "file:///image.png", MimeType = "image/png", - Blob = System.Text.Encoding.UTF8.GetBytes("base64data") + Blob = System.Text.Encoding.UTF8.GetBytes(Convert.ToBase64String(System.Text.Encoding.UTF8.GetBytes("base64data"))) } ], Meta = new JsonObject { ["key"] = "value" } @@ -43,7 +43,7 @@ public static void ReadResourceResult_SerializationRoundTrip_PreservesAllPropert var blobContent = Assert.IsType(deserialized.Contents[1]); Assert.Equal("file:///image.png", blobContent.Uri); Assert.Equal("image/png", blobContent.MimeType); - Assert.Equal("base64data", System.Text.Encoding.UTF8.GetString(blobContent.Blob.ToArray())); + Assert.Equal("base64data", System.Text.Encoding.UTF8.GetString(blobContent.DecodedData.ToArray())); Assert.NotNull(deserialized.Meta); Assert.Equal("value", (string)deserialized.Meta["key"]!); diff --git a/tests/ModelContextProtocol.Tests/Protocol/ResourceContentsTests.cs b/tests/ModelContextProtocol.Tests/Protocol/ResourceContentsTests.cs index 073f349de..5130605ba 100644 --- a/tests/ModelContextProtocol.Tests/Protocol/ResourceContentsTests.cs +++ b/tests/ModelContextProtocol.Tests/Protocol/ResourceContentsTests.cs @@ -483,7 +483,7 @@ public static void BlobResourceContents_NullMimeType_OmittedFromJson() { Uri = "file:///test.bin", MimeType = null, - Blob = new byte[] { 1, 2, 3 } + Blob = System.Text.Encoding.UTF8.GetBytes(Convert.ToBase64String(new byte[] { 1, 2, 3 })) }; var json = JsonSerializer.Serialize(resource, McpJsonUtilities.DefaultOptions); @@ -509,6 +509,21 @@ public static void BlobResourceContents_Deserialization_HandlesEscapedForwardSla Assert.Equal(originalBytes, blob.DecodedData.ToArray()); } + [Fact] + public static void BlobResourceContents_BlobSetter_ThrowsForRawUnencodedBytes() + { + // Assigning raw binary directly to Blob (instead of going through FromBytes) must fail + // immediately and clearly, rather than silently producing a corrupt JSON payload that only + // fails to parse on the receiving end. Regression test for + // https://github.com/modelcontextprotocol/csharp-sdk/issues/1835. + byte[] rawBytes = new byte[900_000]; + new Random(42).NextBytes(rawBytes); + + var ex = Assert.Throws(() => new BlobResourceContents { Blob = rawBytes, Uri = "demo://files/1/test.pdf" }); + Assert.Equal("Blob", ex.ParamName); + Assert.Contains("BlobResourceContents.FromBytes", ex.Message); + } + [Theory] [MemberData(nameof(ContentBlockTests.Base64TestData), MemberType = typeof(ContentBlockTests))] public static void BlobResourceContents_FromBytes_RoundtripsCorrectly(byte[] originalBytes)