Skip to content

Commit a1eb484

Browse files
committed
mcp: correctly disallow GET requests on stateless servers
The condition sessionID == "" was not quite right for disallowing GET requests. Since we decided to differentiate "stateless" vs "sessionless" servers, we need to disallow GET requests for both. For #393
1 parent 92e19c1 commit a1eb484

File tree

2 files changed

+23
-15
lines changed

2 files changed

+23
-15
lines changed

mcp/streamable.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ func (h *StreamableHTTPHandler) ServeHTTP(w http.ResponseWriter, req *http.Reque
171171

172172
switch req.Method {
173173
case http.MethodPost, http.MethodGet:
174-
if req.Method == http.MethodGet && sessionID == "" {
174+
if req.Method == http.MethodGet && (h.opts.Stateless || sessionID == "") {
175175
http.Error(w, "GET requires an active session", http.StatusMethodNotAllowed)
176176
return
177177
}

mcp/streamable_test.go

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -799,7 +799,9 @@ func testStreamableHandler(t *testing.T, handler http.Handler, requests []stream
799799
out := make(chan jsonrpc.Message)
800800
// Cancel the step if we encounter a request that isn't going to be
801801
// handled.
802-
ctx, cancel := context.WithCancel(context.Background())
802+
//
803+
// Also, add a timeout (hopefully generous).
804+
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
803805

804806
var wg sync.WaitGroup
805807
wg.Add(1)
@@ -1168,6 +1170,11 @@ func TestStreamableStateless(t *testing.T) {
11681170
wantBodyContaining: "greet",
11691171
wantSessionID: false,
11701172
},
1173+
{
1174+
method: "GET",
1175+
wantStatusCode: http.StatusMethodNotAllowed,
1176+
wantSessionID: false,
1177+
},
11711178
{
11721179
method: "POST",
11731180
wantStatusCode: http.StatusOK,
@@ -1215,33 +1222,34 @@ func TestStreamableStateless(t *testing.T) {
12151222
}
12161223
}
12171224

1218-
handler := NewStreamableHTTPHandler(func(*http.Request) *Server { return server }, &StreamableHTTPOptions{
1225+
sessionlessHandler := NewStreamableHTTPHandler(func(*http.Request) *Server { return server }, &StreamableHTTPOptions{
12191226
GetSessionID: func() string { return "" },
12201227
Stateless: true,
12211228
})
12221229

1223-
// Test the default stateless mode.
1224-
t.Run("stateless", func(t *testing.T) {
1225-
testStreamableHandler(t, handler, requests)
1226-
testClientCompatibility(t, handler)
1230+
// First, test the "sessionless" stateless mode, where there is no session ID.
1231+
t.Run("sessionless", func(t *testing.T) {
1232+
testStreamableHandler(t, sessionlessHandler, requests)
1233+
testClientCompatibility(t, sessionlessHandler)
12271234
})
12281235

1229-
// Test a "distributed" variant of stateless mode, where it has non-empty
1230-
// session IDs, but is otherwise stateless.
1236+
// Next, test the default stateless mode, where session IDs are permitted.
12311237
//
12321238
// This can be used by tools to look up application state preserved across
12331239
// subsequent requests.
12341240
for i, req := range requests {
1235-
// Now, we want a session for all requests.
1236-
req.wantSessionID = true
1241+
// Now, we want a session for all (valid) requests.
1242+
if req.wantStatusCode != http.StatusMethodNotAllowed {
1243+
req.wantSessionID = true
1244+
}
12371245
requests[i] = req
12381246
}
1239-
distributableHandler := NewStreamableHTTPHandler(func(*http.Request) *Server { return server }, &StreamableHTTPOptions{
1247+
statelessHandler := NewStreamableHTTPHandler(func(*http.Request) *Server { return server }, &StreamableHTTPOptions{
12401248
Stateless: true,
12411249
})
1242-
t.Run("distributed", func(t *testing.T) {
1243-
testStreamableHandler(t, distributableHandler, requests)
1244-
testClientCompatibility(t, handler)
1250+
t.Run("stateless", func(t *testing.T) {
1251+
testStreamableHandler(t, statelessHandler, requests)
1252+
testClientCompatibility(t, sessionlessHandler)
12451253
})
12461254
}
12471255

0 commit comments

Comments
 (0)