From 5ba7060bc4027d99b28b7ddf0f3da680fad5ab39 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Tue, 20 Oct 2026 00:15:06 +0900 Subject: [PATCH] Strip stdio frames as bytes ## Motivation and Context `StdioTransport` reads `$stdin` as UTF-8 and strips each frame before parsing it, in the read loop of `#open` and in the loop of `#send_request` that reads the client's reply. `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 nothing rescued it there: one frame such as `\xff[]`, or a request with an invalid byte in a string value, ended the read loop and the server process, before the parser could answer it. JSON text must be UTF-8 (RFC 8259, section 8.1), and the Python SDK replaces such bytes on its stdio streams instead of stopping. Frames are now stripped as bytes. A frame that is not JSON gets the `-32700` parse error any other unparsable frame gets, and a value holding such a byte reaches the handlers, which serve or refuse it as over HTTP. ## How Has This Been Tested? `test/mcp/server/transports/stdio_transport_test.rb`: a frame starting with an invalid byte, a frame holding one in a value, and a client reply holding one in a value. Against the previous library all three raise from `String#strip` (the value tests from Ruby 3.2 on). ## Breaking Changes None. --- lib/mcp/server/transports/stdio_transport.rb | 12 +++- .../server/transports/stdio_transport_test.rb | 72 +++++++++++++++++++ 2 files changed, 82 insertions(+), 2 deletions(-) diff --git a/lib/mcp/server/transports/stdio_transport.rb b/lib/mcp/server/transports/stdio_transport.rb index 37d33fb3..03269ab9 100644 --- a/lib/mcp/server/transports/stdio_transport.rb +++ b/lib/mcp/server/transports/stdio_transport.rb @@ -49,7 +49,7 @@ def open end break if line.nil? - line = line.strip + line = strip_frame(line) parsed = parse_line(line) response = parsed ? dispatch_with_era(parsed) : @session.handle_json(line) send_response(response) if response @@ -111,7 +111,7 @@ def send_request(method, params = nil) while @open && (line = read_line($stdin)) begin - parsed = JSON.parse(line.strip, symbolize_names: true) + parsed = JSON.parse(strip_frame(line), symbolize_names: true) rescue JSON::ParserError => e MCP.configuration.exception_reporter.call(e, { error: "Failed to parse response" }) raise @@ -152,6 +152,14 @@ def read_line(io) line end + # Strips a frame as bytes. `$stdin` is read as UTF-8, and `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, so one such + # frame would end the read loop instead of being answered. As bytes, it reaches the parser, which refuses + # what is not JSON with a parse error. + def strip_frame(line) + line.b.strip.force_encoding(Encoding::UTF_8) + end + # Parses a frame once so era classification can inspect its method and `_meta`. # Returns `nil` for frames that are not JSON objects; those fall back to # `ServerSession#handle_json` so protocol-level error responses stay identical. diff --git a/test/mcp/server/transports/stdio_transport_test.rb b/test/mcp/server/transports/stdio_transport_test.rb index 86fbb2c0..65e72c83 100644 --- a/test/mcp/server/transports/stdio_transport_test.rb +++ b/test/mcp/server/transports/stdio_transport_test.rb @@ -226,6 +226,49 @@ class StdioTransportTest < ActiveSupport::TestCase end end + test "open serves a frame holding an invalid byte in a value and keeps reading" do + frames = "{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"ping\",\"params\":{\"x\":\"\xFF\"}}\n" \ + "{\"jsonrpc\":\"2.0\",\"id\":2,\"method\":\"ping\"}\n" + input = StringIO.new(frames.dup.force_encoding(Encoding::UTF_8)) + output = StringIO.new + original_stdin = $stdin + original_stdout = $stdout + + begin + $stdin = input + $stdout = output + @transport.open + + responses = output.string.lines.map { |line| JSON.parse(line, symbolize_names: true) } + assert_equal({ jsonrpc: "2.0", id: 1, result: {} }, responses[0]) + assert_equal({ jsonrpc: "2.0", id: 2, result: {} }, responses[1]) + ensure + $stdin = original_stdin + $stdout = original_stdout + end + end + + test "open answers a frame starting with an invalid byte with -32700 and keeps reading" do + frames = "\xFF[]\n{\"jsonrpc\":\"2.0\",\"id\":2,\"method\":\"ping\"}\n" + input = StringIO.new(frames.dup.force_encoding(Encoding::UTF_8)) + output = StringIO.new + original_stdin = $stdin + original_stdout = $stdout + + begin + $stdin = input + $stdout = output + @transport.open + + responses = output.string.lines.map { |line| JSON.parse(line, symbolize_names: true) } + assert_equal(JsonRpcHandler::ErrorCode::PARSE_ERROR, responses[0][:error][:code]) + assert_equal({ jsonrpc: "2.0", id: 2, result: {} }, responses[1]) + ensure + $stdin = original_stdin + $stdout = original_stdout + end + end + test "handles invalid JSON requests" do invalid_json = "invalid json" output = StringIO.new @@ -295,6 +338,35 @@ class StdioTransportTest < ActiveSupport::TestCase end end + test "send_request reads a client reply holding an invalid byte in a value" do + reader, writer = IO.pipe + output = StringIO.new + original_stdin = $stdin + original_stdout = $stdout + + begin + $stdin = reader + $stdout = output + @transport.instance_variable_set(:@open, true) + + Thread.new do + sleep(0.05) # Wait for request to be written to `StringIO`. + request = JSON.parse(output.string.lines.first, symbolize_names: true) + writer.write("{\"jsonrpc\":\"2.0\",\"id\":\"#{request[:id]}\",\"result\":{\"x\":\"\xFF\"}}\n".b) + writer.flush + end + + result = @transport.send_request("test/method") + + assert_equal("\xFF".b, result[:x].b) + ensure + $stdin = original_stdin + $stdout = original_stdout + writer.close + reader.close + end + end + test "send_request raises on error response from client" do reader, writer = IO.pipe output = StringIO.new