Skip to content

Commit 407419c

Browse files
committed
fix: contain lone-surrogate serialization failures in transport writers
model_dump_json rejects lone surrogates (common from os.fsdecode on non-UTF-8 paths) after every earlier stage has already accepted the value. That exception used to end stdio stdout_writer / stdin_writer (and could break SSE / streamable HTTP event framing), tearing down the session. Serialize through a shared helper that substitutes Internal server error for responses/errors with an id, drops or locally fails requests and notifications, and keeps the writer loop alive. Fixes #3707
1 parent 754254c commit 407419c

8 files changed

Lines changed: 264 additions & 11 deletions

File tree

‎src/mcp/client/stdio.py‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
terminate_windows_process_tree,
3535
)
3636
from mcp.shared.message import SessionMessage
37+
from mcp.shared.wire import serialize_outbound_message
3738

3839
logger = logging.getLogger(__name__)
3940

@@ -172,7 +173,25 @@ async def stdin_writer() -> None:
172173
try:
173174
async with write_stream_reader:
174175
async for session_message in write_stream_reader:
175-
json = session_message.message.model_dump_json(by_alias=True, exclude_unset=True)
176+
# Contain serialization failures so one bad outbound message
177+
# cannot tear down the client session (#3707).
178+
json = serialize_outbound_message(session_message.message)
179+
if json is None:
180+
message = session_message.message
181+
if isinstance(message, types.JSONRPCRequest):
182+
error = SessionMessage(
183+
types.JSONRPCError(
184+
jsonrpc="2.0",
185+
id=message.id,
186+
error=types.ErrorData(
187+
code=types.INTERNAL_ERROR,
188+
message="Failed to serialize request",
189+
),
190+
)
191+
)
192+
with suppress(anyio.ClosedResourceError, anyio.BrokenResourceError):
193+
await read_stream_writer.send(error)
194+
continue
176195
data = (json + "\n").encode(encoding=server.encoding, errors=server.encoding_error_handler)
177196
await process.stdin.send(data)
178197
except (anyio.ClosedResourceError, anyio.BrokenResourceError, OSError):

‎src/mcp/server/sse.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ async def handle_sse(request):
5959
)
6060
from mcp.shared._context_streams import ContextSendStream, create_context_streams
6161
from mcp.shared.message import ServerMessageMetadata, SessionMessage
62+
from mcp.shared.wire import serialize_outbound_message
6263

6364
logger = logging.getLogger(__name__)
6465

@@ -184,10 +185,13 @@ async def sse_writer():
184185

185186
async for session_message in write_stream_reader:
186187
logger.debug(f"Sending message via SSE: {session_message}")
188+
data = serialize_outbound_message(session_message.message)
189+
if data is None:
190+
continue
187191
await sse_stream_writer.send(
188192
{
189193
"event": "message",
190-
"data": session_message.message.model_dump_json(by_alias=True, exclude_unset=True),
194+
"data": data,
191195
}
192196
)
193197

‎src/mcp/server/stdio.py‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ async def run_server():
2727
from mcp.os.win32.utilities import rebind_std_handle_to_fd
2828
from mcp.shared._context_streams import create_context_streams
2929
from mcp.shared.message import SessionMessage
30+
from mcp.shared.wire import serialize_outbound_message
3031

3132
if sys.platform != "win32": # pragma: no branch
3233
import fcntl # pragma: lax no cover - POSIX-only line, uncovered on Windows runners
@@ -200,7 +201,12 @@ async def stdout_writer():
200201
try:
201202
async with write_stream_reader:
202203
async for session_message in write_stream_reader:
203-
json = session_message.message.model_dump_json(by_alias=True, exclude_unset=True)
204+
# One unserializable message (e.g. a lone surrogate) must not
205+
# end the writer; other bad values already become Internal
206+
# server error higher up (#3707).
207+
json = serialize_outbound_message(session_message.message)
208+
if json is None:
209+
continue
204210
await stdout.write(json + "\n")
205211
await stdout.flush()
206212
except anyio.ClosedResourceError: # pragma: no cover

‎src/mcp/server/streamable_http.py‎

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
from mcp.shared._stream_protocols import ReadStream, WriteStream
4747
from mcp.shared.inbound import MCP_PROTOCOL_VERSION_HEADER
4848
from mcp.shared.message import CloseSSEStreamCallback, ServerMessageMetadata, SessionMessage
49+
from mcp.shared.wire import serialize_jsonrpc_message, serialize_outbound_message
4950

