Skip to content

Commit 0270b24

Browse files
committed
Refuse ids and method names that cannot be encoded
## Motivation and Context A request id or method name holding bytes that are not valid UTF-8 comes out of the JSON parser as sent and reaches a response. The Streamable HTTP transport echoes the id in the refusals it builds itself: the `-32020` header mismatch, the `-32022` unsupported version, the session id sent in the modern lifecycle, the duplicate `initialize`, the legacy envelope and the over-size `initialize` errors. `JsonRpcHandler` echoes an unknown method name in the `data` of Method not found. With such bytes `to_json` raises: the transport's rescue answers 500 with the error reported instead of the intended 400 or 413, and on the legacy path a Method not found for such a name cannot be generated and is answered `-32603` Internal error. Any client can send either. Those refusals now carry the id only when it is an Integer or a String that is valid UTF-8, and otherwise what they carry for an id that could not be read; `JsonRpcHandler` refuses a method name that is not valid UTF-8 with `-32600` Invalid Request before dispatch, as it refuses one that is not a string. ## How Has This Been Tested? `test/json_rpc_handler_test.rb` (such a method name answers `-32600` without calling the handler registered under it) and four tests in `test/mcp/server/transports/streamable_http_transport_test.rb` (such an id on the modern `-32022` and `-32020` refusals and on a legacy duplicate `initialize` answers 400 with no readable id and no exception report; a refusal keeps an empty or a large integer id and drops a boolean, a fraction or an object; such a method name on a legacy session answers `-32600` in the SSE body). Against the previous library the invalid-byte id tests get a 500, the id-type test sees the boolean echoed, the handler test sees its handler run, and the legacy method-name test gets `-32603`. ## Breaking Changes The transport refusals named above answer a request whose id is not an Integer or a valid UTF-8 String as for an id they could not read, where they echoed an id of another type as sent; the `subscriptions/listen` refusals are unchanged.
1 parent d518025 commit 0270b24

4 files changed

Lines changed: 130 additions & 7 deletions

File tree

‎lib/json_rpc_handler.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ def valid_id?(id, pattern = nil)
227227
end
228228

229229
def valid_method_name?(method)
230-
method.is_a?(String) && !method.start_with?("rpc.")
230+
method.is_a?(String) && method.valid_encoding? && !method.start_with?("rpc.")
231231
end
232232

233233
def valid_params?(params)

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

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -907,7 +907,7 @@ def handle_modern(request, header_version, body_string: nil)
907907
supported: MCP::Configuration::SUPPORTED_MODERN_PROTOCOL_VERSIONS,
908908
requested: header_version,
909909
},
910-
id: body[:id],
910+
id: echoable_id(body[:id]),
911911
)
912912
end
913913

@@ -916,7 +916,7 @@ def handle_modern(request, header_version, body_string: nil)
916916
status: 400,
917917
code: JsonRpcHandler::ErrorCode::INVALID_REQUEST,
918918
message: "Bad Request: Mcp-Session-Id is not accepted in the modern lifecycle",
919-
id: body[:id],
919+
id: echoable_id(body[:id]),
920920
)
921921
end
922922

@@ -1135,6 +1135,14 @@ def json_encodable?(value)
11351135
false
11361136
end
11371137

1138+
# The id a refusal built here can echo: an Integer, or a String that is valid UTF-8. A String holding other
1139+
# bytes would make the body's `to_json` raise, so the refusal carries no id, as for an id it could not read.
1140+
def echoable_id(id)
1141+
return id if id.is_a?(Integer) || (id.is_a?(String) && id.valid_encoding?)
1142+
1143+
nil
1144+
end
1145+
11381146
def listen_subscriptions_full?
11391147
return false unless @max_listen_subscriptions
11401148

@@ -1576,7 +1584,7 @@ def header_mismatch_response(message, id)
15761584
status: 400,
15771585
code: ErrorCodes::HEADER_MISMATCH,
15781586
message: "Header mismatch: #{message}",
1579-
id: id,
1587+
id: echoable_id(id),
15801588
)
15811589
end
15821590

