Merge pull request #625 from koic/strip_stdio_frames_as_bytes

Strip stdio frames as bytes

Author: koic

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)
 
             # Reset for each frame: the loop body shares one scope, so a frame whose parse raises would otherwise
             # be answered with the previous frame's id.
@@ -137,7 +137,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, EncodingError => e
               MCP.configuration.exception_reporter.call(e, { error: "Failed to parse response" })
               raise
@@ -178,6 +178,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.

test/mcp/server/transports/stdio_transport_test.rb

@@ -250,6 +250,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
@@ -319,6 +362,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