diff --git a/lib/json_rpc_handler.rb b/lib/json_rpc_handler.rb index 6cbce1cb..055d2162 100644 --- a/lib/json_rpc_handler.rb +++ b/lib/json_rpc_handler.rb @@ -90,8 +90,17 @@ def process_request(request, id_validation_pattern:, &method_finder) error = if !valid_version?(request[:jsonrpc]) "JSON-RPC version must be 2.0" + elsif request.key?(:id) && id.nil? + # JSON-RPC 2.0 allows a null id; MCP rules it out: a request carries a string or an integer id, + # and a notification carries no id member at all. Refused rather than run as a notification, + # since a sender that wrote an id meant to be answered. + "Request ID must not be null" elsif !valid_id?(id, id_validation_pattern) - "Request ID must match validation pattern, or be an integer or null" + if id_validation_pattern + "Request ID must be a string matching the validation pattern, or an integer" + else + "Request ID must be a string or an integer" + end elsif !valid_method_name?(request[:method]) 'Method name must be a string and not start with "rpc."' end diff --git a/lib/mcp/server.rb b/lib/mcp/server.rb index afb3e845..8d0e3b3d 100644 --- a/lib/mcp/server.rb +++ b/lib/mcp/server.rb @@ -271,6 +271,9 @@ def initialize( Methods::COMPLETION_COMPLETE => ->(_) { DEFAULT_COMPLETION_RESULT }, Methods::LOGGING_SET_LEVEL => method(:configure_logging_level), } + # The protocol's own request methods, as opposed to the notifications above and to custom methods + # registered later: these need a request id to be run. + @protocol_request_methods = @handlers.keys.reject { |name| Methods.notification?(name) }.freeze @transport = transport end @@ -614,6 +617,12 @@ def handle_request(request, method, session: nil, related_request_id: nil, auth_ # the TypeScript and Python SDKs rather than emitting a spurious `result: null`. return if Methods.notification?(method) && !related_request_id.nil? + # The mirror case: a request method of the protocol sent without an id is framed as a notification, + # whose result the sender can never receive, so it is not run either; the TypeScript and Python servers + # drop such a message the same way, as a notification they have no handler for. A custom method keeps + # running without an id, since `define_custom_method` serves notifications too. + return if related_request_id.nil? && @protocol_request_methods.include?(method) + # `notifications/cancelled` is dispatched directly: it is a notification (no JSON-RPC id) # and intentionally bypasses the `@handlers` lookup, capability check, in-flight registry, # and rescue blocks below. diff --git a/lib/mcp/server/transports/streamable_http_transport.rb b/lib/mcp/server/transports/streamable_http_transport.rb index ef85ef2f..e33d379b 100644 --- a/lib/mcp/server/transports/streamable_http_transport.rb +++ b/lib/mcp/server/transports/streamable_http_transport.rb @@ -2037,8 +2037,10 @@ def json_rpc_error_response(status:, code:, message:, data: nil, id: nil) [status, { "content-type" => "application/json" }, [body.to_json]] end + # A notification carries no id member at all; a message with `"id": null` is a malformed request, + # which the server answers with Invalid Request instead of running it unanswered. def notification?(body) - !body[:id] && !!body[:method] + !body.key?(:id) && !!body[:method] end # Dispatches a client-originated notification (e.g. `notifications/cancelled`, @@ -2129,10 +2131,10 @@ def handle_initialization(request, body_string, body) response = server_session.handle_json(body_string, auth_info: auth_info) # `initialize_request?` matches on the method alone, so an `initialize` sent without - # an id (framed as a notification) reaches here. `Server#init` marks the session initialized, - # but JSON-RPC emits no response for an id-less request, so `response` is nil. - # Returning `[200, ..., [nil]]` would place nil in the Rack body (which the web server cannot serialize), - # and the session would be retained since it is marked initialized. Discard the orphaned session + # an id (framed as a notification) reaches here. `Server#init` does not run for it, as for + # any request method without an id, so `response` is nil and the session created above + # is never initialized. Returning `[200, ..., [nil]]` would place nil in the Rack body (which + # the web server cannot serialize), and the session would linger uninitialized. Discard it # and ack with 202, mirroring the nil-response handling in a regular request. if response.nil? cleanup_session(session_id) if session_id diff --git a/test/json_rpc_handler_test.rb b/test/json_rpc_handler_test.rb index 07db0ecf..2904001e 100644 --- a/test/json_rpc_handler_test.rb +++ b/test/json_rpc_handler_test.rb @@ -130,7 +130,7 @@ assert_rpc_error expected_error: { code: -32600, message: "Invalid Request", - data: "Request ID must match validation pattern, or be an integer or null", + data: "Request ID must be a string matching the validation pattern, or an integer", } end @@ -160,7 +160,7 @@ assert_rpc_error expected_error: { code: -32600, message: "Invalid Request", - data: "Request ID must match validation pattern, or be an integer or null", + data: "Request ID must be a string matching the validation pattern, or an integer", } end @@ -170,7 +170,7 @@ assert_rpc_error expected_error: { code: -32600, message: "Invalid Request", - data: "Request ID must match validation pattern, or be an integer or null", + data: "Request ID must be a string matching the validation pattern, or an integer", } end @@ -180,7 +180,7 @@ assert_rpc_error expected_error: { code: -32600, message: "Invalid Request", - data: "Request ID must match validation pattern, or be an integer or null", + data: "Request ID must be a string matching the validation pattern, or an integer", } end @@ -190,7 +190,7 @@ assert_rpc_error expected_error: { code: -32600, message: "Invalid Request", - data: "Request ID must match validation pattern, or be an integer or null", + data: "Request ID must be a string matching the validation pattern, or an integer", } end @@ -200,7 +200,33 @@ assert_rpc_error expected_error: { code: -32600, message: "Invalid Request", - data: "Request ID must match validation pattern, or be an integer or null", + data: "Request ID must be a string matching the validation pattern, or an integer", + } + end + + # MCP narrows JSON-RPC here: "Requests MUST include a string or integer ID" and "the ID MUST NOT be null". + it "returns an error and does not call the method when id is null" do + called = false + register("add") { called = true } + + handle jsonrpc: "2.0", id: nil, method: "add", params: { a: 1, b: 2 } + + assert_rpc_error expected_error: { + code: -32600, + message: "Invalid Request", + data: "Request ID must not be null", + } + assert_nil @response[:id] + refute called + end + + it "names the accepted id types without the pattern when none is given" do + @response = JsonRpcHandler.handle({ jsonrpc: "2.0", id: 3.14, method: "add" }, id_validation_pattern: nil) { |_name, _id| nil } + + assert_rpc_error expected_error: { + code: -32600, + message: "Invalid Request", + data: "Request ID must be a string or an integer", } end @@ -323,7 +349,7 @@ assert_rpc_error expected_error: { code: -32600, message: "Invalid Request", - data: "Request ID must match validation pattern, or be an integer or null", + data: "Request ID must be a string matching the validation pattern, or an integer", } assert_nil @response[:id] end @@ -735,7 +761,7 @@ assert_rpc_error expected_error: { code: -32600, message: "Invalid Request", - data: "Request ID must match validation pattern, or be an integer or null", + data: "Request ID must be a string matching the validation pattern, or an integer", } end @@ -748,7 +774,7 @@ assert_rpc_error expected_error: { code: -32600, message: "Invalid Request", - data: "Request ID must match validation pattern, or be an integer or null", + data: "Request ID must be a string matching the validation pattern, or an integer", } end @@ -776,7 +802,7 @@ assert_rpc_error expected_error: { code: -32600, message: "Invalid Request", - data: "Request ID must match validation pattern, or be an integer or null", + data: "Request ID must be a string matching the validation pattern, or an integer", } end diff --git a/test/mcp/server/transports/streamable_http_transport_test.rb b/test/mcp/server/transports/streamable_http_transport_test.rb index 1e1ebc7a..9c0f46d3 100644 --- a/test/mcp/server/transports/streamable_http_transport_test.rb +++ b/test/mcp/server/transports/streamable_http_transport_test.rb @@ -157,6 +157,82 @@ def string assert_equal JsonRpcHandler::ErrorCode::METHOD_NOT_FOUND, body["error"]["code"] end + test "a request whose id is null is answered with Invalid Request instead of being run unanswered" do + calls = 0 + @server.define_tool(name: "count") do + calls += 1 + Tool::Response.new([{ type: "text", text: "ok" }]) + end + session_id = initialize_test_session + + request = create_rack_request( + "POST", + "/", + { "CONTENT_TYPE" => "application/json", "HTTP_MCP_SESSION_ID" => session_id }, + { jsonrpc: "2.0", id: nil, method: "tools/call", params: { name: "count", arguments: {} } }.to_json, + ) + response = @transport.handle_request(request) + + assert_equal 200, response[0] + io = StringIO.new + response[2].call(io) + body = JSON.parse(io.string.match(/^data: (.+)$/)[1]) + assert_nil body["id"] + assert_equal(-32600, body["error"]["code"]) + assert_equal 0, calls + end + + test "modern POST answers a request whose id is null with 400 and -32600" do + calls = 0 + @server.define_tool(name: "count") do + calls += 1 + Tool::Response.new([{ type: "text", text: "ok" }]) + end + body = JSON.parse(modern_body("tools/call", { name: "count", arguments: {} })) + body["id"] = nil + + response = @transport.handle_request(modern_rack_request(body.to_json, headers: { "HTTP_MCP_NAME" => "count" })) + + assert_equal 400, response[0] + assert_equal(-32600, JSON.parse(response[2][0]).dig("error", "code")) + assert_equal 0, calls + end + + test "a ping sent without an id is acknowledged with no response" do + session_id = initialize_test_session + + request = create_rack_request( + "POST", + "/", + { "CONTENT_TYPE" => "application/json", "HTTP_MCP_SESSION_ID" => session_id }, + { jsonrpc: "2.0", method: "ping" }.to_json, + ) + response = @transport.handle_request(request) + + assert_equal 202, response[0] + assert_equal [], response[2] + end + + test "a tools/call sent without an id is acknowledged without being run" do + calls = 0 + @server.define_tool(name: "count") do + calls += 1 + Tool::Response.new([{ type: "text", text: "ok" }]) + end + session_id = initialize_test_session + + request = create_rack_request( + "POST", + "/", + { "CONTENT_TYPE" => "application/json", "HTTP_MCP_SESSION_ID" => session_id }, + { jsonrpc: "2.0", method: "tools/call", params: { name: "count", arguments: {} } }.to_json, + ) + response = @transport.handle_request(request) + + assert_equal 202, response[0] + assert_equal 0, calls + end + test "handles POST request with invalid JSON" do request = create_rack_request( "POST", diff --git a/test/mcp/server_test.rb b/test/mcp/server_test.rb index c1ac383f..e78a9d58 100644 --- a/test/mcp/server_test.rb +++ b/test/mcp/server_test.rb @@ -1915,6 +1915,42 @@ def read_resource_request(uri) assert_instrumentation_data({ method: "unsupported_method" }) end + test "#handle refuses a request whose id is null without running it" do + calls = 0 + @server.define_tool(name: "count") do + calls += 1 + Tool::Response.new([{ type: "text", text: "ok" }]) + end + + response = @server.handle({ jsonrpc: "2.0", id: nil, method: "tools/call", params: { name: "count", arguments: {} } }) + + assert_equal({ jsonrpc: "2.0", id: nil, error: { code: -32600, message: "Invalid Request", data: "Request ID must not be null" } }, response) + assert_equal 0, calls + end + + test "#handle does not run a protocol request method sent without an id" do + # Framed as a notification, the message could never be answered; the TypeScript and Python servers drop it too. + calls = 0 + @server.define_tool(name: "count") do + calls += 1 + Tool::Response.new([{ type: "text", text: "ok" }]) + end + + assert_nil @server.handle({ jsonrpc: "2.0", method: "tools/call", params: { name: "count", arguments: {} } }) + assert_nil @server.handle({ jsonrpc: "2.0", method: "tools/list" }) + assert_equal 0, calls + end + + test "#handle still runs a custom method sent without an id" do + # `define_custom_method` serves notifications as well as requests, so only the protocol's own request methods need an id. + seen = [] + @server.define_custom_method(method_name: "notify") { |params| seen << params[:event] } + + assert_nil @server.handle({ jsonrpc: "2.0", method: "notify", params: { event: "custom" } }) + + assert_equal ["custom"], seen + end + test "#handle handles custom methods" do @server.define_custom_method(method_name: "add") do |params| params[:a] + params[:b]