Skip to content

Commit 54885b2

Browse files
authored
Merge pull request #612 from koic/reject_requests_with_a_null_id
Refuse a request whose id is null and do not run a request method without an id
2 parents 450f73a + e20b7e4 commit 54885b2

6 files changed

Lines changed: 174 additions & 16 deletions

File tree

‎lib/json_rpc_handler.rb‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,17 @@ def process_request(request, id_validation_pattern:, &method_finder)
9090

9191
error = if !valid_version?(request[:jsonrpc])
9292
"JSON-RPC version must be 2.0"
93+
elsif request.key?(:id) && id.nil?
94+
# JSON-RPC 2.0 allows a null id; MCP rules it out: a request carries a string or an integer id,
95+
# and a notification carries no id member at all. Refused rather than run as a notification,
96+
# since a sender that wrote an id meant to be answered.
97+
"Request ID must not be null"
9398
elsif !valid_id?(id, id_validation_pattern)
94-
"Request ID must match validation pattern, or be an integer or null"
99+
if id_validation_pattern
100+
"Request ID must be a string matching the validation pattern, or an integer"
101+
else
102+
"Request ID must be a string or an integer"
103+
end
95104
elsif !valid_method_name?(request[:method])
96105
'Method name must be a string and not start with "rpc."'
97106
end

‎lib/mcp/server.rb‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,9 @@ def initialize(
271271
Methods::COMPLETION_COMPLETE => ->(_) { DEFAULT_COMPLETION_RESULT },
272272
Methods::LOGGING_SET_LEVEL => method(:configure_logging_level),
273273
}
274+
# The protocol's own request methods, as opposed to the notifications above and to custom methods
275+
# registered later: these need a request id to be run.
276+
@protocol_request_methods = @handlers.keys.reject { |name| Methods.notification?(name) }.freeze
274277
@transport = transport
275278
end
276279

