Skip to content

Commit 526d643

Browse files
committed
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.
1 parent 6e35d13 commit 526d643

File tree

2 files changed

+26
-2
lines changed

2 files changed

+26
-2
lines changed

lib/mcp/server/transports/streamable_http_transport.rb

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,8 +259,7 @@ def handle_accepted
259259

260260
def handle_regular_request(body_string, session_id)
261261
unless @stateless
262-
# If session ID is provided, but not in the sessions hash, return an error
263-
if session_id && !@sessions.key?(session_id)
262+
if session_id && !session_exists?(session_id)
264263
return [400, { "Content-Type" => "application/json" }, [{ error: "Invalid session ID" }.to_json]]
265264
end
266265
end

test/mcp/server/transports/streamable_http_transport_test.rb

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1203,6 +1203,31 @@ class StreamableHTTPTransportTest < ActiveSupport::TestCase
12031203
assert_equal([], response[2])
12041204
end
12051205

1206+
test "handle_regular_request checks session existence under mutex" do
1207+
init_request = create_rack_request(
1208+
"POST",
1209+
"/",
1210+
{ "CONTENT_TYPE" => "application/json" },
1211+
{ jsonrpc: "2.0", method: "initialize", id: "init" }.to_json,
1212+
)
1213+
init_response = @transport.handle_request(init_request)
1214+
session_id = init_response[1]["Mcp-Session-Id"]
1215+
1216+
@transport.expects(:session_exists?).with(session_id).returns(true)
1217+
1218+
request = create_rack_request(
1219+
"POST",
1220+
"/",
1221+
{
1222+
"CONTENT_TYPE" => "application/json",
1223+
"HTTP_MCP_SESSION_ID" => session_id,
1224+
},
1225+
{ jsonrpc: "2.0", method: "ping", id: "456" }.to_json,
1226+
)
1227+
response = @transport.handle_request(request)
1228+
assert_equal(200, response[0])
1229+
end
1230+
12061231
private
12071232

12081233
def create_rack_request(method, path, headers, body = nil)

0 commit comments

Comments
 (0)