diff --git a/lib/json_rpc_handler.rb b/lib/json_rpc_handler.rb index 2c6c836a..9992ae7b 100644 --- a/lib/json_rpc_handler.rb +++ b/lib/json_rpc_handler.rb @@ -63,12 +63,29 @@ def handle_json(request_json, id_validation_pattern: DEFAULT_ALLOWED_ID_CHARACTE message: "Parse error", data: "Invalid JSON", }) + rescue StandardError + # Last-resort guard so an unexpected handling error returns a JSON-RPC error + # instead of escaping to the transport loop. + response = error_response(id: :unknown_id, id_validation_pattern: id_validation_pattern, error: { + code: ErrorCode::INTERNAL_ERROR, + message: "Internal error", + }) end response&.to_json end def process_request(request, id_validation_pattern:, &method_finder) + # A batch element can be any JSON value; a non-object element cannot carry an id or a method, + # so reject it before the `request[:id]` access below. + unless request.is_a?(Hash) + return error_response(id: :unknown_id, id_validation_pattern: id_validation_pattern, error: { + code: ErrorCode::INVALID_REQUEST, + message: "Invalid Request", + data: "Request must be an object", + }) + end + id = request[:id] error = if !valid_version?(request[:jsonrpc]) diff --git a/lib/mcp/client/stdio.rb b/lib/mcp/client/stdio.rb index e8359687..5bcf6517 100644 --- a/lib/mcp/client/stdio.rb +++ b/lib/mcp/client/stdio.rb @@ -259,7 +259,9 @@ def read_response(request) parsed = JSON.parse(line.strip) - next unless parsed.key?("id") + # A JSON-RPC message is an object; skip a non-object frame (array or scalar) + # the same way as a frame without an id. + next unless parsed.is_a?(Hash) && parsed.key?("id") return parsed if parsed["id"] == request_id end diff --git a/test/json_rpc_handler_test.rb b/test/json_rpc_handler_test.rb index 83a18df5..e823e42f 100644 --- a/test/json_rpc_handler_test.rb +++ b/test/json_rpc_handler_test.rb @@ -682,6 +682,37 @@ assert_equal 3, @response.first[:result] assert_equal(-32600, @response.last.dig(:error, :code)) end + + it "returns an invalid request error for a non-object batch element instead of raising" do + [[], [1], ["x"], [nil], [true]].each do |batch| + handle batch + + assert_equal(-32600, @response.dig(:error, :code)) + assert_equal "Invalid Request", @response.dig(:error, :message) + assert_nil @response[:id] + end + end + + it "handles a valid request followed by a non-object element without raising" do + register("add") { |params| params[:a] + params[:b] } + + handle [ + { jsonrpc: "2.0", id: 100, method: "add", params: { a: 1, b: 2 } }, + [], + ] + + assert @response.is_a?(Array) + assert_equal 3, @response.first[:result] + assert_equal(-32600, @response.last.dig(:error, :code)) + assert_nil @response.last[:id] + end + + it "does not raise from handle_json when a batch element is not an object" do + result = handle_json("[[]]") + + assert_equal(-32600, @response.dig(:error, :code)) + refute_nil result + end end # 7 Examples diff --git a/test/mcp/client/stdio_test.rb b/test/mcp/client/stdio_test.rb index 205fae43..1995222b 100644 --- a/test/mcp/client/stdio_test.rb +++ b/test/mcp/client/stdio_test.rb @@ -146,6 +146,68 @@ def test_send_request_skips_notifications stdout_write.close end + def test_send_request_skips_non_object_frames + stdin_read, stdin_write = IO.pipe + stdout_read, stdout_write = IO.pipe + stderr_read, _ = IO.pipe + + Open3.stubs(:popen3).returns([stdin_write, stdout_read, stderr_read, mock_wait_thread]) + + transport = Stdio.new(command: "ruby", args: ["server.rb"]) + + request = { + jsonrpc: "2.0", + id: "test-id", + method: "tools/list", + } + + server_thread = Thread.new do + init_line = stdin_read.gets + init_request = JSON.parse(init_line) + stdout_write.puts(JSON.generate({ + jsonrpc: "2.0", + id: init_request["id"], + result: { + protocolVersion: "2025-11-25", + capabilities: {}, + serverInfo: { name: "test-server", version: "1.0.0" }, + }, + })) + stdout_write.flush + + # Read initialized notification + stdin_read.gets + + # Read tools/list request + stdin_read.gets + + # Non-object frames are skipped like any other non-matching frame. + stdout_write.puts("[[]]") + stdout_write.puts("null") + stdout_write.flush + + # Then send the actual response + stdout_write.puts(JSON.generate({ + jsonrpc: "2.0", + id: "test-id", + result: { tools: [] }, + })) + stdout_write.flush + end + + transport.connect + response = transport.send_request(request: request) + + assert_equal("test-id", response["id"]) + assert_empty(response.dig("result", "tools")) + ensure + server_thread.join + stdin_read.close + stdin_write.close + stdout_read.close + stdout_write.close + end + def test_send_request_raises_error_when_process_exits stdin_read, stdin_write = IO.pipe stdout_read, stdout_write = IO.pipe diff --git a/test/mcp/server/transports/stdio_transport_test.rb b/test/mcp/server/transports/stdio_transport_test.rb index c9cda78f..f8d81a02 100644 --- a/test/mcp/server/transports/stdio_transport_test.rb +++ b/test/mcp/server/transports/stdio_transport_test.rb @@ -133,6 +133,27 @@ class StdioTransportTest < ActiveSupport::TestCase end end + test "open handles a malformed JSON-RPC batch element and emits an error response" do + input = StringIO.new("[[]]\n") + output = StringIO.new + original_stdin = $stdin + original_stdout = $stdout + + begin + $stdin = input + $stdout = output + @transport.open + + response = JSON.parse(output.string, symbolize_names: true) + assert_nil(response[:id]) + assert_equal(-32600, response[:error][:code]) + assert_equal("Invalid Request", response[:error][:message]) + ensure + $stdin = original_stdin + $stdout = original_stdout + end + end + test "rejects duplicate initialize on the same stdio session with -32600" do first = { jsonrpc: "2.0",