Skip to content

fix(transport): bound buffered Streamable HTTP responses - #1246

Open
GTanger wants to merge 2 commits into
modelcontextprotocol:mainfrom
GTanger:fix/streamable-http-response-limits
Open

fix(transport): bound buffered Streamable HTTP responses#1246
GTanger wants to merge 2 commits into
modelcontextprotocol:mainfrom
GTanger:fix/streamable-http-response-limits

Conversation

@GTanger

@GTanger GTanger commented Sep 5, 2026

Copy link
Copy Markdown

Summary

Refs #1030. Addresses the Streamable HTTP client JSON/error response scope described in #1180 and redirected to #1030 by the maintainer. The stdio work in #1049 remains separate; this PR should not close the combined tracker on its own.

The built-in clients already bound individual SSE events, but buffer JSON and HTTP error bodies before parsing them. This change adds configurable pre-decode bounds without replacing the transports or adding dependencies.

Changes

  • Add StreamableHttpResponseLimits and transport config setters. Defaults: 64 MiB per complete JSON response, regardless of HTTP status, 64 KiB per HTTP diagnostic prefix, and the unchanged independent 16 MiB SSE-event limit.
  • Share bounded and prefix readers between reqwest and Unix socket backends. JSON is rejected with ResponseBodyTooLarge { limit } if Content-Length exceeds its limit or a chunk would exceed it. Count bytes even when the length is missing or understated; do not drain oversized JSON to EOF.
  • For non-JSON HTTP error pages, read_truncated_body retains at most the diagnostic limit and stops immediately when that prefix is full, without polling for another chunk or EOF. Large legacy 4xx pages can still reach the existing server/discover -> initialize fallback. The fallback eligibility rules are unchanged.
  • application/json error responses use the same complete-body JSON limit as successful responses. Parse the full bounded body before considering discovery fallback or truncating diagnostic text. Never parse a diagnostic prefix as a complete JSON-RPC message. Malformed/non-error JSON gets only a diagnostic prefix; oversized JSON still fails without fallback/retry.
  • Align the Unix socket backend with reqwest by forwarding complete HTTP JSON-RPC errors as StreamableHttpPostResponse::Json, before synthetic discovery fallback. This also fixes the previous backend discrepancy for these errors.
  • Forward limits through all six POST sending sites, including startup, discovery fallback, initialized notifications, ordinary/control requests, and session recovery. AuthClient preserves them across token refresh.
  • Keep the old POST entry points and provide a default implementation of the new trait method that delegates to the old SSE-limit override. Built-in backends apply the new defaults through old entry points too.

Compatibility and limits

Custom HTTP backends remain source-compatible, but must override post_message_with_response_limits to enforce JSON/diagnostic limits; the default cannot bound a body already decoded by custom code. The API docs explain this distinction and the JSON-versus-diagnostic behavior.

Previously accepted JSON bodies above the new default now fail; callers can raise each limit independently. A zero JSON limit accepts only empty bodies. A zero diagnostic limit omits diagnostic text rather than rejecting a non-JSON error response. The defaults are proposed for review.

Existing 202/204, authentication challenge, session-expired, within-limit malformed-success-JSON, and backend-specific success-body-I/O-error behavior is preserved. An error-body I/O failure before the diagnostic limit still uses the existing read-failure placeholder (subject to the diagnostic prefix limit). This does not subsume #1208's separate malformed-JSON proposal.

Limits count bytes yielded by the HTTP backend (decompressed bytes when decompression is enabled). They bound accumulated body length, not backend/decompressor allocations, vector capacity, UTF-8 conversion, or JSON parser allocations, and are not a total RSS bound.

Validation

Linux, Rust 1.96.1 (the repository's pinned 1.96 toolchain), with the same fixed validation lockfile kept out of this PR:

  • 686 passing tests, 0 failures/ignored: 503 rmcp library tests plus 183 integration tests in 19 targeted binaries. This includes 71 cases added by the PR: body readers 12, OAuth forwarding 1, reqwest 39, Unix socket 8, lifecycle/custom-client compatibility 10, and old-public-API diagnostic regression 1.
  • Red/green review regression: the exact same 58 focused integration tests and lockfile on the original PR head 296d1faea6ec1fd3b538eb629e93bd9910725778 compile and produce 31 passes / 27 assertion failures; all 58 pass with the revision. This includes the two real loopback legacy startup regressions.
  • Real loopback startup completes server/discover -> initialize -> notifications/initialized within two seconds after a >64 KiB diagnostic error page, with either a fixed body or an unfinished chunked stream. Oversized JSON on HTTP 200 and 400 still fails within two seconds, with exactly one POST and no fallback/retry.
  • Identical JSON-RPC errors on HTTP 200/400 share exact-limit and N+1 behavior on both backends. Coverage also includes a >16 MiB base64 tool result accepted by the new default, old POST APIs, zero limits, EOF-free diagnostic prefixes, partial-JSON negatives, auth/session precedence, independent SSE limits, and lifecycle limit forwarding.
  • Strict Clippy passes for the affected client library and four focused integration binaries. Both cargo +nightly-2026-06-20 fmt --all --check and the repository hook's cargo +nightly fmt --all --check pass, as does git diff --check.
  • Four no-default-feature builds pass: transport without a built-in backend, Unix-only, reqwest+auth, and both backends+auth+local. Existing feature-specific dead-code warnings remain.

The initial submission's broader server-enabled --all-targets -D warnings Clippy check reported the same 11 diagnostics in unchanged streamable_http_server/tower.rs on both baseline and patch; that broad check was not rerun for this revision. No lint suppression or unrelated server cleanup is included. Full workspace/all-features tests, Python/JS interoperability, and an actual MSRV-1.88 compiler run were not performed locally; CI remains a separate check.

Prepared with AI assistance; reviewed and tested locally before submission.

@GTanger
GTanger requested a review from a team as a code owner September 5, 2026 10:35
@github-actions github-actions Bot added T-test Testing related changes T-core Core library changes T-transport Transport layer changes labels Sep 5, 2026
Comment thread crates/rmcp/src/transport/common/reqwest/streamable_http_client.rs Outdated
Comment thread crates/rmcp/src/transport/common/reqwest/streamable_http_client.rs Outdated
Comment thread crates/rmcp/src/transport/streamable_http_client.rs Outdated
Apply the complete JSON limit on every HTTP status, raise its default to
64 MiB, and truncate diagnostic prefixes without waiting for EOF.
Align Unix socket JSON-RPC error handling with reqwest and add regression
coverage for review feedback.
@GTanger
GTanger force-pushed the fix/streamable-http-response-limits branch from a94edbc to 4500e81 Compare September 8, 2026 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-core Core library changes T-test Testing related changes T-transport Transport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants