Skip to content

Commit e757931

Browse files
committed
Strip stdio client frames as bytes
## Motivation and Context `MCP::Client::Stdio` reads the server's stdout as UTF-8 and strips each frame before parsing it. `String#strip` raises for a UTF-8 string holding an invalid byte at either end on every supported Ruby, and anywhere in it from Ruby 3.2 on, and the read loop rescued only `JSON::ParserError`. A server writing such a frame therefore failed the awaited request with `ArgumentError` or `Encoding::CompatibilityError` instead of the transport's own `RequestHandlerError`, which callers rescue. The server transport strips its frames the same way and has the same change separately. Frames are now stripped as bytes. A frame that is not JSON fails the request with the transport's parse failure, as any other unparsable frame does, and a response holding such a byte in a value is returned with the byte kept. ## How Has This Been Tested? `test/mcp/client/stdio_test.rb`: a frame starting with an invalid byte and a response holding one in a value. Against the previous library the first raises `ArgumentError` and the second `Encoding::CompatibilityError`. ## Breaking Changes None.
1 parent 0e59bb4 commit e757931

2 files changed

Lines changed: 44 additions & 1 deletion

File tree

‎lib/mcp/client/stdio.rb‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@ def read_response(request)
434434
line = read_line(method, params)
435435
raise_connection_error!(method, params) if line.nil?
436436

437-
parsed = JSON.parse(line.strip)
437+
parsed = JSON.parse(strip_frame(line))
438438

439439
# A frame carrying `method` is a request or notification, never the awaited response,
440440
# whatever its id says: JSON-RPC id spaces are per sender, so a server-chosen ping id
@@ -534,6 +534,14 @@ def read_line(method, params)
534534
)
535535
end
536536

537+
# Strips a frame as bytes. The server's stdout is read as UTF-8, and `String#strip` raises for a UTF-8 string
538+
# holding an invalid byte at either end on every supported Ruby, and anywhere in it from Ruby 3.2 on, which would
539+
# surface as that exception instead of the client's own error. As bytes, the frame reaches the parser, which
540+
# refuses what is not JSON with the client's parse failure.
541+
def strip_frame(line)
542+
line.b.strip.force_encoding(Encoding::UTF_8)
543+
end
544+
537545
def raise_connection_error!(method, params)
538546
raise RequestHandlerError.new(
539547
"Server process closed stdout unexpectedly",

‎test/mcp/client/stdio_test.rb‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -709,6 +709,41 @@ def test_send_request_raises_error_on_invalid_json
709709
stdout_write.close
710710
end
711711

712+
def test_send_request_raises_its_parse_error_on_a_frame_starting_with_an_invalid_byte
713+
server = lambda do |requests, frames|
714+
answer_handshake(requests, frames)
715+
request = JSON.parse(requests.gets)
716+
frames.write("\xFF".b + JSON.generate({ jsonrpc: "2.0", id: request["id"], result: {} }) + "\n")
717+
frames.flush
718+
end
719+
720+
with_scripted_server(server) do |transport|
721+
transport.connect
722+
error = assert_raises(RequestHandlerError) do
723+
transport.send_request(request: { jsonrpc: "2.0", id: "test-id", method: "tools/list" })
724+
end
725+
726+
assert_equal("Failed to parse server response", error.message)
727+
assert_instance_of(JSON::ParserError, error.original_error)
728+
end
729+
end
730+
731+
def test_send_request_returns_a_response_holding_an_invalid_byte_in_a_value
732+
server = lambda do |requests, frames|
733+
answer_handshake(requests, frames)
734+
request = JSON.parse(requests.gets)
735+
frames.write("{\"jsonrpc\":\"2.0\",\"id\":#{JSON.generate(request["id"])},\"result\":{\"x\":\"a\xFFb\"}}\n".b)
736+
frames.flush
737+
end
738+
739+
with_scripted_server(server) do |transport|
740+
transport.connect
741+
response = transport.send_request(request: { jsonrpc: "2.0", id: "test-id", method: "tools/list" })
742+
743+
assert_equal("a\xFFb".b, response.dig("result", "x").b)
744+
end
745+
end
746+
712747
def test_send_request_raises_error_on_a_response_that_is_not_json_rpc_2_0
713748
answers = [
714749
{ result: { tools: [] } },

0 commit comments

Comments
 (0)