@@ -614,6 +617,12 @@ def handle_request(request, method, session: nil, related_request_id: nil, auth_
614617
# the TypeScript and Python SDKs rather than emitting a spurious `result: null`.
615618
return if Methods.notification?(method) && !related_request_id.nil?
616619

620+
# The mirror case: a request method of the protocol sent without an id is framed as a notification,
621+
# whose result the sender can never receive, so it is not run either; the TypeScript and Python servers
622+
# drop such a message the same way, as a notification they have no handler for. A custom method keeps
623+
# running without an id, since `define_custom_method` serves notifications too.
624+
return if related_request_id.nil? && @protocol_request_methods.include?(method)
625+
617626
# `notifications/cancelled` is dispatched directly: it is a notification (no JSON-RPC id)
618627
# and intentionally bypasses the `@handlers` lookup, capability check, in-flight registry,
619628
# and rescue blocks below.

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2035,8 +2035,10 @@ def json_rpc_error_response(status:, code:, message:, data: nil, id: nil)
20352035
[status, { "content-type" => "application/json" }, [body.to_json]]
20362036
end
20372037

2038+
# A notification carries no id member at all; a message with `"id": null` is a malformed request,
2039+
# which the server answers with Invalid Request instead of running it unanswered.
20382040
def notification?(body)
2039-
!body[:id] && !!body[:method]
2041+
!body.key?(:id) && !!body[:method]
20402042
end
20412043

20422044
# Dispatches a client-originated notification (e.g. `notifications/cancelled`,
@@ -2127,10 +2129,10 @@ def handle_initialization(request, body_string, body)
21272129
response = server_session.handle_json(body_string, auth_info: auth_info)
21282130

21292131
# `initialize_request?` matches on the method alone, so an `initialize` sent without
2130-
# an id (framed as a notification) reaches here. `Server#init` marks the session initialized,
2131-
# but JSON-RPC emits no response for an id-less request, so `response` is nil.
2132-
# Returning `[200, ..., [nil]]` would place nil in the Rack body (which the web server cannot serialize),
2133-
# and the session would be retained since it is marked initialized. Discard the orphaned session
2132+
# an id (framed as a notification) reaches here. `Server#init` does not run for it, as for
2133+
# any request method without an id, so `response` is nil and the session created above
2134+
# is never initialized. Returning `[200, ..., [nil]]` would place nil in the Rack body (which
2135+
# the web server cannot serialize), and the session would linger uninitialized. Discard it
21342136
# and ack with 202, mirroring the nil-response handling in a regular request.
21352137
if response.nil?
21362138
cleanup_session(session_id) if session_id

‎test/json_rpc_handler_test.rb‎

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@
130130
assert_rpc_error expected_error: {
131131
code: -32600,
132132
message: "Invalid Request",
133-
data: "Request ID must match validation pattern, or be an integer or null",
133+
data: "Request ID must be a string matching the validation pattern, or an integer",
134134
}
135135
end
136136

@@ -160,7 +160,7 @@
160160
assert_rpc_error expected_error: {
161161
code: -32600,
162162
message: "Invalid Request",
163-
data: "Request ID must match validation pattern, or be an integer or null",
163+
data: "Request ID must be a string matching the validation pattern, or an integer",
164164
}
165165
end
166166

@@ -170,7 +170,7 @@
170170
assert_rpc_error expected_error: {
171171
code: -32600,
172172
message: "Invalid Request",
173-
data: "Request ID must match validation pattern, or be an integer or null",
173+
data: "Request ID must be a string matching the validation pattern, or an integer",
174174
}
175175
end
176176

@@ -180,7 +180,7 @@
180180
assert_rpc_error expected_error: {
181181
code: -32600,
182182
message: "Invalid Request",
183-
data: "Request ID must match validation pattern, or be an integer or null",
183+
data: "Request ID must be a string matching the validation pattern, or an integer",
184184
}
185185
end
186186

@@ -190,7 +190,7 @@
190190
assert_rpc_error expected_error: {
191191
code: -32600,
192192
message: "Invalid Request",
193-
data: "Request ID must match validation pattern, or be an integer or null",
193+
data: "Request ID must be a string matching the validation pattern, or an integer",
194194
}
195195
end
196196

@@ -200,7 +200,33 @@
200200
assert_rpc_error expected_error: {
201201
code: -32600,
202202
message: "Invalid Request",
203-
data: "Request ID must match validation pattern, or be an integer or null",
203+
data: "Request ID must be a string matching the validation pattern, or an integer",
204+
}
205+
end
206+
207+
# MCP narrows JSON-RPC here: "Requests MUST include a string or integer ID" and "the ID MUST NOT be null".
208+
it "returns an error and does not call the method when id is null" do
209+
called = false
210+
register("add") { called = true }
211+
212+
handle jsonrpc: "2.0", id: nil, method: "add", params: { a: 1, b: 2 }
213+
214+
assert_rpc_error expected_error: {
215+
code: -32600,
216+
message: "Invalid Request",
217+
data: "Request ID must not be null",
218+
}
219+
assert_nil @response[:id]
220+
refute called
221+
end
222+
223+
it "names the accepted id types without the pattern when none is given" do
224+
@response = JsonRpcHandler.handle({ jsonrpc: "2.0", id: 3.14, method: "add" }, id_validation_pattern: nil) { |_name, _id| nil }
225+
226+
assert_rpc_error expected_error: {
227+
code: -32600,
228+
message: "Invalid Request",
229+
data: "Request ID must be a string or an integer",
204230
}
205231
end
206232

@@ -323,7 +349,7 @@
323349
assert_rpc_error expected_error: {
324350
code: -32600,
325351
message: "Invalid Request",
326-
data: "Request ID must match validation pattern, or be an integer or null",
352+
data: "Request ID must be a string matching the validation pattern, or an integer",
327353
}
328354
assert_nil @response[:id]
329355
end
@@ -735,7 +761,7 @@
735761
assert_rpc_error expected_error: {
736762
code: -32600,
737763
message: "Invalid Request",
738-
data: "Request ID must match validation pattern, or be an integer or null",
764+
data: "Request ID must be a string matching the validation pattern, or an integer",
739765
}
740766
end
741767

@@ -748,7 +774,7 @@
748774
assert_rpc_error expected_error: {
749775
code: -32600,
750776
message: "Invalid Request",
751-
data: "Request ID must match validation pattern, or be an integer or null",
777+
data: "Request ID must be a string matching the validation pattern, or an integer",
752778
}
753779
end
754780

@@ -776,7 +802,7 @@
776802
assert_rpc_error expected_error: {
777803
code: -32600,
778804
message: "Invalid Request",
779-
data: "Request ID must match validation pattern, or be an integer or null",
805+
data: "Request ID must be a string matching the validation pattern, or an integer",
780806
}
781807
end
782808

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

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,82 @@ def string
157157
assert_equal JsonRpcHandler::ErrorCode::METHOD_NOT_FOUND, body["error"]["code"]
158158
end
159159

160+
test "a request whose id is null is answered with Invalid Request instead of being run unanswered" do
161+
calls = 0
162+
@server.define_tool(name: "count") do
163+
calls += 1
164+
Tool::Response.new([{ type: "text", text: "ok" }])
165+
end
166+
session_id = initialize_test_session
167+
168+
request = create_rack_request(
169+
"POST",
170+
"/",
171+
{ "CONTENT_TYPE" => "application/json", "HTTP_MCP_SESSION_ID" => session_id },
172+
{ jsonrpc: "2.0", id: nil, method: "tools/call", params: { name: "count", arguments: {} } }.to_json,
173+
)
174+
response = @transport.handle_request(request)
175+
176+
assert_equal 200, response[0]
177+
io = StringIO.new
178+
response[2].call(io)
179+
body = JSON.parse(io.string.match(/^data: (.+)$/)[1])
180+
assert_nil body["id"]
181+
assert_equal(-32600, body["error"]["code"])
182+
assert_equal 0, calls
183+
end
184+
185+
test "modern POST answers a request whose id is null with 400 and -32600" do
186+
calls = 0
187+
@server.define_tool(name: "count") do
188+
calls += 1
189+
Tool::Response.new([{ type: "text", text: "ok" }])
190+
end
191+
body = JSON.parse(modern_body("tools/call", { name: "count", arguments: {} }))
192+
body["id"] = nil
193+
194+
response = @transport.handle_request(modern_rack_request(body.to_json, headers: { "HTTP_MCP_NAME" => "count" }))
195+
196+
assert_equal 400, response[0]
197+
assert_equal(-32600, JSON.parse(response[2][0]).dig("error", "code"))
198+
assert_equal 0, calls
199+
end
200+
201+
test "a ping sent without an id is acknowledged with no response" do
202+
session_id = initialize_test_session
203+
204+
request = create_rack_request(
205+
"POST",
206+
"/",
207+
{ "CONTENT_TYPE" => "application/json", "HTTP_MCP_SESSION_ID" => session_id },
208+
{ jsonrpc: "2.0", method: "ping" }.to_json,
209+
)
210+
response = @transport.handle_request(request)
211+
212+
assert_equal 202, response[0]
213+
assert_equal [], response[2]
214+
end
215+
216+
test "a tools/call sent without an id is acknowledged without being run" do
217+
calls = 0
218+
@server.define_tool(name: "count") do
219+
calls += 1
220+
Tool::Response.new([{ type: "text", text: "ok" }])
221+
end
222+
session_id = initialize_test_session
223+
224+
request = create_rack_request(
225+
"POST",
226+
"/",
227+
{ "CONTENT_TYPE" => "application/json", "HTTP_MCP_SESSION_ID" => session_id },
228+
{ jsonrpc: "2.0", method: "tools/call", params: { name: "count", arguments: {} } }.to_json,
229+
)
230+
response = @transport.handle_request(request)
231+
232+
assert_equal 202, response[0]
233+
assert_equal 0, calls
234+
end
235+
160236
test "handles POST request with invalid JSON" do
161237
request = create_rack_request(
162238
"POST",

‎test/mcp/server_test.rb‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1924,6 +1924,42 @@ def read_resource_request(uri)
19241924
assert_instrumentation_data({ method: "unsupported_method" })
19251925
end
19261926

1927+
test "#handle refuses a request whose id is null without running it" do
1928+
calls = 0
1929+
@server.define_tool(name: "count") do
1930+
calls += 1
1931+
Tool::Response.new([{ type: "text", text: "ok" }])
1932+
end
1933+
1934+
response = @server.handle({ jsonrpc: "2.0", id: nil, method: "tools/call", params: { name: "count", arguments: {} } })
1935+
1936+
assert_equal({ jsonrpc: "2.0", id: nil, error: { code: -32600, message: "Invalid Request", data: "Request ID must not be null" } }, response)
1937+
assert_equal 0, calls
1938+
end
1939+
1940+
test "#handle does not run a protocol request method sent without an id" do
1941+
# Framed as a notification, the message could never be answered; the TypeScript and Python servers drop it too.
1942+
calls = 0
1943+
@server.define_tool(name: "count") do
1944+
calls += 1
1945+
Tool::Response.new([{ type: "text", text: "ok" }])
1946+
end
1947+
1948+
assert_nil @server.handle({ jsonrpc: "2.0", method: "tools/call", params: { name: "count", arguments: {} } })
1949+
assert_nil @server.handle({ jsonrpc: "2.0", method: "tools/list" })
1950+
assert_equal 0, calls
1951+
end
1952+
1953+
test "#handle still runs a custom method sent without an id" do
1954+
# `define_custom_method` serves notifications as well as requests, so only the protocol's own request methods need an id.
1955+
seen = []
1956+
@server.define_custom_method(method_name: "notify") { |params| seen << params[:event] }
1957+
1958+
assert_nil @server.handle({ jsonrpc: "2.0", method: "notify", params: { event: "custom" } })
1959+
1960+
assert_equal ["custom"], seen
1961+
end
1962+
19271963
test "#handle handles custom methods" do
19281964
@server.define_custom_method(method_name: "add") do |params|
19291965
params[:a] + params[:b]

0 commit comments

Comments
 (0)