Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 5, 2025

Plan: Fix Binary Data Representation in Protocol Types

Based on the issue analysis, I need to change how binary data is represented in the protocol types to avoid unnecessary UTF-16 transcoding.

Changes Completed:

  • Update BlobResourceContents property

    • Changed Blob from string to ReadOnlyMemory<byte> for base64-encoded UTF-8 bytes
    • Renamed Data to DecodedData for consistency with ContentBlock types
    • Added lazy decoding with caching using Base64.DecodeFromUtf8 (no string intermediate)
    • Added cache invalidation when Blob is set
    • Added Debug.Assert for encode length validation
    • Added FromData factory method for efficient initialization
    • Removed "zero-copy" claims from documentation
  • Update ImageContentBlock.Data property

    • Changed from string to ReadOnlyMemory<byte>
    • Added lazy DecodedData property with Base64.DecodeFromUtf8 (no string intermediate)
    • Added cache invalidation when Data is set
    • Renamed FromImage to FromBytes
    • Removed "zero-copy" claims from documentation
  • Update AudioContentBlock.Data property

    • Changed from string to ReadOnlyMemory<byte>
    • Added lazy DecodedData property with Base64.DecodeFromUtf8 (no string intermediate)
    • Added cache invalidation when Data is set
    • Renamed FromAudio to FromBytes
    • Removed "zero-copy" claims from documentation
  • Created Base64Helpers class

    • Dedupl icated encode/decode logic into Base64Helpers.EncodeToBase64Utf8 and DecodeFromBase64Utf8
    • Includes conditional compilation for NET vs netstandard2.0
    • All three types (BlobResourceContents, ImageContentBlock, AudioContentBlock) use these helpers
  • Fixed AIContentExtensions.cs

    • Updated to use DecodedData property
    • Removed unsafe code from GetUtf8Bytes helper
    • Use span-based Encoding methods with polyfills for netstandard2.0
  • Fixed sample files

    • Moved System.Text to using statements per code review feedback
    • Simplified fully-qualified names
  • Created polyfills

    • Added EncodingExtensions polyfill for span-based GetByteCount and GetBytes methods
    • Enabled AllowUnsafeBlocks in ModelContextProtocol.csproj and tests/Directory.Build.props
  • Fixed AIFunctionMcpServerResource.cs

    • Created GetBase64Utf8Bytes helper to deduplicate DataContent Base64Data extraction
    • Used helper in both switch cases
  • Updated all tests

    • Replaced FromImage with FromBytes
    • All tests passing
  • Build and test

    • Build succeeds across all target frameworks
    • All protocol and binary data tests passing

Summary:

Successfully converted binary data properties from string to ReadOnlyMemory<byte> with complete elimination of UTF-16 transcoding:

  1. Wire format: Base64-encoded UTF-8 bytes stored directly from JSON
  2. Decoding: Direct UTF-8 → binary via Base64.DecodeFromUtf8 (no string intermediate on .NET)
  3. Caching: Lazy decode with result caching
  4. Initialization: Factory methods (FromData, FromBytes) for efficient creation from binary data
  5. Consistency: DecodedData naming across all binary types, FromBytes naming for factory methods
  6. Code reuse: Base64Helpers class deduplicates encode/decode logic
  7. Helper methods: GetBase64Utf8Bytes deduplicates DataContent extraction
Original prompt

This section details on the original issue you should resolve

<issue_title>Content types that represent binary data are exposing UTF16 encoded strings</issue_title>
<issue_description>

/// <summary>
/// Gets or sets the base64-encoded string representing the binary data of the item.
/// </summary>
[JsonPropertyName("blob")]
public required string Blob { get; set; }

/// <summary>
/// Gets or sets the base64-encoded image data.
/// </summary>
[JsonPropertyName("data")]
public required string Data { get; set; }

/// <summary>
/// Gets or sets the base64-encoded audio data.
/// </summary>
[JsonPropertyName("data")]
public required string Data { get; set; }

