From 0270b24f56bcacae0ad925b36125efd65351b8ea Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Fri, 16 Oct 2026 02:13:34 +0900 Subject: [PATCH] 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. --- lib/json_rpc_handler.rb | 2 +- .../transports/streamable_http_transport.rb | 20 ++-- test/json_rpc_handler_test.rb | 15 +++ .../streamable_http_transport_test.rb | 100 ++++++++++++++++++ 4 files changed, 130 insertions(+), 7 deletions(-) diff --git a/lib/json_rpc_handler.rb b/lib/json_rpc_handler.rb index 3ce87a96..b62523ee 100644 --- a/lib/json_rpc_handler.rb +++ b/lib/json_rpc_handler.rb @@ -227,7 +227,7 @@ def valid_id?(id, pattern = nil) end def valid_method_name?(method) - method.is_a?(String) && !method.start_with?("rpc.") + method.is_a?(String) && method.valid_encoding? && !method.start_with?("rpc.") end def valid_params?(params) diff --git a/lib/mcp/server/transports/streamable_http_transport.rb b/lib/mcp/server/transports/streamable_http_transport.rb index 29b3518e..42c92df6 100644 --- a/lib/mcp/server/transports/streamable_http_transport.rb +++ b/lib/mcp/server/transports/streamable_http_transport.rb @@ -907,7 +907,7 @@ def handle_modern(request, header_version, body_string: nil) supported: MCP::Configuration::SUPPORTED_MODERN_PROTOCOL_VERSIONS, requested: header_version, }, - id: body[:id], + id: echoable_id(body[:id]), ) end @@ -916,7 +916,7 @@ def handle_modern(request, header_version, body_string: nil) status: 400, code: JsonRpcHandler::ErrorCode::INVALID_REQUEST, message: "Bad Request: Mcp-Session-Id is not accepted in the modern lifecycle", - id: body[:id], + id: echoable_id(body[:id]), ) end @@ -1135,6 +1135,14 @@ def json_encodable?(value) false end + # The id a refusal built here can echo: an Integer, or a String that is valid UTF-8. A String holding other + # bytes would make the body's `to_json` raise, so the refusal carries no id, as for an id it could not read. + def echoable_id(id) + return id if id.is_a?(Integer) || (id.is_a?(String) && id.valid_encoding?) + + nil + end + def listen_subscriptions_full? return false unless @max_listen_subscriptions @@ -1576,7 +1584,7 @@ def header_mismatch_response(message, id) status: 400, code: ErrorCodes::HEADER_MISMATCH, message: "Header mismatch: #{message}", - id: id, + id: echoable_id(id), ) end @@ -1653,7 +1661,7 @@ def handle_post(request, body_string: nil) if header_version && MCP::Configuration.modern_protocol_version?(header_version) return invalid_request_response( "Invalid Request: the session already negotiated the legacy lifecycle via `initialize`", - request_id: body[:id], + request_id: echoable_id(body[:id]), ) end @@ -1699,7 +1707,7 @@ def handle_post(request, body_string: nil) session = @mutex.synchronize { @sessions[session_id] } return session_not_found_response unless session && session_principal_matches?(session, request) - return already_initialized_response(body[:id]) + return already_initialized_response(echoable_id(body[:id])) end return session_not_found_response @@ -1966,7 +1974,7 @@ def initialize_too_large_response(request_id, reason) status: 413, code: JsonRpcHandler::ErrorCode::INVALID_REQUEST, message: "Payload too large: initialize request #{reason}", - id: request_id, + id: echoable_id(request_id), ) end diff --git a/test/json_rpc_handler_test.rb b/test/json_rpc_handler_test.rb index 994bb72c..0ab6b72d 100644 --- a/test/json_rpc_handler_test.rb +++ b/test/json_rpc_handler_test.rb @@ -73,6 +73,21 @@ assert_equal 1, @response[:id] end + it "returns an error and does not call the method when the method name is not valid UTF-8" do + # Such a name reached dispatch: unknown, its Method not found error could not be written as JSON, and registered, it ran. + called = false + register("no\xFFpe".dup.force_encoding(Encoding::UTF_8)) { called = true } + + handle_json "{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"no\xFFpe\"}".b + + assert_rpc_error expected_error: { + code: -32600, + message: "Invalid Request", + data: 'Method name must be a string and not start with "rpc."', + } + refute called + end + it "returns an error when method begins with 'rpc.'" do handle jsonrpc: "2.0", id: 1, method: "rpc.add", params: { a: 1, b: 2 } diff --git a/test/mcp/server/transports/streamable_http_transport_test.rb b/test/mcp/server/transports/streamable_http_transport_test.rb index df1d173c..8daecd9b 100644 --- a/test/mcp/server/transports/streamable_http_transport_test.rb +++ b/test/mcp/server/transports/streamable_http_transport_test.rb @@ -398,6 +398,63 @@ def string assert_equal 200, ping_response[0] end + test "a duplicate initialize whose id is not valid UTF-8 is refused with no id" do + reported = [] + original_reporter = MCP.configuration.exception_reporter + MCP.configuration.exception_reporter = ->(error, _context) { reported << error } + init_request = create_rack_request( + "POST", + "/", + { "CONTENT_TYPE" => "application/json" }, + { jsonrpc: "2.0", method: "initialize", id: "first", params: initialize_params }.to_json, + ) + session_id = @transport.handle_request(init_request)[1]["mcp-session-id"] + duplicate = { jsonrpc: "2.0", method: "initialize", id: "second", params: initialize_params }.to_json + .b.sub('"second"', "\"a\xFF\"".b) + + response = @transport.handle_request(create_rack_request( + "POST", + "/", + { "CONTENT_TYPE" => "application/json", "HTTP_MCP_SESSION_ID" => session_id }, + duplicate, + )) + + assert_equal 400, response[0] + parsed = JSON.parse(response[2][0]) + refute parsed.key?("id") + assert_equal JsonRpcHandler::ErrorCode::INVALID_REQUEST, parsed["error"]["code"] + assert_empty reported + ensure + MCP.configuration.exception_reporter = original_reporter + end + + test "a request whose method name is not valid UTF-8 is refused with -32600 on the legacy path" do + # Dispatched as sent, the name reached the Method not found error, whose `data` could not be written + # as JSON, so the request was answered `-32603` Internal error instead of refused. + init_request = create_rack_request( + "POST", + "/", + { "CONTENT_TYPE" => "application/json" }, + { jsonrpc: "2.0", method: "initialize", id: "first", params: initialize_params }.to_json, + ) + session_id = @transport.handle_request(init_request)[1]["mcp-session-id"] + body = { jsonrpc: "2.0", method: "PLACEHOLDER", id: "m1" }.to_json.b.sub("PLACEHOLDER", "no\xFFpe".b) + + response = @transport.handle_request(create_rack_request( + "POST", + "/", + { "CONTENT_TYPE" => "application/json", "HTTP_MCP_SESSION_ID" => session_id }, + body, + )) + + assert_equal 200, response[0] + io = StringIO.new + response[2].call(io) + parsed = JSON.parse(io.string.match(/^data: (.+)$/)[1]) + assert_equal "m1", parsed["id"] + assert_equal JsonRpcHandler::ErrorCode::INVALID_REQUEST, parsed["error"]["code"] + end + test "rejects initialize with stale Mcp-Session-Id with 404" do request = create_rack_request( "POST", @@ -6300,6 +6357,49 @@ def string assert_equal(-32020, JSON.parse(response[2][0]).dig("error", "code")) end + test "modern POST refusals carry no id when the request id is not valid UTF-8" do + # The refusal is built before the id is validated; one whose bytes could not be encoded made `to_json` + # raise and the rescue answer 500 with an exception report. + reported = [] + original_reporter = MCP.configuration.exception_reporter + MCP.configuration.exception_reporter = ->(error, _context) { reported << error } + unsupported = modern_body("ping", {}, version: "2027-01-01").b.sub('"id":1', "\"id\":\"a\xFF\"".b) + mismatched = modern_body("ping", {}).b.sub('"id":1', "\"id\":\"a\xFF\"".b) + + by_version = @transport.handle_request(modern_rack_request(unsupported, version: "2027-01-01")) + by_header = @transport.handle_request(modern_rack_request(mismatched, headers: { "HTTP_MCP_METHOD" => "tools/call" })) + + [[by_version, -32022], [by_header, -32020]].each do |response, code| + assert_equal 400, response[0] + parsed = JSON.parse(response[2][0]) + assert_equal code, parsed.dig("error", "code") + refute parsed.key?("id") + end + assert_empty reported + ensure + MCP.configuration.exception_reporter = original_reporter + end + + test "modern POST refusals keep an integer or string id and drop any other" do + kept = ['""', "123456789012345678901234567890"] + dropped = ["true", "1.5", "{}"] + + (kept + dropped).each do |id| + body = modern_body("ping", {}, version: "2027-01-01").sub('"id":1', "\"id\":#{id}") + + response = @transport.handle_request(modern_rack_request(body, version: "2027-01-01")) + + assert_equal 400, response[0], id + parsed = JSON.parse(response[2][0]) + assert_equal(-32022, parsed.dig("error", "code"), id) + if kept.include?(id) + assert_equal JSON.parse(id), parsed["id"], id + else + refute parsed.key?("id"), id + end + end + end + test "modern POST without the Mcp-Method header returns 400 with -32020" do # The header is required on every modern POST; a missing required standard header is # a validation failure, matching the TypeScript SDK's enforcement.