@@ -1653,7 +1661,7 @@ def handle_post(request, body_string: nil)
16531661
if header_version && MCP::Configuration.modern_protocol_version?(header_version)
16541662
return invalid_request_response(
16551663
"Invalid Request: the session already negotiated the legacy lifecycle via `initialize`",
1656-
request_id: body[:id],
1664+
request_id: echoable_id(body[:id]),
16571665
)
16581666
end
16591667

@@ -1699,7 +1707,7 @@ def handle_post(request, body_string: nil)
16991707
session = @mutex.synchronize { @sessions[session_id] }
17001708
return session_not_found_response unless session && session_principal_matches?(session, request)
17011709

1702-
return already_initialized_response(body[:id])
1710+
return already_initialized_response(echoable_id(body[:id]))
17031711
end
17041712

17051713
return session_not_found_response
@@ -1966,7 +1974,7 @@ def initialize_too_large_response(request_id, reason)
19661974
status: 413,
19671975
code: JsonRpcHandler::ErrorCode::INVALID_REQUEST,
19681976
message: "Payload too large: initialize request #{reason}",
1969-
id: request_id,
1977+
id: echoable_id(request_id),
19701978
)
19711979
end
19721980

‎test/json_rpc_handler_test.rb‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,21 @@
7373
assert_equal 1, @response[:id]
7474
end
7575

76+
it "returns an error and does not call the method when the method name is not valid UTF-8" do
77+
# Such a name reached dispatch: unknown, its Method not found error could not be written as JSON, and registered, it ran.
78+
called = false
79+
register("no\xFFpe".dup.force_encoding(Encoding::UTF_8)) { called = true }
80+
81+
handle_json "{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"no\xFFpe\"}".b
82+
83+
assert_rpc_error expected_error: {
84+
code: -32600,
85+
message: "Invalid Request",
86+
data: 'Method name must be a string and not start with "rpc."',
87+
}
88+
refute called
89+
end
90+
7691
it "returns an error when method begins with 'rpc.'" do
7792
handle jsonrpc: "2.0", id: 1, method: "rpc.add", params: { a: 1, b: 2 }
7893

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

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,63 @@ def string
398398
assert_equal 200, ping_response[0]
399399
end
400400

