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
This commit is contained in:
parent
92e19c1be8
commit
a1eb4849b7
2 changed files with 23 additions and 15 deletions
|
|
@ -171,7 +171,7 @@ func (h *StreamableHTTPHandler) ServeHTTP(w http.ResponseWriter, req *http.Reque
|
||||||
|
|
||||||
switch req.Method {
|
switch req.Method {
|
||||||
case http.MethodPost, http.MethodGet:
|
case http.MethodPost, http.MethodGet:
|
||||||
if req.Method == http.MethodGet && sessionID == "" {
|
if req.Method == http.MethodGet && (h.opts.Stateless || sessionID == "") {
|
||||||
http.Error(w, "GET requires an active session", http.StatusMethodNotAllowed)
|
http.Error(w, "GET requires an active session", http.StatusMethodNotAllowed)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -799,7 +799,9 @@ func testStreamableHandler(t *testing.T, handler http.Handler, requests []stream
|
||||||
out := make(chan jsonrpc.Message)
|
out := make(chan jsonrpc.Message)
|
||||||
// Cancel the step if we encounter a request that isn't going to be
|
// Cancel the step if we encounter a request that isn't going to be
|
||||||
// handled.
|
// handled.
|
||||||
ctx, cancel := context.WithCancel(context.Background())
|
//
|
||||||
|
// Also, add a timeout (hopefully generous).
|
||||||
|
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
||||||
|
|
||||||
var wg sync.WaitGroup
|
var wg sync.WaitGroup
|
||||||
wg.Add(1)
|
wg.Add(1)
|
||||||
|
|
@ -1168,6 +1170,11 @@ func TestStreamableStateless(t *testing.T) {
|
||||||
wantBodyContaining: "greet",
|
wantBodyContaining: "greet",
|
||||||
wantSessionID: false,
|
wantSessionID: false,
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
method: "GET",
|
||||||
|
wantStatusCode: http.StatusMethodNotAllowed,
|
||||||
|
wantSessionID: false,
|
||||||
|
},
|
||||||
{
|
{
|
||||||
method: "POST",
|
method: "POST",
|
||||||
wantStatusCode: http.StatusOK,
|
wantStatusCode: http.StatusOK,
|
||||||
|
|
@ -1215,33 +1222,34 @@ func TestStreamableStateless(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
handler := NewStreamableHTTPHandler(func(*http.Request) *Server { return server }, &StreamableHTTPOptions{
|
sessionlessHandler := NewStreamableHTTPHandler(func(*http.Request) *Server { return server }, &StreamableHTTPOptions{
|
||||||
GetSessionID: func() string { return "" },
|
GetSessionID: func() string { return "" },
|
||||||
Stateless: true,
|
Stateless: true,
|
||||||
})
|
})
|
||||||
|
|
||||||
// Test the default stateless mode.
|
// First, test the "sessionless" stateless mode, where there is no session ID.
|
||||||
t.Run("stateless", func(t *testing.T) {
|
t.Run("sessionless", func(t *testing.T) {
|
||||||
testStreamableHandler(t, handler, requests)
|
testStreamableHandler(t, sessionlessHandler, requests)
|
||||||
testClientCompatibility(t, handler)
|
testClientCompatibility(t, sessionlessHandler)
|
||||||
})
|
})
|
||||||
|
|
||||||
// Test a "distributed" variant of stateless mode, where it has non-empty
|
// Next, test the default stateless mode, where session IDs are permitted.
|
||||||
// session IDs, but is otherwise stateless.
|
|
||||||
//
|
//
|
||||||
// This can be used by tools to look up application state preserved across
|
// This can be used by tools to look up application state preserved across
|
||||||
// subsequent requests.
|
// subsequent requests.
|
||||||
for i, req := range requests {
|
for i, req := range requests {
|
||||||
// Now, we want a session for all requests.
|
// Now, we want a session for all (valid) requests.
|
||||||
req.wantSessionID = true
|
if req.wantStatusCode != http.StatusMethodNotAllowed {
|
||||||
|
req.wantSessionID = true
|
||||||
|
}
|
||||||
requests[i] = req
|
requests[i] = req
|
||||||
}
|
}
|
||||||
distributableHandler := NewStreamableHTTPHandler(func(*http.Request) *Server { return server }, &StreamableHTTPOptions{
|
statelessHandler := NewStreamableHTTPHandler(func(*http.Request) *Server { return server }, &StreamableHTTPOptions{
|
||||||
Stateless: true,
|
Stateless: true,
|
||||||
})
|
})
|
||||||
t.Run("distributed", func(t *testing.T) {
|
t.Run("stateless", func(t *testing.T) {
|
||||||
testStreamableHandler(t, distributableHandler, requests)
|
testStreamableHandler(t, statelessHandler, requests)
|
||||||
testClientCompatibility(t, handler)
|
testClientCompatibility(t, sessionlessHandler)
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue