diff --git a/lib/json_rpc_handler.rb b/lib/json_rpc_handler.rb index 6cbce1cb..b4a888db 100644 --- a/lib/json_rpc_handler.rb +++ b/lib/json_rpc_handler.rb @@ -88,10 +88,12 @@ def process_request(request, id_validation_pattern:, &method_finder) id = request[:id] + # Unlike base JSON-RPC, MCP does not allow a null request id, + # so only a message without an `id` member is a notification. error = if !valid_version?(request[:jsonrpc]) "JSON-RPC version must be 2.0" - elsif !valid_id?(id, id_validation_pattern) - "Request ID must match validation pattern, or be an integer or null" + elsif request.key?(:id) && !valid_id?(id, id_validation_pattern) + "Request ID must match validation pattern, or be an integer" elsif !valid_method_name?(request[:method]) 'Method name must be a string and not start with "rpc."' end @@ -175,7 +177,7 @@ def valid_version?(version) end def valid_id?(id, pattern = nil) - return true if id.nil? || id.is_a?(Integer) + return true if id.is_a?(Integer) return false unless id.is_a?(String) pattern ? id.match?(pattern) : true diff --git a/lib/mcp/server/transports/streamable_http_transport.rb b/lib/mcp/server/transports/streamable_http_transport.rb index ef85ef2f..f715dd76 100644 --- a/lib/mcp/server/transports/streamable_http_transport.rb +++ b/lib/mcp/server/transports/streamable_http_transport.rb @@ -2037,8 +2037,9 @@ def json_rpc_error_response(status:, code:, message:, data: nil, id: nil) [status, { "content-type" => "application/json" }, [body.to_json]] end + # A null id makes the message a request, not a notification, so its Invalid Request error reaches the client. def notification?(body) - !body[:id] && !!body[:method] + !body.key?(:id) && !!body[:method] end # Dispatches a client-originated notification (e.g. `notifications/cancelled`, diff --git a/test/json_rpc_handler_test.rb b/test/json_rpc_handler_test.rb index 07db0ecf..c3b9990c 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 match validation pattern, or be 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 match validation pattern, or be 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 match validation pattern, or be 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 match validation pattern, or be 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 match validation pattern, or be an integer", } end @@ -200,10 +200,28 @@ 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 match validation pattern, or be an integer", } end + # MCP is stricter here (Basic Protocol, Requests): "Requests MUST include a string or integer ID. Unlike base + # JSON-RPC, the ID MUST NOT be null." + + it "returns an error without calling the method when the request 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 match validation pattern, or be an integer", + } + assert_nil @response[:id] + refute called + end + # 4.1 Notification # # A Notification is a Request object without an "id" member. A Request object that is a Notification signifies the @@ -215,6 +233,15 @@ # the Client would not be aware of any errors (like e.g. "Invalid params","Internal error"). describe "with a notification request" do + it "calls the method" do + called = false + register("ping") { called = true } + + handle jsonrpc: "2.0", method: "ping" + + assert called + end + it "returns nil even if the method returns a result" do register("ping") { "pong" } @@ -323,7 +350,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 match validation pattern, or be an integer", } assert_nil @response[:id] end @@ -735,7 +762,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 match validation pattern, or be an integer", } end @@ -748,7 +775,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 match validation pattern, or be an integer", } end @@ -776,7 +803,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 match validation pattern, or be 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..37c9cba0 100644 --- a/test/mcp/server/transports/streamable_http_transport_test.rb +++ b/test/mcp/server/transports/streamable_http_transport_test.rb @@ -2521,6 +2521,30 @@ def string assert_nil(body) end + test "stateless mode answers a request with a null id with -32600 without running it" do + calls = 0 + @server.define_tool(name: "counting_tool") do + calls += 1 + Tool::Response.new([{ type: "text", text: "ok" }]) + end + stateless_transport = StreamableHTTPTransport.new(@server, stateless: true) + + request = create_rack_request( + "POST", + "/", + { "CONTENT_TYPE" => "application/json" }, + { jsonrpc: "2.0", method: "tools/call", id: nil, params: { name: "counting_tool", arguments: {} } }.to_json, + ) + + response = stateless_transport.handle_request(request) + assert_equal 200, response[0] + + body = JSON.parse(response[2][0]) + assert_equal(-32600, body.dig("error", "code")) + assert_nil(body["id"]) + assert_equal 0, calls + end + test "POST request returns SSE response even with GET SSE connected" do init_request = create_rack_request( "POST", @@ -6047,6 +6071,23 @@ def string assert_equal(-32601, JSON.parse(response[2][0]).dig("error", "code")) end + test "modern POST answers a request with a null id with 400 and -32600 without running it" do + calls = 0 + @server.define_tool(name: "counting_tool") do + calls += 1 + Tool::Response.new([{ type: "text", text: "ok" }]) + end + body = JSON.parse(modern_body("tools/call", { name: "counting_tool", arguments: {} })).merge("id" => nil) + + response = @transport.handle_request(modern_rack_request(body.to_json)) + + assert_equal 400, response[0] + parsed = JSON.parse(response[2][0]) + assert_equal(-32600, parsed.dig("error", "code")) + assert_nil(parsed["id"]) + assert_equal 0, calls + end + test "modern POST requires the _meta envelope" do response = @transport.handle_request(modern_rack_request( { jsonrpc: "2.0", method: "tools/list", id: 1 }.to_json,