Skip to content

Commit e6d58ce

Browse files
committed
Catch RecursionError at the parse guard; disambiguate envelope vs handler faults in the schema listing
json.loads raises RecursionError, not ValueError, for deeply nested bodies - widen the parse guard so they stay 400 + PARSE_ERROR. The synthetic tools/list now surface-validates its params before dispatch: a mis-shaped envelope is caught up front (debug, client fault), so a ValidationError escaping serve_one can only be handler-origin and gets the error-level log the fail-open skip promises.
1 parent 6c1743e commit e6d58ce

2 files changed

Lines changed: 40 additions & 7 deletions

File tree

src/mcp/server/_streamable_http_modern.py

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
ProgressToken,
4545
RequestId,
4646
)
47+
from mcp_types import methods as _methods
4748
from pydantic import BaseModel, ValidationError
4849
from starlette.requests import Request
4950
from starlette.responses import Response
@@ -242,6 +243,13 @@ async def _tool_input_schema(
242243
CLIENT_CAPABILITIES_META_KEY: verdict.client_capabilities,
243244
}
244245
list_params: dict[str, Any] = {"_meta": meta}
246+
try:
247+
_methods.validate_client_request("tools/list", verdict.protocol_version, list_params)
248+
except ValidationError:
249+
# Client-fault envelope: the real dispatch produces the INVALID_PARAMS
250+
# reply, and anything above a debug line would let clients flood the log.
251+
logger.debug("Mcp-Param header validation skipped: the request envelope fails tools/list validation")
252+
return None
245253
seen_cursors: set[str] = set()
246254
client_info = _typed(Implementation, verdict.client_info)
247255
client_capabilities = _typed(ClientCapabilities, verdict.client_capabilities)
@@ -261,11 +269,6 @@ async def _tool_input_schema(
261269
if tool.get("name") == name:
262270
return tool.get("inputSchema")
263271
cursor = result.get("nextCursor")
264-
except ValidationError:
265-
# Client-fault envelope: the real dispatch produces the INVALID_PARAMS
266-
# reply, and anything above a debug line would let clients flood the log.
267-
logger.debug("Mcp-Param header validation skipped: the request envelope fails tools/list validation")
268-
return None
269272
except Exception:
270273
# Fail-open boundary by design: header validation must never break a
271274
# working call path. Loud, precisely because the skip is fail-open.
@@ -357,8 +360,8 @@ async def handle_modern_request(
357360
body = await request.body()
358361
try:
359362
decoded = json.loads(body)
360-
except ValueError:
361-
# Not just JSONDecodeError: integer literals beyond CPython's digit limit raise bare ValueError.
363+
except (ValueError, RecursionError):
364+
# Not just JSONDecodeError: oversized integer literals raise bare ValueError, deep nesting RecursionError.
362365
rej = JSONRPCError(jsonrpc="2.0", id=None, error=ErrorData(code=PARSE_ERROR, message="Parse error"))
363366
await _write(rej, scope, receive, send)
364367
return

tests/server/test_streamable_http_modern.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -978,3 +978,33 @@ async def test_modern_post_with_an_oversized_integer_literal_is_parse_error_not_
978978
response = await http.post("/mcp", content=body, headers={"content-type": "application/json"})
979979
assert response.status_code == 400
980980
assert response.json()["error"]["code"] == PARSE_ERROR
981+
982+
983+
async def test_modern_tools_call_logs_a_handler_raised_validation_error_loudly(
984+
caplog: pytest.LogCaptureFixture,
985+
) -> None:
986+
"""A ValidationError from inside the tools/list handler is a server fault: the skip is
987+
logged at error level, not mistaken for a client-fault envelope."""
988+
989+
async def broken_list(ctx: ServerRequestContext, params: PaginatedRequestParams | None) -> ListToolsResult:
990+
return ListToolsResult(tools=[Tool.model_validate({"bogus": 1})], ttl_ms=0, cache_scope="public")
991+
992+
server: Server[Any] = Server("test", on_list_tools=broken_list, on_call_tool=_ok_call_tool)
993+
with caplog.at_level(logging.ERROR, logger=_streamable_http_modern.__name__):
994+
async with _asgi_client(server) as http:
995+
response = await http.post(
996+
"/mcp",
997+
json=_tool_call_body({"region": "us"}),
998+
headers=_TOOL_CALL_HEADERS | {"mcp-param-region": "eu"},
999+
)
1000+
assert response.status_code == 200
1001+
assert "Mcp-Param header validation skipped: the tools/list listing failed" in caplog.text
1002+
1003+
1004+
async def test_modern_post_with_deeply_nested_body_is_parse_error_not_a_crash() -> None:
1005+
"""Deep nesting makes json.loads raise RecursionError; still an unparseable body: 400 + PARSE_ERROR."""
1006+
body = b"[" * 100_000 + b"]" * 100_000
1007+
async with _asgi_client(_x_mcp_server()) as http:
1008+
response = await http.post("/mcp", content=body, headers={"content-type": "application/json"})
1009+
assert response.status_code == 400
1010+
assert response.json()["error"]["code"] == PARSE_ERROR

0 commit comments

Comments
 (0)