Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion docs/_client/transports.md
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
29 changes: 26 additions & 3 deletions lib/mcp/client/http.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
212 changes: 212 additions & 0 deletions test/mcp/client/http_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A DELETE answered 404 or 405 with an over-limit body now raises before raise_error classifies the status, so a case close used to tolerate becomes a RequestHandlerError. If that precedence is intended, a test should pin it; the PR body's "dependency requirements do not change" also needs the parser note above.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The order is intentional and pinned by test_close_applies_response_size_limit_before_status_errors: over-limit 404, 405, and 500 replies raise RequestHandlerError before status handling. test_close_tolerates_404_and_405_response_bodies_within_max_message_bytes pins the unchanged normal-size tolerance. Local session state is cleared in both cases. The PR body and docs now state that order and explain that these discard reads do not need event_stream_parser.

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" })
Expand Down Expand Up @@ -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"
Expand Down
Loading