Skip to content

fix: return HTTP 400 PARSE_ERROR for non-UTF-8 POST bodies#3182

Open
tirthfx wants to merge 1 commit into
modelcontextprotocol:v1.xfrom
tirthfx:fix/v1x-nonutf8-post-body-parse-error
Open

fix: return HTTP 400 PARSE_ERROR for non-UTF-8 POST bodies#3182
tirthfx wants to merge 1 commit into
modelcontextprotocol:v1.xfrom
tirthfx:fix/v1x-nonutf8-post-body-parse-error

Conversation

@tirthfx

@tirthfx tirthfx commented Jul 26, 2026

Copy link
Copy Markdown

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 400 PARSE_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.

AI assistance (Claude Code) was used for this change. I reviewed the diff, ran the reproduction and the test suite myself, and can speak to the change.

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_request in src/mcp/server/streamable_http.py parsed the body with json.loads(body) under except json.JSONDecodeError. For non-UTF-8 bytes, json.loads() fails in its internal decode step and raises UnicodeDecodeError, which is not a JSONDecodeError — so it bypassed the parse-error branch and reached the generic except Exception handler, which returns the 500, logs the traceback, and forwards the exception into the session's incoming stream (producing the second ERROR record, from mcp.server.lowlevel.server).

The change widens the catch to except ValueError. Both json.JSONDecodeError and UnicodeDecodeError subclass ValueError, 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_json and 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 the Parse error: message text for a bad-encoding body now differs between v1.x (CPython's UnicodeDecodeError wording) and main (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?

  • New regression test test_non_utf8_body_returns_parse_error in tests/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 current v1.x tip (assert <HTTPStatus.INTERNAL_SERVER_ERROR: 500> == 400, plus the captured ERROR traceback) and passes with the fix.
  • It lives in the manager tests rather than beside test_json_validation in tests/shared/test_streamable_http.py because that file's server fixture runs in a separate process, where caplog cannot 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 pyright 0 errors.
  • Ran the reproduction script from [v1.x] Backport #1912 and cut a release: a non-UTF-8 POST body on Streamable HTTP gets HTTP 500 INTERNAL_ERROR plus an ERROR cascade instead of the HTTP 400 PARSE_ERROR; the parse broadening ships only on main and the 2.0.0 pre-releases #3150 unchanged against this branch: both the control and the Windows-1252 request now answer HTTP 400 / -32700, with zero ERROR records.

Breaking Changes

None. A request that previously produced HTTP 500 INTERNAL_ERROR now produces HTTP 400 PARSE_ERROR; no successful request changes behavior.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

Targets v1.x rather than main per the CONTRIBUTING branch table (non-breaking bug fixes for v1), and because main and 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.

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 cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Re-trigger cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant