Content types that represent binary data are exposing UTF16 encoded strings

[ericstj]

csharp-sdk/src/ModelContextProtocol.Core/Protocol/BlobResourceContents.cs

Lines 25 to 29 in 4a84412

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

csharp-sdk/src/ModelContextProtocol.Core/Protocol/ContentBlock.cs

Lines 377 to 381 in 4a84412

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

csharp-sdk/src/ModelContextProtocol.Core/Protocol/ContentBlock.cs

Lines 399 to 403 in 4a84412

	/// <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; }

[ericstj]

@mikekistler @halter73 @stephentoub -- what do you think about the name Blob. In my opinion the name is too attractive since using this property is almost never what people will want to do, and doing so could be a mistake. It needs to be decoded, but will have the same type users expect - so they might mistake it for the decoded data.

[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?

[ericstj]

Data doesn't need to be ROM since this type will create as a single contiguous array. IMO byte[] is better and more usable. Maybe ROM allows more efficient decoding since you can over allocate the max and then slice. If using ROM avoids a copy it's acceptable.

It will cache it lazily. Tried to mention that in remarks above.

Alternate names - EncodedData, Base64Blob, Base64Utf8Blob. I think EncodedData might be my first choice, but any are fine.