401+
test "a duplicate initialize whose id is not valid UTF-8 is refused with no id" do
402+
reported = []
403+
original_reporter = MCP.configuration.exception_reporter
404+
MCP.configuration.exception_reporter = ->(error, _context) { reported << error }
405+
init_request = create_rack_request(
406+
"POST",
407+
"/",
408+
{ "CONTENT_TYPE" => "application/json" },
409+
{ jsonrpc: "2.0", method: "initialize", id: "first", params: initialize_params }.to_json,
410+
)
411+
session_id = @transport.handle_request(init_request)[1]["mcp-session-id"]
412+
duplicate = { jsonrpc: "2.0", method: "initialize", id: "second", params: initialize_params }.to_json
413+
.b.sub('"second"', "\"a\xFF\"".b)
414+
415+
response = @transport.handle_request(create_rack_request(
416+
"POST",
417+
"/",
418+
{ "CONTENT_TYPE" => "application/json", "HTTP_MCP_SESSION_ID" => session_id },
419+
duplicate,
420+
))
421+
422+
assert_equal 400, response[0]
423+
parsed = JSON.parse(response[2][0])
424+
refute parsed.key?("id")
425+
assert_equal JsonRpcHandler::ErrorCode::INVALID_REQUEST, parsed["error"]["code"]
426+
assert_empty reported
427+
ensure
428+
MCP.configuration.exception_reporter = original_reporter
429+
end
430+
431+
test "a request whose method name is not valid UTF-8 is refused with -32600 on the legacy path" do
432+
# Dispatched as sent, the name reached the Method not found error, whose `data` could not be written
433+
# as JSON, so the request was answered `-32603` Internal error instead of refused.
434+
init_request = create_rack_request(
435+
"POST",
436+
"/",
437+
{ "CONTENT_TYPE" => "application/json" },
438+
{ jsonrpc: "2.0", method: "initialize", id: "first", params: initialize_params }.to_json,
439+
)
440+
session_id = @transport.handle_request(init_request)[1]["mcp-session-id"]
441+
body = { jsonrpc: "2.0", method: "PLACEHOLDER", id: "m1" }.to_json.b.sub("PLACEHOLDER", "no\xFFpe".b)
442+
443+
response = @transport.handle_request(create_rack_request(
444+
"POST",
445+
"/",
446+
{ "CONTENT_TYPE" => "application/json", "HTTP_MCP_SESSION_ID" => session_id },
447+
body,
448+
))
449+
450+
assert_equal 200, response[0]
451+
io = StringIO.new
452+
response[2].call(io)
453+
parsed = JSON.parse(io.string.match(/^data: (.+)$/)[1])
454+
assert_equal "m1", parsed["id"]
455+
assert_equal JsonRpcHandler::ErrorCode::INVALID_REQUEST, parsed["error"]["code"]
456+
end
457+
401458
test "rejects initialize with stale Mcp-Session-Id with 404" do
402459
request = create_rack_request(
403460
"POST",
@@ -6300,6 +6357,49 @@ def string
63006357
assert_equal(-32020, JSON.parse(response[2][0]).dig("error", "code"))
63016358
end
63026359

6360+
test "modern POST refusals carry no id when the request id is not valid UTF-8" do
6361+
# The refusal is built before the id is validated; one whose bytes could not be encoded made `to_json`
6362+
# raise and the rescue answer 500 with an exception report.
6363+
reported = []
6364+
original_reporter = MCP.configuration.exception_reporter
6365+
MCP.configuration.exception_reporter = ->(error, _context) { reported << error }
6366+
unsupported = modern_body("ping", {}, version: "2027-01-01").b.sub('"id":1', "\"id\":\"a\xFF\"".b)
6367+
mismatched = modern_body("ping", {}).b.sub('"id":1', "\"id\":\"a\xFF\"".b)
6368+
6369+
by_version = @transport.handle_request(modern_rack_request(unsupported, version: "2027-01-01"))
6370+
by_header = @transport.handle_request(modern_rack_request(mismatched, headers: { "HTTP_MCP_METHOD" => "tools/call" }))
6371+
6372+
[[by_version, -32022], [by_header, -32020]].each do |response, code|
6373+
assert_equal 400, response[0]
6374+
parsed = JSON.parse(response[2][0])
6375+
assert_equal code, parsed.dig("error", "code")
6376+
refute parsed.key?("id")
6377+
end
6378+
assert_empty reported
6379+
ensure
6380+
MCP.configuration.exception_reporter = original_reporter
6381+
end
6382+
6383+
test "modern POST refusals keep an integer or string id and drop any other" do
6384+
kept = ['""', "123456789012345678901234567890"]
6385+
dropped = ["true", "1.5", "{}"]
6386+
6387+
(kept + dropped).each do |id|
6388+
body = modern_body("ping", {}, version: "2027-01-01").sub('"id":1', "\"id\":#{id}")
6389+
6390+
response = @transport.handle_request(modern_rack_request(body, version: "2027-01-01"))
6391+
6392+
assert_equal 400, response[0], id
6393+
parsed = JSON.parse(response[2][0])
6394+
assert_equal(-32022, parsed.dig("error", "code"), id)
6395+
if kept.include?(id)
6396+
assert_equal JSON.parse(id), parsed["id"], id
6397+
else
6398+
refute parsed.key?("id"), id
6399+
end
6400+
end
6401+
end
6402+
63036403
test "modern POST without the Mcp-Method header returns 400 with -32020" do
63046404
# The header is required on every modern POST; a missing required standard header is
63056405
# a validation failure, matching the TypeScript SDK's enforcement.

0 commit comments

Comments
 (0)