Potentially more -- https://github.com/search?q=repo%3Amodelcontextprotocol%2Fcsharp-sdk%20%22base64-encoded%22&type=code

Doing this means that the UTF8 data that comes across the wire is transcoded to UTF16 just to be then base64 decoded to bytes when anyone uses it. The intermediate step to UTF16 (.NET string) is unnecessary, wastes cycles, creates garbage, and makes the API harder to use.

Instead these types should keep the data in UTF8 and then have lazy getters that will do the base64-decoding. We can tell folks to use those decoded getter properties.

https://github.com/dotnet/extensions/blob/main/src/Libraries/Microsoft.Extensions.AI.Abstractions/Contents/DataContent.cs follows a similar pattern, though it uses the Uri as it's serialized representation. It seems these MCP types should use the UTF8 encoded data as their serialized representation.

The OpenAI SDK exposes a BinaryData as its representation to deal with this problem https://github.com/openai/openai-dotnet/blob/636075222205d626a770acb1af1e5cc060abd517/src/Custom/Images/GeneratedImage.cs#L23 however that's slightly less usable since it forces the caller to decode.

The proposal here would be to have these be instead:

/// <summary> 
/// Gets or sets the base64-encoded UTF-8 byes representing the binary data of the item.
/// </summary> 
/// <remarks>
/// This is a zero-copy representation of the wire payload of this item.  Setting this value will invalidate any cached value of <see cref="Data" />.
/// </remarks>
[JsonPropertyName("blob")] 
public required ReadOnlyMemory<byte> Blob { get; set; } 

/// <summary> 
/// Gets the decoded data represented by <see cref="Blob" />.
/// </summary>
/// <remarks>
/// Accessing this member will decode the value in <see cref="Blob" /> and cache the result.
/// </remarks>
[JsonIgnore]
public byte[] Data { get; }

</issue_description>

<agent_instructions>Please find any protocol types that expose binary data as string (typically base64 encoded) and follow the cited recommendations for how to expose them differently.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@stephentoub > The proposal here would be to have these be instead:

Should Data also be a ReadOnlyMemory<byte>?

Is Data going to cache the decoded data lazily, or is it going to produce a new array on each access? If the latter, it shouldn't be a property.

what do you think about the name Blob

I'm torn. On the one hand, I agree with you. On the other hand, it's nice to keep it the same or super close to the spec. Offline you'd proposed something like Base64Blob; is that still your first choice?</comment_new>


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 3 commits December 5, 2025 23:00
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix binary data types exposing UTF16 encoded strings Use ReadOnlyMemory<byte> for binary data to eliminate UTF-16 transcoding Dec 5, 2025
Copilot AI requested a review from stephentoub December 5, 2025 23:28
@stephentoub stephentoub requested a review from ericstj December 8, 2025 03:42
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
@stephentoub
Copy link
Contributor

@ericstj, I'm not sure this will actually end up saving much end-to-end. It'd help if some of the data wasn't actually looked at, but if it ends up being looked at, it seems it'll actually make the end-to-end worse.

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
@ericstj
Copy link
Collaborator

ericstj commented Dec 8, 2025

I see this PR is still going though a string when decoding - I thought we had API that would go straight from ROS (base64 UTF8) to byte[] without an intermediate string.

@stephentoub
Copy link
Contributor

I thought we had API that would go straight from ROS (base64 UTF8) to byte[] without an intermediate string.

We have operation status APIs that write into an existing destinatiom buffer, but no one-shot that creates the array. We do for Base64Url but we didn't square the circle for Base64.

@ericstj
Copy link
Collaborator

ericstj commented Dec 8, 2025

Can't we do Base64.GetMaxDecodedFromUtf8Length to get the size, create a byte array, then Base64.DecodeFromUtf8, followed by slice to final actual size?

@stephentoub
Copy link
Contributor

Base64.DecodeFromUtf8

That's the operation status API I was referring to.

