Return 404 for invalid session ID in handle_delete
## Motivation and Context The MCP specification requires that when a server receives a request containing a session ID that is no longer valid, it MUST respond with HTTP 404 Not Found. `handle_delete` was unconditionally calling `cleanup_session` and returning 200 even for nonexistent session IDs. Ref: https://modelcontextprotocol.io/specification/2025-11-25/basic/transports#session-management > The server MAY terminate the session at any time, after which it MUST respond > to requests containing that session ID with HTTP 404 Not Found. ## How Has This Been Tested? Added tests for DELETE with invalid session ID, and lifecycle tests for delete-then-POST and delete-then-DELETE scenarios. ## Breaking Change DELETE requests with an invalid session ID now return HTTP 404 "Session not found" instead of HTTP 200. However, this change is considered a bug fix because it brings the behavior into compliance with the MCP specification.
This commit is contained in:
parent
283c7a43e4
commit
abc7d6c7ea
2 changed files with 80 additions and 0 deletions
|
|
@ -155,8 +155,10 @@ module MCP
|
||||||
end
|
end
|
||||||
|
|
||||||
return missing_session_id_response unless (session_id = request.env["HTTP_MCP_SESSION_ID"])
|
return missing_session_id_response unless (session_id = request.env["HTTP_MCP_SESSION_ID"])
|
||||||
|
return session_not_found_response unless session_exists?(session_id)
|
||||||
|
|
||||||
cleanup_session(session_id)
|
cleanup_session(session_id)
|
||||||
|
|
||||||
success_response
|
success_response
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -332,6 +332,84 @@ module MCP
|
||||||
assert body["success"]
|
assert body["success"]
|
||||||
end
|
end
|
||||||
|
|
||||||
|
test "handles DELETE request with invalid session ID" do
|
||||||
|
request = create_rack_request(
|
||||||
|
"DELETE",
|
||||||
|
"/",
|
||||||
|
{ "HTTP_MCP_SESSION_ID" => "invalid_id" },
|
||||||
|
)
|
||||||
|
|
||||||
|
response = @transport.handle_request(request)
|
||||||
|
assert_equal 404, response[0]
|
||||||
|
assert_equal({ "Content-Type" => "application/json" }, response[1])
|
||||||
|
|
||||||
|
body = JSON.parse(response[2][0])
|
||||||
|
assert_equal "Session not found", body["error"]
|
||||||
|
end
|
||||||
|
|
||||||
|
test "POST returns 404 after session is deleted" do
|
||||||
|
init_request = create_rack_request(
|
||||||
|
"POST",
|
||||||
|
"/",
|
||||||
|
{ "CONTENT_TYPE" => "application/json" },
|
||||||
|
{ jsonrpc: "2.0", method: "initialize", id: "init" }.to_json,
|
||||||
|
)
|
||||||
|
init_response = @transport.handle_request(init_request)
|
||||||
|
session_id = init_response[1]["Mcp-Session-Id"]
|
||||||
|
|
||||||
|
delete_request = create_rack_request(
|
||||||
|
"DELETE",
|
||||||
|
"/",
|
||||||
|
{ "HTTP_MCP_SESSION_ID" => session_id },
|
||||||
|
)
|
||||||
|
@transport.handle_request(delete_request)
|
||||||
|
|
||||||
|
post_request = create_rack_request(
|
||||||
|
"POST",
|
||||||
|
"/",
|
||||||
|
{
|
||||||
|
"CONTENT_TYPE" => "application/json",
|
||||||
|
"HTTP_MCP_SESSION_ID" => session_id,
|
||||||
|
},
|
||||||
|
{ jsonrpc: "2.0", method: "ping", id: "456" }.to_json,
|
||||||
|
)
|
||||||
|
response = @transport.handle_request(post_request)
|
||||||
|
assert_equal 404, response[0]
|
||||||
|
|
||||||
|
body = JSON.parse(response[2][0])
|
||||||
|
assert_equal "Session not found", body["error"]
|
||||||
|
end
|
||||||
|
|
||||||
|
test "DELETE returns 404 after session is already deleted" do
|
||||||
|
init_request = create_rack_request(
|
||||||
|
"POST",
|
||||||
|
"/",
|
||||||
|
{ "CONTENT_TYPE" => "application/json" },
|
||||||
|
{ jsonrpc: "2.0", method: "initialize", id: "init" }.to_json,
|
||||||
|
)
|
||||||
|
init_response = @transport.handle_request(init_request)
|
||||||
|
session_id = init_response[1]["Mcp-Session-Id"]
|
||||||
|
|
||||||
|
first_delete = create_rack_request(
|
||||||
|
"DELETE",
|
||||||
|
"/",
|
||||||
|
{ "HTTP_MCP_SESSION_ID" => session_id },
|
||||||
|
)
|
||||||
|
response = @transport.handle_request(first_delete)
|
||||||
|
assert_equal 200, response[0]
|
||||||
|
|
||||||
|
second_delete = create_rack_request(
|
||||||
|
"DELETE",
|
||||||
|
"/",
|
||||||
|
{ "HTTP_MCP_SESSION_ID" => session_id },
|
||||||
|
)
|
||||||
|
response = @transport.handle_request(second_delete)
|
||||||
|
assert_equal 404, response[0]
|
||||||
|
|
||||||
|
body = JSON.parse(response[2][0])
|
||||||
|
assert_equal "Session not found", body["error"]
|
||||||
|
end
|
||||||
|
|
||||||
test "handles DELETE request with missing session ID" do
|
test "handles DELETE request with missing session ID" do
|
||||||
request = create_rack_request(
|
request = create_rack_request(
|
||||||
"DELETE",
|
"DELETE",
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue