diff --git a/src/mcp/server/mcpserver/utilities/func_metadata.py b/src/mcp/server/mcpserver/utilities/func_metadata.py index 0ffac07c4e..1d6d3627a8 100644 --- a/src/mcp/server/mcpserver/utilities/func_metadata.py +++ b/src/mcp/server/mcpserver/utilities/func_metadata.py @@ -1,3 +1,4 @@ +import base64 import functools import inspect import json @@ -217,11 +218,7 @@ def convert_result(self, result: Any) -> CallToolResult | InputRequiredResult: # The tool hands back Python-side names; the wire (and outputSchema) use aliases. adapter = self._output_adapter(output_model) validated = adapter.validate_python(result, by_alias=True, by_name=True) - if isinstance(validated, BaseModel): - # Dump via the instance so a returned subclass keeps its own fields. - structured_content = validated.model_dump(mode="json", by_alias=True) - else: - structured_content = adapter.dump_python(validated, mode="json", by_alias=True) + structured_content = _dump_structured(validated, adapter) return CallToolResult(content=unstructured_content, structured_content=structured_content) @@ -621,6 +618,69 @@ def _create_wrapped_model(func_name: str, annotation: Any) -> type[BaseModel]: return create_model(model_name, result=annotation) +def _base64_encode(data: bytes | bytearray | memoryview) -> str: + return base64.b64encode(bytes(data)).decode("ascii") + + +def _has_bytes(value: Any) -> bool: + """Whether a python-mode dump (or a raw value) carries bytes anywhere.""" + if isinstance(value, bytes | bytearray | memoryview): + return True + if isinstance(value, dict): + return any(_has_bytes(item) for item in cast("dict[Any, Any]", value).values()) + if isinstance(value, list | tuple | set | frozenset): + return any(_has_bytes(item) for item in cast("list[Any]", value)) + if isinstance(value, BaseModel): + return _has_bytes(value.model_dump(mode="python", by_alias=True)) + return False + + +def _json_safe(value: Any) -> Any: + """Base64-encode bytes leaves; JSON-encode every other leaf as mode="json" would.""" + if isinstance(value, bytes | bytearray | memoryview): + return _base64_encode(cast("bytes | bytearray | memoryview", value)) + if isinstance(value, dict): + return {key: _json_safe(item) for key, item in cast("dict[Any, Any]", value).items()} + if isinstance(value, list | tuple | set | frozenset): + return [_json_safe(item) for item in cast("list[Any]", value)] + if isinstance(value, BaseModel): + return _json_safe(value.model_dump(mode="python", by_alias=True)) + if value is None or isinstance(value, str | int | float | bool): + return value + return json.loads(pydantic_core.to_json(value, fallback=str)) + + +def _result_json_text(value: Any) -> str: + """JSON text for an unstructured result, base64-encoding bytes leaves.""" + if _has_bytes(value): + value = _json_safe(value) + return pydantic_core.to_json(value, fallback=str, indent=2).decode() + + +def _dump_structured(validated: Any, adapter: TypeAdapter[Any]) -> Any: + """Dump validated structured output as JSON, base64-encoding bytes leaves. + + ``mode="json"`` decodes bytes as UTF-8 and raises on binary data, so a + payload carrying bytes is dumped in Python mode with those leaves + base64-encoded first — the encoding every other bytes path in this package + uses (BlobResourceContents, Image/Audio content). A payload without bytes + takes the plain JSON dump, byte for byte. + """ + dumped = ( + # Dump via the instance so a returned subclass keeps its own fields. + validated.model_dump(mode="python", by_alias=True) + if isinstance(validated, BaseModel) + else adapter.dump_python(validated, mode="python", by_alias=True) + ) + if _has_bytes(dumped): + return _json_safe(dumped) + return ( + validated.model_dump(mode="json", by_alias=True) + if isinstance(validated, BaseModel) + else adapter.dump_python(validated, mode="json", by_alias=True) + ) + + def _convert_to_content(result: Any) -> list[ContentBlock]: """Convert a result to a sequence of content objects. @@ -649,7 +709,11 @@ def _convert_to_content(result: Any) -> list[ContentBlock]: ) ) + if isinstance(result, bytes | bytearray | memoryview): + # Binary payloads cannot survive JSON verbatim; publish them base64-encoded. + return [TextContent(type="text", text=_base64_encode(cast("bytes | bytearray | memoryview", result)))] + if not isinstance(result, str): - result = pydantic_core.to_json(result, fallback=str, indent=2).decode() + result = _result_json_text(result) return [TextContent(type="text", text=result)] diff --git a/tests/server/mcpserver/test_func_metadata.py b/tests/server/mcpserver/test_func_metadata.py index dba0637ded..cadb2dfbb0 100644 --- a/tests/server/mcpserver/test_func_metadata.py +++ b/tests/server/mcpserver/test_func_metadata.py @@ -3,6 +3,7 @@ # pyright: reportMissingParameterType=false # pyright: reportUnknownArgumentType=false # pyright: reportUnknownLambdaType=false +import base64 from collections.abc import Callable from dataclasses import dataclass from typing import TYPE_CHECKING, Annotated, Any, Final, NamedTuple, TypedDict @@ -724,6 +725,102 @@ def func_bytes() -> bytes: # pragma: no cover } +PNG_MAGIC = b"\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\x01\x00\x00\x00\x01" + + +def test_bytes_result_is_base64_encoded_in_both_channels(): + """Binary tool results are delivered base64-encoded in both channels. + + Pins the #3554 fix: mode="json" decodes bytes as UTF-8 and used to raise on + the first non-UTF-8 byte, failing the whole tool call. + """ + + def read_thumbnail() -> bytes: # pragma: no cover + return PNG_MAGIC + + meta = func_metadata(read_thumbnail) + encoded = base64.b64encode(PNG_MAGIC).decode() + + result = meta.convert_result(PNG_MAGIC) + + assert isinstance(result, CallToolResult) + assert not result.is_error + assert result.structured_content == {"result": encoded} + assert len(result.content) == 1 + assert isinstance(result.content[0], TextContent) + assert result.content[0].text == encoded + + +def test_utf8_bytes_result_is_base64_encoded(): + """UTF-8-decodable bytes take the same base64 encoding as binary bytes.""" + + def read_note() -> bytes: # pragma: no cover + return b"hello" + + meta = func_metadata(read_note) + + result = meta.convert_result(b"hello") + + assert isinstance(result, CallToolResult) + assert result.structured_content == {"result": base64.b64encode(b"hello").decode()} + + +def test_bytes_field_in_output_model_is_base64_encoded(): + """A bytes field inside an output model is base64-encoded, not a crash.""" + + class Thumb(BaseModel): + data: bytes + + def get_thumbnail() -> Thumb: # pragma: no cover + return Thumb(data=PNG_MAGIC) + + meta = func_metadata(get_thumbnail) + + result = meta.convert_result(Thumb(data=PNG_MAGIC)) + + assert isinstance(result, CallToolResult) + assert not result.is_error + assert result.structured_content == {"data": base64.b64encode(PNG_MAGIC).decode()} + + +def test_bytes_inside_generic_result_is_base64_encoded(): + """bytes nested in a generic result are base64-encoded leaf by leaf.""" + + def two_blobs() -> list[bytes]: # pragma: no cover + return [PNG_MAGIC, b"hello"] + + def blob_map() -> dict[str, bytes]: # pragma: no cover + return {"a": PNG_MAGIC} + + listed = func_metadata(two_blobs).convert_result([PNG_MAGIC, b"hello"]) + mapped = func_metadata(blob_map).convert_result({"a": PNG_MAGIC}) + + assert isinstance(listed, CallToolResult) + assert listed.structured_content == { + "result": [base64.b64encode(PNG_MAGIC).decode(), base64.b64encode(b"hello").decode()] + } + assert isinstance(mapped, CallToolResult) + assert mapped.structured_content == {"a": base64.b64encode(PNG_MAGIC).decode()} + + +def test_result_without_bytes_is_serialized_unchanged(): + """Payloads without bytes keep the plain JSON serialization, byte for byte.""" + + def get_note() -> str: # pragma: no cover + return "hello" + + def get_count() -> int: # pragma: no cover + return 7 + + noted = func_metadata(get_note).convert_result("hello") + counted = func_metadata(get_count).convert_result(7) + + assert isinstance(noted, CallToolResult) + assert noted.structured_content == {"result": "hello"} + assert isinstance(counted, CallToolResult) + assert counted.structured_content == {"result": 7} + + def test_structured_output_generic_types(): """Test structured output with generic types (list, dict, Union, etc.)""" diff --git a/tests/server/mcpserver/test_integration.py b/tests/server/mcpserver/test_integration.py index 2c57aab086..27c8fc611f 100644 --- a/tests/server/mcpserver/test_integration.py +++ b/tests/server/mcpserver/test_integration.py @@ -9,6 +9,7 @@ # pyright: reportUnknownVariableType=false # pyright: reportUnknownArgumentType=false +import base64 import json import pytest @@ -47,6 +48,7 @@ tool_progress, ) from mcp.client import Client, ClientRequestContext, IncomingMessage +from mcp.server.mcpserver import MCPServer pytestmark = pytest.mark.anyio @@ -342,3 +344,27 @@ async def test_structured_output() -> None: assert "sunny" in result_text # condition assert "45" in result_text # humidity assert "5.2" in result_text # wind_speed + + +async def test_binary_tool_result_is_base64_encoded() -> None: + """A tool returning binary bytes delivers base64 in both channels, not an error. + + Pins the #3554 fix: mode="json" used to raise on non-UTF-8 bytes, so the + tool call failed before the payload reached the client. + """ + server = MCPServer("Binary Result") + + @server.tool() + def read_thumbnail() -> bytes: + """Return PNG magic bytes.""" + return b"\x89PNG\r\n\x1a\n\x00\x00" + + async with Client(server) as client: + result = await client.call_tool("read_thumbnail", {}) + + encoded = base64.b64encode(b"\x89PNG\r\n\x1a\n\x00\x00").decode() + assert not result.is_error + assert result.structured_content == {"result": encoded} + assert len(result.content) == 1 + assert isinstance(result.content[0], TextContent) + assert result.content[0].text == encoded