Skip to content

Commit ce91e6f

Browse files
committed
fix: return HTTP 400 PARSE_ERROR for non-UTF-8 POST bodies
The Streamable HTTP POST handler parsed the request body with json.loads() under `except json.JSONDecodeError`. When the body bytes are not valid UTF-8, json.loads() raises UnicodeDecodeError from its internal decode step, which is not a JSONDecodeError, so it bypassed the parse-error branch and reached the generic exception handler. The client got an unexplained HTTP 500 INTERNAL_ERROR and the server logged a traceback at ERROR plus a second ERROR record from the forwarded exception, while a merely malformed UTF-8 body got a clean HTTP 400. Widen the catch to ValueError. Both json.JSONDecodeError and UnicodeDecodeError subclass it, so a body that cannot be parsed for either reason is now answered as the client error it is, with no ERROR-level server logging. Fixes #3150 Github-Issue: #3150 Reported-by: Aleksandr Filippov
1 parent c0c5a9d commit ce91e6f

2 files changed

Lines changed: 58 additions & 2 deletions

File tree

src/mcp/server/streamable_http.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,9 @@ async def _handle_post_request(self, scope: Scope, request: Request, receive: Re
492492

493493
try:
494494
raw_message = json.loads(body)
495-
except json.JSONDecodeError as e:
495+
except ValueError as e:
496+
# Both json.JSONDecodeError (bad syntax) and UnicodeDecodeError (body bytes
497+
# that are not valid UTF-8) subclass ValueError; both are client errors.
496498
response = self._create_error_response(f"Parse error: {str(e)}", HTTPStatus.BAD_REQUEST, PARSE_ERROR)
497499
await response(scope, receive, send)
498500
return

tests/server/test_streamable_http_manager.py

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""Tests for StreamableHTTPSessionManager."""
22

33
import json
4+
import logging
45
from collections.abc import Iterator
56
from typing import Any
67
from unittest.mock import AsyncMock, patch
@@ -19,7 +20,7 @@
1920
RequestBodyLimitMiddleware,
2021
StreamableHTTPSessionManager,
2122
)
22-
from mcp.types import INVALID_REQUEST
23+
from mcp.types import INVALID_REQUEST, PARSE_ERROR
2324

2425

2526
@pytest.mark.anyio
@@ -442,6 +443,59 @@ async def mock_receive():
442443
assert error_data["error"]["message"] == "Session not found"
443444

444445

446+
@pytest.mark.anyio
447+
async def test_non_utf8_body_returns_parse_error(caplog: pytest.LogCaptureFixture):
448+
"""A POST body that is not valid UTF-8 is a client error, not a server error.
449+
450+
json.loads() raises UnicodeDecodeError rather than json.JSONDecodeError for such a
451+
body, so it used to escape the parse-error branch and surface as HTTP 500 with an
452+
ERROR-level traceback.
453+
"""
454+
app = Server("test-non-utf8-body")
455+
manager = StreamableHTTPSessionManager(app=app, stateless=True)
456+
457+
async with manager.run():
458+
sent_messages: list[Message] = []
459+
response_body = b""
460+
461+
async def mock_send(message: Message):
462+
nonlocal response_body
463+
sent_messages.append(message)
464+
if message["type"] == "http.response.body":
465+
response_body += message.get("body", b"")
466+
467+
scope: Scope = {
468+
"type": "http",
469+
"method": "POST",
470+
"path": "/mcp",
471+
"headers": [
472+
(b"content-type", b"application/json"),
473+
(b"accept", b"application/json, text/event-stream"),
474+
],
475+
}
476+
477+
# Valid JSON syntax, but encoded as Windows-1252: the em dash is byte 0x97.
478+
body = '{"jsonrpc": "2.0", "id": 1, "method": "x — y"}'.encode("cp1252")
479+
480+
async def mock_receive():
481+
return {"type": "http.request", "body": body, "more_body": False}
482+
483+
with caplog.at_level(logging.DEBUG):
484+
await manager.handle_request(scope, mock_receive, mock_send)
485+
486+
response_start = next((msg for msg in sent_messages if msg["type"] == "http.response.start"), None)
487+
assert response_start is not None, "Should have sent a response"
488+
assert response_start["status"] == 400
489+
490+
error_data = json.loads(response_body)
491+
assert error_data["jsonrpc"] == "2.0"
492+
assert error_data["error"]["code"] == PARSE_ERROR
493+
assert error_data["error"]["message"].startswith("Parse error:")
494+
495+
error_records = [record for record in caplog.records if record.levelno >= logging.ERROR]
496+
assert error_records == [], f"Unparseable body should not log at ERROR: {[r.getMessage() for r in error_records]}"
497+
498+
445499
@pytest.mark.anyio
446500
async def test_idle_session_is_reaped():
447501
"""After idle timeout fires, the session returns 404."""

0 commit comments

Comments
 (0)