Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/json_rpc_handler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
20 changes: 14 additions & 6 deletions lib/mcp/server/transports/streamable_http_transport.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
15 changes: 15 additions & 0 deletions test/json_rpc_handler_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand Down
100 changes: 100 additions & 0 deletions test/mcp/server/transports/streamable_http_transport_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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.
Expand Down
Loading