Can't we do Base64.GetMaxDecodedFromUtf8Length to get the size, create a byte array, then Base64.DecodeFromUtf8, followed by slice to final actual size?

I thought you'd stated you wanted the decoded data property to be a byte[]. If it is, slicing would entail allocating a whole new array unless the max happened to equal actual, which would happen only ~33% of the time.

If the destination property is a ReadOnlyMemory<byte>, then yes, it can be done that way.

@ericstj
Copy link
Collaborator

ericstj commented Dec 9, 2025

If the destination property is a ReadOnlyMemory<byte>, then yes, it can be done that way.

This is exactly what I was thinking of when mentioning:

If using ROM avoids a copy it's acceptable.

And the current implementation is already exposing as public ReadOnlyMemory<byte> DecodedData so I think we've already committed to this. Let's go ahead and remove those string intermediates. @copilot

Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 9, 2025

If the destination property is a ReadOnlyMemory<byte>, then yes, it can be done that way.

This is exactly what I was thinking of when mentioning:
...

Implemented using Base64.DecodeFromUtf8 to decode directly from UTF-8 bytes without string intermediate in commit 39213fb. The implementation:

  • Uses Base64.GetMaxDecodedFromUtf8Length to allocate the buffer
  • Calls Base64.DecodeFromUtf8 to decode directly
  • Slices to actual size using ReadOnlyMemory when needed (only allocates new array ~33% of time when actual != max)
  • Falls back to optimized string-based approach for .NET Standard 2.0

@ericstj
Copy link
Collaborator

ericstj commented Jan 30, 2026

Test failure is #1222

@ericstj ericstj marked this pull request as ready for review February 2, 2026 18:31
ericstj
ericstj previously approved these changes Feb 5, 2026
@ericstj
Copy link
Collaborator

ericstj commented Feb 5, 2026

@stephentoub when you have a chance would appreciate your pass on this once more, since I'm the last committer.

@ericstj
Copy link
Collaborator

ericstj commented Feb 6, 2026

@copilot please address feedback

…sed encoding, remove System.Text qualifiers

Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 6, 2026

@copilot please address feedback

Addressed all feedback in commit 0a3015f:

  • Renamed Data to DecodedData in BlobResourceContents for consistency
  • Removed unsafe code from AIContentExtensions, using span-based Encoding methods
  • Created EncodingExtensions polyfill for netstandard2.0 support
  • Moved System.Text to using statements in sample files
  • Added Debug.Assert for bytesWritten validation
  • Return _decodedData.Value directly as suggested
  • All binary data tests passing

Copilot AI requested a review from ericstj February 6, 2026 16:23
@ericstj
Copy link
Collaborator

ericstj commented Feb 7, 2026

@copilot please address feedback, and be sure to consider variants of each suggestion.

…dio to FromBytes, add Base64 helpers, fix Encoding references

Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 7, 2026

@copilot please address feedback, and be sure to consider variants of each suggestion.

Addressed all feedback in commit e4f2816:

  • Removed all "zero-copy" claims from documentation
  • Updated Debug.Assert message as suggested
  • Renamed FromImage/FromAudio to FromBytes with updated documentation
  • Created Base64Helpers class to deduplicate encode/decode logic (used by BlobResourceContents, ImageContentBlock, AudioContentBlock)
  • Created GetBase64Utf8Bytes helper to deduplicate DataContent Base64Data extraction
  • Fixed System.Text.Encoding references in Base64Helpers for netstandard2.0
  • Updated all tests to use FromBytes
  • All tests passing

@ericstj ericstj requested a review from stephentoub February 10, 2026 20:55
@ericstj
Copy link
Collaborator

ericstj commented Feb 10, 2026

Ok, I think this is ready for another pass. I looked elsewhere in MCP and found #1259. Not going to tackle that in this PR but will follow up. I think that would create more large garbage strings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Content types that represent binary data are exposing UTF16 encoded strings

5 participants