fix: return HTTP 400 PARSE_ERROR for non-UTF-8 POST bodies - #3182
fix: return HTTP 400 PARSE_ERROR for non-UTF-8 POST bodies#3182tirthfx wants to merge 3 commits into
Conversation
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 modelcontextprotocol#3150 Github-Issue: modelcontextprotocol#3150 Reported-by: Aleksandr Filippov
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
| @@ -492,7 +492,9 @@ async def _handle_post_request(self, scope: Scope, request: Request, receive: Re | |||
|
|
|||
| try: | |||
| raw_message = json.loads(body) | |||
There was a problem hiding this comment.
json.loads(body) still accepts UTF-16 and UTF-32 bytes (documented Python behavior), so this does not cover all non-UTF-8 bodies: a UTF-16/32 JSON-RPC request is parsed, passes JSONRPCMessage.model_validate, and continues through handling even though MCP requires every JSON-RPC message to be UTF-8. I reproduced both encodings at this head; pydantic_core.from_json rejects the same bytes. Could we decode explicitly with body.decode("utf-8") before json.loads (or reuse #1912 parser) and parameterize the regression test for UTF-16/32 as well?
There was a problem hiding this comment.
Good catch, thank you — you're right that json.loads's own encoding auto-detection is the gap here, not just the except clause. A UTF-16/32 body that happens to be valid JSON never raises anything, so it sails through even though it shouldn't per spec. I'll update this to decode explicitly with body.decode("utf-8") before json.loads and parameterize the regression test for UTF-16/32 as you suggested.
Appreciate you digging into this — would be really great to stay in touch if you're open to it.
json.loads() auto-detects UTF-8/16/32 per RFC 4627 sec. 3, so a POST body that is valid JSON when read as UTF-16 or UTF-32 was parsed and accepted even though the MCP spec requires UTF-8. Decoding the body explicitly before parsing closes that gap; UnicodeDecodeError still subclasses ValueError, so it's caught by the existing parse-error branch and reported as HTTP 400 PARSE_ERROR. Parameterizes the regression test across cp1252, utf-16, and utf-32. Reported-by: iamroylim
2 participants
Fixes #3150
A POST body that is syntactically valid JSON but encoded in something other than UTF-8 is answered with HTTP 500
INTERNAL_ERROR(-32603) instead of HTTP 400PARSE_ERROR(-32700), and logs two ERROR-level records including a full traceback. This widens the parse-error catch by one line so such a body is treated as the client error it is.Motivation and Context
The common trigger is Windows-1252 punctuation — an em dash (
0x97) or curly quotes — inside a string value of otherwise well-formed JSON, produced by a client serializing with a legacy default codepage. A body that is merely malformed but valid UTF-8 already gets a clean HTTP 400 with an actionable message, so today the more understandable encoding mistake gets the worse experience: an unexplained 500 plus a server-side ERROR cascade that reads as a server defect to operators.Root cause:
_handle_post_requestinsrc/mcp/server/streamable_http.pyparsed the body withjson.loads(body)underexcept json.JSONDecodeError. For non-UTF-8 bytes,json.loads()fails in its internal decode step and raisesUnicodeDecodeError, which is not aJSONDecodeError— so it bypassed the parse-error branch and reached the genericexcept Exceptionhandler, which returns the 500, logs the traceback, and forwards the exception into the session's incoming stream (producing the second ERROR record, frommcp.server.lowlevel.server).The change widens the catch to
except ValueError. Bothjson.JSONDecodeErrorandUnicodeDecodeErrorsubclassValueError, so a body that cannot be parsed for either reason now takes the same client-error path.Why this rather than cherry-picking #1912, which the issue proposes as its headline ask: #1912 swaps the parser to
pydantic_core.from_jsonand was motivated by performance, not this behavior. On a branch that takes only non-breaking bug and security fixes, widening the existing catch reaches the identical behavioral outcome with a smaller surface and no new import or parser swap — this is the "minimal alternative" the issue itself describes. The visible trade-off is that theParse error:message text for a bad-encoding body now differs betweenv1.x(CPython'sUnicodeDecodeErrorwording) andmain(pydantic-core's wording); the HTTP status and JSON-RPC error code match. Happy to switch to the cherry-pick if you would rather keep the branches aligned on the parser itself.The issue also asks for a v1.x patch release containing this. That is a maintainer action and is not part of this PR.
How Has This Been Tested?
test_non_utf8_body_returns_parse_errorintests/server/test_streamable_http_manager.py: POSTs a Windows-1252 body and asserts HTTP 400, JSON-RPC code-32700, and zero ERROR-level log records. Verified it fails on the currentv1.xtip (assert <HTTPStatus.INTERNAL_SERVER_ERROR: 500> == 400, plus the captured ERROR traceback) and passes with the fix.test_json_validationintests/shared/test_streamable_http.pybecause that file's server fixture runs in a separate process, wherecaplogcannot observe the server-side records this fix is meant to eliminate. Happy to add an HTTP-level test there too if you'd prefer it alongside the existing parse tests.uv run --frozen pytest— 1152 passed, 95 skipped, 1 xfailed.uv run --frozen ruff check .clean;ruff format .reports no changes;uv run --frozen pyright0 errors.-32700, with zero ERROR records.Breaking Changes
None. A request that previously produced HTTP 500
INTERNAL_ERRORnow produces HTTP 400PARSE_ERROR; no successful request changes behavior.Types of changes
Checklist
Additional context
Targets
v1.xrather thanmainper the CONTRIBUTING branch table (non-breaking bug fixes for v1), and becausemainand the 2.0.0 pre-releases already have the corrected behavior via #1912.Scope is deliberately limited to the Streamable HTTP POST path in #3150. #3142 tracks a related report in the same ERROR-cascade family and is not addressed here.