Use mutex-protected session_exists? in handle_regular_request
## Motivation and Context `handle_regular_request` was checking `@sessions.key?(session_id)` directly without holding `@mutex`, while concurrent threads could modify `@sessions` via `cleanup_session` or `handle_delete`. This created a TOCTOU race where the check could pass but the session could be deleted before subsequent use. The class already provides a mutex-protected `session_exists?` helper, and `handle_get` already uses it. This change makes `handle_regular_request` consistent with `handle_get`. ## How Has This Been Tested? Added a test that verifies `handle_regular_request` delegates to the mutex-protected `session_exists?` helper instead of accessing `@sessions` directly. All existing tests pass. ## Breaking Change None.
This commit is contained in:
parent
8823bd34ef
commit
be1bbc25c1
2 changed files with 26 additions and 2 deletions
|
|
@ -259,8 +259,7 @@ module MCP
|
||||||
|
|
||||||
def handle_regular_request(body_string, session_id)
|
def handle_regular_request(body_string, session_id)
|
||||||
unless @stateless
|
unless @stateless
|
||||||
# If session ID is provided, but not in the sessions hash, return an error
|
if session_id && !session_exists?(session_id)
|
||||||
if session_id && !@sessions.key?(session_id)
|
|
||||||
return session_not_found_response
|
return session_not_found_response
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
||||||
|
|
@ -1222,6 +1222,31 @@ module MCP
|
||||||
assert_equal([], response[2])
|
assert_equal([], response[2])
|
||||||
end
|
end
|
||||||
|
|
||||||
|
test "handle_regular_request checks session existence under mutex" 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"]
|
||||||
|
|
||||||
|
@transport.expects(:session_exists?).with(session_id).returns(true)
|
||||||
|
|
||||||
|
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(request)
|
||||||
|
assert_equal(200, response[0])
|
||||||
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
def create_rack_request(method, path, headers, body = nil)
|
def create_rack_request(method, path, headers, body = nil)
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue