Skip to content

Commit 5ba7060

Browse files
committed
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.
1 parent 4559336 commit 5ba7060

2 files changed

Lines changed: 82 additions & 2 deletions

File tree

‎lib/mcp/server/transports/stdio_transport.rb‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ def open
4949
end
5050
break if line.nil?
5151

52-
line = line.strip
52+
line = strip_frame(line)
5353
parsed = parse_line(line)
5454
response = parsed ? dispatch_with_era(parsed) : @session.handle_json(line)
5555
send_response(response) if response
@@ -111,7 +111,7 @@ def send_request(method, params = nil)
111111

112112
while @open && (line = read_line($stdin))
113113
begin
114-
parsed = JSON.parse(line.strip, symbolize_names: true)
114+
parsed = JSON.parse(strip_frame(line), symbolize_names: true)
115115
rescue JSON::ParserError => e
116116
MCP.configuration.exception_reporter.call(e, { error: "Failed to parse response" })
117117
raise
@@ -152,6 +152,14 @@ def read_line(io)
152152
line
153153
end
154154

155+
# Strips a frame as bytes. `$stdin` is read as UTF-8, and `String#strip` raises for a UTF-8 string holding
156+
# an invalid byte at either end on every supported Ruby, and anywhere in it from Ruby 3.2 on, so one such
157+
# frame would end the read loop instead of being answered. As bytes, it reaches the parser, which refuses
158+
# what is not JSON with a parse error.
159+
def strip_frame(line)
160+
line.b.strip.force_encoding(Encoding::UTF_8)
161+
end
162+
155163
# Parses a frame once so era classification can inspect its method and `_meta`.
156164
# Returns `nil` for frames that are not JSON objects; those fall back to
157165
# `ServerSession#handle_json` so protocol-level error responses stay identical.

‎test/mcp/server/transports/stdio_transport_test.rb‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,49 @@ class StdioTransportTest < ActiveSupport::TestCase
226226
end
227227
end
228228

229+
test "open serves a frame holding an invalid byte in a value and keeps reading" do
230+
frames = "{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"ping\",\"params\":{\"x\":\"\xFF\"}}\n" \
231+
"{\"jsonrpc\":\"2.0\",\"id\":2,\"method\":\"ping\"}\n"
232+
input = StringIO.new(frames.dup.force_encoding(Encoding::UTF_8))
233+
output = StringIO.new
234+
original_stdin = $stdin
235+
original_stdout = $stdout
236+
237+
begin
238+
$stdin = input
239+
$stdout = output
240+
@transport.open
241+
242+
responses = output.string.lines.map { |line| JSON.parse(line, symbolize_names: true) }
243+
assert_equal({ jsonrpc: "2.0", id: 1, result: {} }, responses[0])
244+
assert_equal({ jsonrpc: "2.0", id: 2, result: {} }, responses[1])
245+
ensure
246+
$stdin = original_stdin
247+
$stdout = original_stdout
248+
end
249+
end
250+
251+
test "open answers a frame starting with an invalid byte with -32700 and keeps reading" do
252+
frames = "\xFF[]\n{\"jsonrpc\":\"2.0\",\"id\":2,\"method\":\"ping\"}\n"
253+
input = StringIO.new(frames.dup.force_encoding(Encoding::UTF_8))
254+
output = StringIO.new
255+
original_stdin = $stdin
256+
original_stdout = $stdout
257+
258+
begin
259+
$stdin = input
260+
$stdout = output
261+
@transport.open
262+
263+
responses = output.string.lines.map { |line| JSON.parse(line, symbolize_names: true) }
264+
assert_equal(JsonRpcHandler::ErrorCode::PARSE_ERROR, responses[0][:error][:code])
265+
assert_equal({ jsonrpc: "2.0", id: 2, result: {} }, responses[1])
266+
ensure
267+
$stdin = original_stdin
268+
$stdout = original_stdout
269+
end
270+
end
271+
229272
test "handles invalid JSON requests" do
230273
invalid_json = "invalid json"
231274
output = StringIO.new
@@ -295,6 +338,35 @@ class StdioTransportTest < ActiveSupport::TestCase
295338
end
296339
end
297340

341+
test "send_request reads a client reply holding an invalid byte in a value" do
342+
reader, writer = IO.pipe
343+
output = StringIO.new
344+
original_stdin = $stdin
345+
original_stdout = $stdout
346+
347+
begin
348+
$stdin = reader
349+
$stdout = output
350+
@transport.instance_variable_set(:@open, true)
351+
352+
Thread.new do
353+
sleep(0.05) # Wait for request to be written to `StringIO`.
354+
request = JSON.parse(output.string.lines.first, symbolize_names: true)
355+
writer.write("{\"jsonrpc\":\"2.0\",\"id\":\"#{request[:id]}\",\"result\":{\"x\":\"\xFF\"}}\n".b)
356+
writer.flush
357+
end
358+
359+
result = @transport.send_request("test/method")
360+
361+
assert_equal("\xFF".b, result[:x].b)
362+
ensure
363+
$stdin = original_stdin
364+
$stdout = original_stdout
365+
writer.close
366+
reader.close
367+
end
368+
end
369+
298370
test "send_request raises on error response from client" do
299371
reader, writer = IO.pipe
300372
output = StringIO.new

0 commit comments

Comments
 (0)