From bee872408de05b1c069aaad3799189076a7c3696 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ate=C5=9F=20G=C3=B6ral?= Date: Sun, 8 Nov 2026 18:50:33 +0100 Subject: [PATCH] fix: apply max_message_bytes to remaining HTTP responses Apply the configured response limit to notification acknowledgements, replies to server-to-client requests, and session DELETE responses. Count incoming bytes without parsing or retaining these bodies. These discard reads do not load event_stream_parser. Report an over-limit body as RequestHandlerError, as send_request does. Check the size before DELETE status handling, and clear local session state even when close raises. --- docs/_client/transports.md | 5 +- lib/mcp/client/http.rb | 29 ++++- test/mcp/client/http_test.rb | 212 +++++++++++++++++++++++++++++++++++ 3 files changed, 242 insertions(+), 4 deletions(-) diff --git a/docs/_client/transports.md b/docs/_client/transports.md index 4d40cc64..e784f2ad 100644 --- a/docs/_client/transports.md +++ b/docs/_client/transports.md @@ -131,7 +131,10 @@ See the [Progress](/server/progress/) page. `MCP::Client::HTTP.new` accepts an optional `max_message_bytes:` keyword that caps the bytes buffered in memory for a single message from the server - an SSE event or a JSON response body. A message that reaches this limit before completing is rejected as a transport error, preventing unbounded memory growth from -a server that never terminates an SSE event. It defaults to `4 * 1024 * 1024` (4 MiB); raise it if your server returns larger responses. +a server that never terminates an SSE event. The same limit applies to the bodies the client reads and drops: the acknowledgement of a notification or of a response +to a server-to-client request, and the answer to the session DELETE. These bodies are counted in full, regardless of content type, without parsing or loading +`event_stream_parser`. An over-limit DELETE reply raises a transport error before its status is handled, including 404 or 405; replies with those statuses +within the limit are still tolerated. It defaults to `4 * 1024 * 1024` (4 MiB); raise it if your server returns larger responses. `MCP::Client::HTTP.new` also accepts `max_reconnection_wait:`, a budget in seconds for resuming a closed SSE stream. It gates every wait between reconnection attempts, and what is left of it becomes the read timeout of each resumed stream. The server chooses that wait through the SSE `retry:` field, and resuming happens on the calling thread, diff --git a/lib/mcp/client/http.rb b/lib/mcp/client/http.rb index 29e3e02a..18c85e77 100644 --- a/lib/mcp/client/http.rb +++ b/lib/mcp/client/http.rb @@ -545,8 +545,12 @@ def send_request(request:) def send_notification(notification:) method = notification[:method] || notification["method"] - redacting_authorization_header { client.post("", notification, session_headers) } + redacting_authorization_header do + client.post("", notification, session_headers) { |req| req.options.on_data = bounded_on_data } + end nil + rescue MessageTooLargeError => e + raise RequestHandlerError.new(e.message, { method: method }, error_type: :internal_error) rescue Faraday::Error => e raise RequestHandlerError.new( "Failed to send #{method} notification", @@ -573,7 +577,11 @@ def close end begin - redacting_authorization_header { client.delete("", nil, session_headers) } + redacting_authorization_header do + client.delete("", nil, session_headers) { |req| req.options.on_data = bounded_on_data } + end + rescue MessageTooLargeError => e + raise RequestHandlerError.new(e.message, {}, error_type: :internal_error) rescue Faraday::ClientError => e raise unless [404, 405].include?(e.response&.dig(:status)) ensure @@ -1234,7 +1242,22 @@ def dispatch_server_request(message) end def send_client_response(response) - redacting_authorization_header { client.post("", response, session_headers) } + redacting_authorization_header do + client.post("", response, session_headers) { |req| req.options.on_data = bounded_on_data } + end + end + + # Counts discarded response bytes without parsing the body, whatever its content type. + def bounded_on_data + received_bytes = 0 + + proc do |chunk, _received_bytes, _env| + received_bytes += chunk.bytesize + + if received_bytes > @max_message_bytes + raise MessageTooLargeError, "Server response body exceeds #{@max_message_bytes} bytes" + end + end end def parse_json_buffer(buffer, method, params) diff --git a/test/mcp/client/http_test.rb b/test/mcp/client/http_test.rb index 5c063efc..7d2705fe 100644 --- a/test/mcp/client/http_test.rb +++ b/test/mcp/client/http_test.rb @@ -1338,6 +1338,210 @@ def test_send_request_accepts_sse_events_that_each_stay_within_max_message_bytes assert_equal({ "tools" => [] }, response["result"]) end + def test_send_notification_rejects_response_body_exceeding_max_message_bytes + notification = { + jsonrpc: "2.0", + method: MCP::Methods::NOTIFICATIONS_CANCELLED, + params: { requestId: "req-1" }, + } + client = HTTP.new(url: url, max_message_bytes: 64) + + stub_request(:post, url).with(body: notification.to_json).to_return(status: 202, body: "a" * 128) + + error = assert_raises(RequestHandlerError) do + client.send_notification(notification: notification) + end + + assert_includes(error.message, "Server response body exceeds 64 bytes") + assert_equal(:internal_error, error.error_type) + end + + def test_send_request_rejects_pong_acknowledgement_exceeding_max_message_bytes + request = { + jsonrpc: "2.0", + id: "test_id", + method: "tools/list", + } + client = HTTP.new(url: url, max_message_bytes: 128) + + server_ping = { jsonrpc: "2.0", id: 7, method: "ping" } + tools_result = { jsonrpc: "2.0", id: "test_id", result: { tools: [] } } + stub_request(:post, url).with( + body: request.to_json, + ).to_return( + status: 200, + headers: { "Content-Type" => "text/event-stream" }, + body: "data: #{server_ping.to_json}\n\ndata: #{tools_result.to_json}\n\n", + ) + + pong = { jsonrpc: "2.0", id: 7, result: {} } + stub_request(:post, url).with(body: pong.to_json).to_return(status: 202, body: "a" * 256) + + error = assert_raises(RequestHandlerError) do + client.send_request(request: request) + end + + assert_includes(error.message, "Server response body exceeds 128 bytes") + assert_equal(:internal_error, error.error_type) + end + + def test_close_rejects_response_body_exceeding_max_message_bytes_and_still_clears_state + client = HTTP.new(url: url, max_message_bytes: 64) + stub_initialize + client.send_request(request: { jsonrpc: "2.0", id: "1", method: "initialize" }) + + stub_request(:delete, url).to_return(status: 200, body: "a" * 128) + + error = assert_raises(RequestHandlerError) do + client.close + end + + assert_includes(error.message, "Server response body exceeds 64 bytes") + assert_nil(client.session_id) + end + + def test_send_notification_bounds_sse_framed_response_without_parser + client = HTTP.new(url: url, max_message_bytes: 64) + notification = { jsonrpc: "2.0", method: MCP::Methods::NOTIFICATIONS_CANCELLED } + stub_event_stream_parser_unavailable + stub_request(:post, url).with(body: notification.to_json).to_return( + status: 202, + headers: { "Content-Type" => "text/event-stream" }, + body: "data: {}\n\n" * 16, + ) + + error = assert_raises(RequestHandlerError) do + client.send_notification(notification: notification) + end + + assert_includes(error.message, "Server response body exceeds 64 bytes") + assert_equal(:internal_error, error.error_type) + end + + def test_pong_acknowledgement_bounds_sse_framed_response_without_parser + client = HTTP.new(url: url, max_message_bytes: 64) + server_ping = { "jsonrpc" => "2.0", "id" => 7, "method" => "ping" } + pong = { jsonrpc: "2.0", id: 7, result: {} } + stub_event_stream_parser_unavailable + stub_request(:post, url).with(body: pong.to_json).to_return( + status: 202, + headers: { "Content-Type" => "text/event-stream" }, + body: "data: {}\n\n" * 16, + ) + + error = assert_raises(HTTP.const_get(:MessageTooLargeError)) do + client.send(:dispatch_server_request, server_ping) + end + + assert_includes(error.message, "Server response body exceeds 64 bytes") + end + + def test_close_bounds_sse_framed_response_without_parser + client = HTTP.new(url: url, max_message_bytes: 64) + stub_initialize + client.send_request(request: { jsonrpc: "2.0", id: "1", method: "initialize" }) + stub_event_stream_parser_unavailable + stub_request(:delete, url).to_return( + status: 200, + headers: { "Content-Type" => "text/event-stream" }, + body: "data: {}\n\n" * 16, + ) + + error = assert_raises(RequestHandlerError) do + client.close + end + + assert_includes(error.message, "Server response body exceeds 64 bytes") + assert_nil(client.session_id) + assert_nil(client.protocol_version) + end + + def test_discarded_responses_within_limit_do_not_load_parser + client = HTTP.new(url: url, max_message_bytes: 64) + stub_initialize + client.send_request(request: { jsonrpc: "2.0", id: "1", method: "initialize" }) + stub_event_stream_parser_unavailable + stub_request(:post, url).to_return( + status: 202, + headers: { "Content-Type" => "text/event-stream" }, + body: "data: {}\n\n" * 4, + ) + stub_request(:delete, url).to_return( + status: 200, + headers: { "Content-Type" => "text/event-stream" }, + body: "data: {}\n\n" * 4, + ) + + notification = { jsonrpc: "2.0", method: MCP::Methods::NOTIFICATIONS_CANCELLED } + assert_nil(client.send_notification(notification: notification)) + client.send(:dispatch_server_request, { "jsonrpc" => "2.0", "id" => 7, "method" => "ping" }) + client.close + + assert_requested(:post, url, times: 3) + assert_nil(client.session_id) + assert_nil(client.protocol_version) + end + + def test_discarded_response_reader_counts_bytes_across_chunks + client = HTTP.new(url: url, max_message_bytes: 64) + reader = client.send(:bounded_on_data) + reader.call("é" * 16, 0) + reader.call("é" * 16, 0) + + error = assert_raises(HTTP.const_get(:MessageTooLargeError)) do + reader.call("a", 0) + end + + assert_includes(error.message, "Server response body exceeds 64 bytes") + client.send(:bounded_on_data).call("a" * 64, 0) + end + + def test_discarded_response_reader_bounds_the_whole_sse_framed_body + client = HTTP.new(url: url, max_message_bytes: 64) + reader = client.send(:bounded_on_data) + env = stub(response_headers: { "content-type" => "text/event-stream" }) + chunk = "data: {}\n\n" * 4 + reader.call(chunk, chunk.bytesize, env) + + error = assert_raises(HTTP.const_get(:MessageTooLargeError)) do + reader.call(chunk, chunk.bytesize * 2, env) + end + + assert_includes(error.message, "Server response body exceeds 64 bytes") + end + + def test_close_applies_response_size_limit_before_status_errors + [404, 405, 500].each do |status| + client = HTTP.new(url: url, max_message_bytes: 64) + stub_initialize + client.send_request(request: { jsonrpc: "2.0", id: "1", method: "initialize" }) + stub_request(:delete, url).to_return(status: status, body: "a" * 128) + + error = assert_raises(RequestHandlerError) do + client.close + end + + assert_includes(error.message, "Server response body exceeds 64 bytes") + assert_equal(:internal_error, error.error_type) + assert_nil(client.session_id) + assert_nil(client.protocol_version) + end + end + + def test_close_tolerates_404_and_405_response_bodies_within_max_message_bytes + [404, 405].each do |status| + client = HTTP.new(url: url, max_message_bytes: 64) + stub_initialize + client.send_request(request: { jsonrpc: "2.0", id: "1", method: "initialize" }) + stub_request(:delete, url).to_return(status: status, body: "a" * 64) + + client.close + + assert_nil(client.session_id) + assert_nil(client.protocol_version) + end + end + def test_sse_stream_rejects_unterminated_event_exceeding_max_message_bytes stream = HTTP.const_get(:SSEStream).new(abortable: false, max_message_bytes: 64) env = stub(response_headers: { "content-type" => "text/event-stream" }) @@ -3222,6 +3426,14 @@ def stub_notification .to_return(status: 202, body: "") end + def stub_event_stream_parser_unavailable + HTTP.any_instance.stubs(:require).with("faraday").returns(true) + HTTP.any_instance.stubs(:require).with("event_stream_parser").raises(LoadError, "cannot load event_stream_parser") + HTTP.const_get(:SSEStream).any_instance.stubs(:require).with("event_stream_parser") + .raises(LoadError, "cannot load event_stream_parser") + EventStreamParser::Parser.expects(:new).never + end + def stub_discover stub_request(:post, url).with do |req| JSON.parse(req.body)["method"] == "server/discover"