Skip to content

Commit 8875990

Browse files
committed
Handle non-object JSON-RPC messages without raising
## Motivation and Context A JSON-RPC message is an object, but two stdio-facing paths assumed that and raised on a non-object value parsed from an incoming frame: - `JsonRpcHandler.handle` maps each element of a batch array through `process_request`, which reads `request[:id]` assuming a Hash. A non-object batch element (for example `[[]]` or `[5]`) makes `[][:id]` raise `TypeError`. `handle_json` rescued only `JSON::ParserError`, so the error escaped; over stdio it escaped the `StdioTransport#open` read loop. The HTTP transport already rejects array bodies before dispatch, so this surfaced only on stdio. - `MCP::Client::Stdio#read_response` parses each frame from the server and calls `parsed.key?("id")`, which raises `NoMethodError` when the server emits a non-object frame. Only `JSON::ParserError` was rescued, so one malformed line from the server escaped. Both are robustness gaps: a malformed frame should be reported or skipped, not raise an unhandled exception out of the transport. - `process_request` now returns an `INVALID_REQUEST` error response for a non-object request before touching `request[:id]`, so a malformed batch element becomes a normal per-element error. - `handle_json` gains a last-resort `StandardError` rescue that returns an `INTERNAL_ERROR` response, so an unexpected handling error still produces a JSON-RPC reply. - `read_response` skips a non-object frame the same way it skips a frame without an id. ## How Has This Been Tested? New tests in `test/json_rpc_handler_test.rb` cover non-object batch elements (`[[]]`, `[5]`, `["x"]`, `[nil]`, `[true]`), a valid request followed by one non-object element, and `handle_json("[[]]")` returning an error instead of raising. A test in `test/mcp/server/transports/stdio_transport_test.rb` drives `open` with a `[[]]` line and asserts it emits an error response without raising. A test in `test/mcp/client/stdio_test.rb` has the server emit non-object frames before the response and asserts the client skips them and still returns the response. ## Breaking Changes None. A well-formed request, batch, or response is handled exactly as before; only a non-object message now yields a JSON-RPC error on the server or is skipped by the client instead of raising.
1 parent 61233ed commit 8875990

5 files changed

Lines changed: 134 additions & 1 deletion

File tree

lib/json_rpc_handler.rb

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,29 @@ def handle_json(request_json, id_validation_pattern: DEFAULT_ALLOWED_ID_CHARACTE
6363
message: "Parse error",
6464
data: "Invalid JSON",
6565
})
66+
rescue StandardError
67+
# Last-resort guard so an unexpected handling error returns a JSON-RPC error
68+
# instead of escaping to the transport loop.
69+
response = error_response(id: :unknown_id, id_validation_pattern: id_validation_pattern, error: {
70+
code: ErrorCode::INTERNAL_ERROR,
71+
message: "Internal error",
72+
})
6673
end
6774

6875
response&.to_json
6976
end
7077

7178
def process_request(request, id_validation_pattern:, &method_finder)
79+
# A batch element can be any JSON value; a non-object element cannot carry an id or a method,
80+
# so reject it before the `request[:id]` access below.
81+
unless request.is_a?(Hash)
82+
return error_response(id: :unknown_id, id_validation_pattern: id_validation_pattern, error: {
83+
code: ErrorCode::INVALID_REQUEST,
84+
message: "Invalid Request",
85+
data: "Request must be an object",
86+
})
87+
end
88+
7289
id = request[:id]
7390

7491
error = if !valid_version?(request[:jsonrpc])

lib/mcp/client/stdio.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,9 @@ def read_response(request)
259259

260260
parsed = JSON.parse(line.strip)
261261

262-
next unless parsed.key?("id")
262+
# A JSON-RPC message is an object; skip a non-object frame (array or scalar)
263+
# the same way as a frame without an id.
264+
next unless parsed.is_a?(Hash) && parsed.key?("id")
263265

264266
return parsed if parsed["id"] == request_id
265267
end