5051
logger = logging.getLogger(__name__)
5152

@@ -366,7 +367,10 @@ async def _run_sse_writer(
366367
if priming_event is not None:
367368
await sse_stream_writer.send(priming_event)
368369
async for event_message in request_stream_reader:
369-
await sse_stream_writer.send(self._create_event_data(event_message))
370+
event_data = self._create_event_data(event_message)
371+
if event_data is None:
372+
continue
373+
await sse_stream_writer.send(event_data)
370374
if isinstance(event_message.message, JSONRPCResponse | JSONRPCError):
371375
break
372376
except anyio.ClosedResourceError: # pragma: lax no cover
@@ -401,7 +405,7 @@ def _create_error_response(
401405
)
402406

403407
return Response(
404-
error_response.model_dump_json(by_alias=True, exclude_unset=True),
408+
serialize_jsonrpc_message(error_response),
405409
status_code=status_code,
406410
headers=response_headers,
407411
)
@@ -420,8 +424,21 @@ def _create_json_response(
420424
if self.mcp_session_id:
421425
response_headers[MCP_SESSION_ID_HEADER] = self.mcp_session_id
422426

427+
body = None
428+
if response_message is not None:
429+
body = serialize_outbound_message(response_message)
430+
if body is None:
431+
# Request/notification with no id fallback: still answer the HTTP
432+
# request so the session is not left without a body.
433+
body = serialize_jsonrpc_message(
434+
JSONRPCError(
435+
jsonrpc="2.0",
436+
id=None,
437+
error=ErrorData(code=INTERNAL_ERROR, message="Internal server error"),
438+
)
439+
)
423440
return Response(
424-
response_message.model_dump_json(by_alias=True, exclude_unset=True) if response_message else None,
441+
body,
425442
status_code=status_code,
426443
headers=response_headers,
427444
)
@@ -430,11 +447,19 @@ def _get_session_id(self, request: Request) -> str | None:
430447
"""Extract the session ID from request headers."""
431448
return request.headers.get(MCP_SESSION_ID_HEADER)
432449

433-
def _create_event_data(self, event_message: EventMessage) -> SSEEvent:
434-
"""Create event data dictionary from an EventMessage."""
435-
event_data = {
450+
def _create_event_data(self, event_message: EventMessage) -> SSEEvent | None:
451+
"""Create event data dictionary from an EventMessage.
452+
453+
Returns None when the message cannot be serialized and has no request id
454+
to attach an Internal server error to, so the SSE loop can skip it.
455+
"""
456+
data = serialize_outbound_message(event_message.message)
457+
if data is None:
458+
return None
459+
460+
event_data: SSEEvent = {
436461
"event": "message",
437-
"data": event_message.message.model_dump_json(by_alias=True, exclude_unset=True),
462+
"data": data,
438463
}
439464

440465
# If an event ID was provided, include it
@@ -795,6 +820,8 @@ async def standalone_sse_writer():
795820

796821
# Send the message via SSE
797822
event_data = self._create_event_data(event_message)
823+
if event_data is None:
824+
continue
798825
await sse_stream_writer.send(event_data)
799826
except anyio.ClosedResourceError:
800827
# Session teardown can close the stream while the writer is between dequeues.
@@ -958,6 +985,8 @@ async def replay_sender():
958985
# Define an async callback for sending events
959986
async def send_event(event_message: EventMessage) -> None:
960987
event_data = self._create_event_data(event_message)
988+
if event_data is None:
989+
return
961990
await sse_stream_writer.send(event_data)
962991

963992
# Replay past events and get the stream ID
@@ -986,7 +1015,8 @@ async def send_event(event_message: EventMessage) -> None:
9861015
async with msg_reader:
9871016
async for event_message in msg_reader:
9881017
event_data = self._create_event_data(event_message)
989-
1018+
if event_data is None:
1019+
continue
9901020
await sse_stream_writer.send(event_data)
9911021
finally:
9921022
self._sse_stream_writers.pop(stream_id, None)

‎src/mcp/shared/wire.py‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
"""Outbound JSON-RPC wire serialization shared by transports.
2+
3+
`model_dump_json` is the normal path. Lone surrogates (and any other value that
4+
survives `model_dump(mode="json")` but fails the final UTF-8 encode) must not
5+
tear down a writer task: one bad message costs only itself.
6+
"""
7+
8+
from __future__ import annotations
9+
10+
import logging
11+
12+
from mcp_types import INTERNAL_ERROR, ErrorData, JSONRPCError, JSONRPCMessage, JSONRPCResponse
13+
14+
logger = logging.getLogger(__name__)
15+
16+
17+
def serialize_jsonrpc_message(message: JSONRPCMessage) -> str:
18+
"""Serialize a JSON-RPC message the way transports always have."""
19+
return message.model_dump_json(by_alias=True, exclude_unset=True)
20+
21+
22+
def serialize_outbound_message(message: JSONRPCMessage) -> str | None:
23+
"""Serialize for the wire; never raise for a single unserializable message.
24+
25+
On failure:
26+
- A response or error with an id becomes ``Internal server error`` (same
27+
shape as other handler serialization failures higher in the stack).
28+
- A request, notification, or id-less error returns ``None`` so the caller
29+
can drop it or fail that request locally without closing the session.
30+
"""
31+
try:
32+
return serialize_jsonrpc_message(message)
33+
except Exception:
34+
logger.exception("Failed to serialize outbound JSON-RPC message")
35+
return _fallback_after_serialization_failure(message)
36+
37+
38+
def _fallback_after_serialization_failure(message: JSONRPCMessage) -> str | None:
39+
if isinstance(message, JSONRPCResponse | JSONRPCError) and message.id is not None:
40+
replacement = JSONRPCError(
41+
jsonrpc="2.0",
42+
id=message.id,
43+
error=ErrorData(code=INTERNAL_ERROR, message="Internal server error"),
44+
)
45+
try:
46+
return serialize_jsonrpc_message(replacement)
47+
except Exception: # pragma: no cover
48+
logger.exception("Failed to serialize serialization-error fallback")
49+
return None
50+
return None

‎tests/client/test_stdio.py‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1456,3 +1456,40 @@ async def test_a_graceful_exit_with_a_surviving_child_leaks_no_pipe_fds( # prag
14561456
# Subset, not equality: other machinery may close fds, but never open new
14571457
# ones; a leaked pipe fd would show up as an extra entry.
14581458
assert set(os.listdir("/proc/self/fd")) <= baseline
1459+
1460+
1461+
@pytest.mark.anyio
1462+
async def test_surrogate_in_outbound_request_does_not_tear_down_writer(
1463+
monkeypatch: pytest.MonkeyPatch,
1464+
) -> None:
1465+
"""A lone surrogate in one outbound request fails that request only; later writes continue (#3707)."""
1466+
import os
1467+
1468+
from mcp_types import INTERNAL_ERROR, JSONRPCError
1469+
1470+
surrogate = os.fsdecode(b"a\xffb")
1471+
bad = JSONRPCRequest(
1472+
jsonrpc="2.0",
1473+
id=1,
1474+
method="tools/call",
1475+
params={"name": "echo", "arguments": {"text": surrogate}},
1476+
)
1477+
good = JSONRPCRequest(jsonrpc="2.0", id=2, method="ping")
1478+
process = FakeProcess(on_stdin_close=lambda: process.exit(0))
1479+
install_fake_process(monkeypatch, process)
1480+
1481+
with anyio.fail_after(5):
1482+
async with stdio_client(FAKE_PARAMS) as (read_stream, write_stream):
1483+
await write_stream.send(SessionMessage(bad))
1484+
await anyio.wait_all_tasks_blocked()
1485+
failed = await read_stream.receive()
1486+
assert isinstance(failed, SessionMessage)
1487+
assert isinstance(failed.message, JSONRPCError)
1488+
assert failed.message.id == 1
1489+
assert failed.message.error.code == INTERNAL_ERROR
1490+
assert failed.message.error.message == "Failed to serialize request"
1491+
assert process.written == []
1492+
1493+
await write_stream.send(SessionMessage(good))
1494+
await anyio.wait_all_tasks_blocked()
1495+
assert process.written == [_line(good)]

‎tests/server/test_stdio.py‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -692,3 +692,38 @@ def test_mcpserver_run_stdio_serves_a_modern_connection(monkeypatch: pytest.Monk
692692
# resultType is modern-only: proves the request was served at the discovered version.
693693
assert responses[1].result["tools"] == []
694694
assert responses[1].result["resultType"] == "complete"
695+
696+
697+
@pytest.mark.anyio
698+
async def test_stdio_server_surrogate_in_response_does_not_kill_writer() -> None:
699+
"""A lone surrogate in one outbound response becomes Internal error; the writer continues (#3707)."""
700+
import json
701+
import os
702+
703+
from mcp_types import INTERNAL_ERROR
704+
705+
stdin = io.StringIO()
706+
stdout = io.StringIO()
707+
surrogate = os.fsdecode(b"report-\xff.txt")
708+
709+
with anyio.fail_after(5):
710+
async with stdio_server(stdin=anyio.AsyncFile(stdin), stdout=anyio.AsyncFile(stdout)) as (
711+
read_stream,
712+
write_stream,
713+
):
714+
await write_stream.send(SessionMessage(JSONRPCResponse(jsonrpc="2.0", id=1, result={"text": surrogate})))
715+
await write_stream.send(SessionMessage(JSONRPCResponse(jsonrpc="2.0", id=2, result={"ok": True})))
716+
await write_stream.aclose()
717+
await read_stream.aclose()
718+
719+
stdout.seek(0)
720+
lines = [line.strip() for line in stdout.readlines() if line.strip()]
721+
assert len(lines) == 2
722+
first = json.loads(lines[0])
723+
assert first == {
724+
"jsonrpc": "2.0",
725+
"id": 1,
726+
"error": {"code": INTERNAL_ERROR, "message": "Internal server error"},
727+
}
728+
second = jsonrpc_message_adapter.validate_json(lines[1])
729+
assert second == JSONRPCResponse(jsonrpc="2.0", id=2, result={"ok": True})

‎tests/shared/test_wire.py‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
"""Outbound wire serialization must contain lone-surrogate failures (#3707)."""
2+
3+
import json
4+
import os
5+
6+
import pytest
7+
from mcp_types import (
8+
INTERNAL_ERROR,
9+
ErrorData,
10+
JSONRPCError,
11+
JSONRPCNotification,
12+
JSONRPCRequest,
13+
JSONRPCResponse,
14+
)
15+
16+
from mcp.shared.wire import serialize_jsonrpc_message, serialize_outbound_message
17+
18+
19+
def _surrogate() -> str:
20+
return os.fsdecode(b"report-\xff.txt")
21+
22+
23+
def test_serialize_jsonrpc_message_rejects_lone_surrogate() -> None:
24+
message = JSONRPCResponse(jsonrpc="2.0", id=1, result={"text": _surrogate()})
25+
with pytest.raises(Exception):
26+
serialize_jsonrpc_message(message)
27+
28+
29+
def test_serialize_outbound_response_with_surrogate_becomes_internal_error() -> None:
30+
message = JSONRPCResponse(jsonrpc="2.0", id=7, result={"text": _surrogate()})
31+
wire = serialize_outbound_message(message)
32+
assert wire is not None
33+
body = json.loads(wire)
34+
assert body == {
35+
"jsonrpc": "2.0",
36+
"id": 7,
37+
"error": {"code": INTERNAL_ERROR, "message": "Internal server error"},
38+
}
39+
40+
41+
def test_serialize_outbound_request_with_surrogate_returns_none() -> None:
42+
message = JSONRPCRequest(
43+
jsonrpc="2.0",
44+
id=3,
45+
method="tools/call",
46+
params={"name": "echo", "arguments": {"text": _surrogate()}},
47+
)
48+
assert serialize_outbound_message(message) is None
49+
50+
51+
def test_serialize_outbound_notification_with_surrogate_returns_none() -> None:
52+
message = JSONRPCNotification(
53+
jsonrpc="2.0",
54+
method="notifications/message",
55+
params={"level": "info", "data": _surrogate()},
56+
)
57+
assert serialize_outbound_message(message) is None
58+
59+
60+
def test_serialize_outbound_healthy_message_unchanged() -> None:
61+
message = JSONRPCResponse(jsonrpc="2.0", id=1, result={"ok": True})
62+
assert serialize_outbound_message(message) == serialize_jsonrpc_message(message)
63+
64+
65+
def test_serialize_outbound_error_with_surrogate_in_message_becomes_internal_error() -> None:
66+
message = JSONRPCError(jsonrpc="2.0", id=9, error=ErrorData(code=-32000, message=_surrogate()))
67+
wire = serialize_outbound_message(message)
68+
assert wire is not None
69+
body = json.loads(wire)
70+
assert body["id"] == 9
71+
assert body["error"]["code"] == INTERNAL_ERROR
72+
assert body["error"]["message"] == "Internal server error"

0 commit comments

Comments
 (0)