Skip to content

Commit d280b57

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 6d7b3c4 commit d280b57

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

5454
# Reset for each frame: the loop body shares one scope, so a frame whose parse raises would otherwise
5555
# be answered with the previous frame's id.
@@ -137,7 +137,7 @@ def send_request(method, params = nil)
137137

138138
while @open && (line = read_line($stdin))
139139
begin
140-
parsed = JSON.parse(line.strip, symbolize_names: true)
140+
parsed = JSON.parse(strip_frame(line), symbolize_names: true)
141141
rescue JSON::ParserError, EncodingError => e
142142
MCP.configuration.exception_reporter.call(e, { error: "Failed to parse response" })
143143
raise
@@ -178,6 +178,14 @@ def read_line(io)
178178
line
179179
end
180180

181+
# Strips a frame as bytes. `$stdin` is read as UTF-8, and `String#strip` raises for a UTF-8 string holding
182+
# an invalid byte at either end on every supported Ruby, and anywhere in it from Ruby 3.2 on, so one such
183+
# frame would end the read loop instead of being answered. As bytes, it reaches the parser, which refuses
184+
# what is not JSON with a parse error.
185+
def strip_frame(line)
186+
line.b.strip.force_encoding(Encoding::UTF_8)
187+
end
188+
181189
# Parses a frame once so era classification can inspect its method and `_meta`.
182190
# Returns `nil` for frames that are not JSON objects; those fall back to
183191
# `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
@@ -250,6 +250,49 @@ class StdioTransportTest < ActiveSupport::TestCase
250250
end
251251
end
252252

253+
test "open serves a frame holding an invalid byte in a value and keeps reading" do
254+
frames = "{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"ping\",\"params\":{\"x\":\"\xFF\"}}\n" \
255+
"{\"jsonrpc\":\"2.0\",\"id\":2,\"method\":\"ping\"}\n"
256+
input = StringIO.new(frames.dup.force_encoding(Encoding::UTF_8))
257+
output = StringIO.new
258+
original_stdin = $stdin
259+
original_stdout = $stdout
260+
261+
begin
262+
$stdin = input
263+
$stdout = output
264+
@transport.open
265+
266+
responses = output.string.lines.map { |line| JSON.parse(line, symbolize_names: true) }
267+
assert_equal({ jsonrpc: "2.0", id: 1, result: {} }, responses[0])
268+
assert_equal({ jsonrpc: "2.0", id: 2, result: {} }, responses[1])
269+
ensure
270+
$stdin = original_stdin
271+
$stdout = original_stdout
272+
end
273+
end
274+
275+
test "open answers a frame starting with an invalid byte with -32700 and keeps reading" do
276+
frames = "\xFF[]\n{\"jsonrpc\":\"2.0\",\"id\":2,\"method\":\"ping\"}\n"
277+
input = StringIO.new(frames.dup.force_encoding(Encoding::UTF_8))
278+
output = StringIO.new
279+
original_stdin = $stdin
280+
original_stdout = $stdout
281+
282+
begin
283+
$stdin = input
284+
$stdout = output
285+
@transport.open
286+
287+
responses = output.string.lines.map { |line| JSON.parse(line, symbolize_names: true) }
288+
assert_equal(JsonRpcHandler::ErrorCode::PARSE_ERROR, responses[0][:error][:code])
289+
assert_equal({ jsonrpc: "2.0", id: 2, result: {} }, responses[1])
290+
ensure
291+
$stdin = original_stdin
292+
$stdout = original_stdout
293+
end
294+
end
295+
253296
test "handles invalid JSON requests" do
254297
invalid_json = "invalid json"
255298
output = StringIO.new
@@ -319,6 +362,35 @@ class StdioTransportTest < ActiveSupport::TestCase
319362
end
320363
end
321364

365+
test "send_request reads a client reply holding an invalid byte in a value" do
366+
reader, writer = IO.pipe
367+
output = StringIO.new
368+
original_stdin = $stdin
369+
original_stdout = $stdout
370+
371+
begin
372+
$stdin = reader
373+
$stdout = output
374+
@transport.instance_variable_set(:@open, true)
375+
376+
Thread.new do
377+
sleep(0.05) # Wait for request to be written to `StringIO`.
378+
request = JSON.parse(output.string.lines.first, symbolize_names: true)
379+
writer.write("{\"jsonrpc\":\"2.0\",\"id\":\"#{request[:id]}\",\"result\":{\"x\":\"\xFF\"}}\n".b)
380+
writer.flush
381+
end
382+
383+
result = @transport.send_request("test/method")
384+
385+
assert_equal("\xFF".b, result[:x].b)
386+
ensure
387+
$stdin = original_stdin
388+
$stdout = original_stdout
389+
writer.close
390+
reader.close
391+
end
392+
end
393+
322394
test "send_request raises on error response from client" do
323395
reader, writer = IO.pipe
324396
output = StringIO.new

0 commit comments

Comments
 (0)