test/json_rpc_handler_test.rb

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -682,6 +682,37 @@
682682
assert_equal 3, @response.first[:result]
683683
assert_equal(-32600, @response.last.dig(:error, :code))
684684
end
685+
686+
it "returns an invalid request error for a non-object batch element instead of raising" do
687+
[[], [1], ["x"], [nil], [true]].each do |batch|
688+
handle batch
689+
690+
assert_equal(-32600, @response.dig(:error, :code))
691+
assert_equal "Invalid Request", @response.dig(:error, :message)
692+
assert_nil @response[:id]
693+
end
694+
end
695+
696+
it "handles a valid request followed by a non-object element without raising" do
697+
register("add") { |params| params[:a] + params[:b] }
698+
699+
handle [
700+
{ jsonrpc: "2.0", id: 100, method: "add", params: { a: 1, b: 2 } },
701+
[],
702+
]
703+
704+
assert @response.is_a?(Array)
705+
assert_equal 3, @response.first[:result]
706+
assert_equal(-32600, @response.last.dig(:error, :code))
707+
assert_nil @response.last[:id]
708+
end
709+
710+
it "does not raise from handle_json when a batch element is not an object" do
711+
result = handle_json("[[]]")
712+
713+
assert_equal(-32600, @response.dig(:error, :code))
714+
refute_nil result
715+
end
685716
end
686717

687718
# 7 Examples

test/mcp/client/stdio_test.rb

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,68 @@ def test_send_request_skips_notifications
146146
stdout_write.close
147147
end
148148

149+
def test_send_request_skips_non_object_frames
150+
stdin_read, stdin_write = IO.pipe
151+
stdout_read, stdout_write = IO.pipe
152+
stderr_read, _ = IO.pipe
153+
154+
Open3.stubs(:popen3).returns([stdin_write, stdout_read, stderr_read, mock_wait_thread])
155+
156+
transport = Stdio.new(command: "ruby", args: ["server.rb"])
157+
158+
request = {
159+
jsonrpc: "2.0",
160+
id: "test-id",
161+
method: "tools/list",
162+
}
163+
164+
server_thread = Thread.new do
165+
init_line = stdin_read.gets
166+
init_request = JSON.parse(init_line)
167+
stdout_write.puts(JSON.generate({
168+
jsonrpc: "2.0",
169+
id: init_request["id"],
170+
result: {
171+
protocolVersion: "2025-11-25",
172+
capabilities: {},
173+
serverInfo: { name: "test-server", version: "1.0.0" },
174+
},
175+
}))
176+
stdout_write.flush
177+
178+
# Read initialized notification
179+
stdin_read.gets
180+
181+
# Read tools/list request
182+
stdin_read.gets
183+
184+
# Non-object frames are skipped like any other non-matching frame.
185+
stdout_write.puts("[[]]")
186+
stdout_write.puts("null")
187+
stdout_write.flush
188+
189+
# Then send the actual response
190+
stdout_write.puts(JSON.generate({
191+
jsonrpc: "2.0",
192+
id: "test-id",
193+
result: { tools: [] },
194+
}))
195+
stdout_write.flush
196+
end
197+
198+
transport.connect
199+
response = transport.send_request(request: request)
200+
201+
assert_equal("test-id", response["id"])
202+
assert_empty(response.dig("result", "tools"))
203+
ensure
204+
server_thread.join
205+
stdin_read.close
206+
stdin_write.close
207+
stdout_read.close
208+
stdout_write.close
209+
end
210+
149211
def test_send_request_raises_error_when_process_exits
150212
stdin_read, stdin_write = IO.pipe
151213
stdout_read, stdout_write = IO.pipe

test/mcp/server/transports/stdio_transport_test.rb

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,27 @@ class StdioTransportTest < ActiveSupport::TestCase
133133
end
134134
end
135135

136+
test "open handles a malformed JSON-RPC batch element and emits an error response" do
137+
input = StringIO.new("[[]]\n")
138+
output = StringIO.new
139+
original_stdin = $stdin
140+
original_stdout = $stdout
141+
142+
begin
143+
$stdin = input
144+
$stdout = output
145+
@transport.open
146+
147+
response = JSON.parse(output.string, symbolize_names: true)
148+
assert_nil(response[:id])
149+
assert_equal(-32600, response[:error][:code])
150+
assert_equal("Invalid Request", response[:error][:message])
151+
ensure
152+
$stdin = original_stdin
153+
$stdout = original_stdout
154+
end
155+
end
156+
136157
test "rejects duplicate initialize on the same stdio session with -32600" do
137158
first = {
138159
jsonrpc: "2.0",

0 commit comments

Comments
 (0)