diff --git a/mcp/sse.go b/mcp/sse.go index a668c6d0..ae65c16c 100644 --- a/mcp/sse.go +++ b/mcp/sse.go @@ -202,7 +202,8 @@ func (h *SSEHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { } if req.Method != http.MethodGet { - http.Error(w, "invalid method", http.StatusMethodNotAllowed) + w.Header().Set("Allow", "GET, POST") + http.Error(w, "Method Not Allowed", http.StatusMethodNotAllowed) return } diff --git a/mcp/sse_test.go b/mcp/sse_test.go index 132a0317..8746cc8b 100644 --- a/mcp/sse_test.go +++ b/mcp/sse_test.go @@ -186,3 +186,38 @@ func TestSSEClientTransport_HTTPErrors(t *testing.T) { }) } } + +// TestSSE405AllowHeader verifies RFC 9110 §15.5.6 compliance: +// 405 Method Not Allowed responses MUST include an Allow header. +func TestSSE405AllowHeader(t *testing.T) { + server := NewServer(testImpl, nil) + + handler := NewSSEHandler(func(req *http.Request) *Server { return server }, nil) + httpServer := httptest.NewServer(handler) + defer httpServer.Close() + + methods := []string{"PUT", "PATCH", "DELETE", "OPTIONS"} + for _, method := range methods { + t.Run(method, func(t *testing.T) { + req, err := http.NewRequest(method, httpServer.URL, nil) + if err != nil { + t.Fatal(err) + } + + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + + if got, want := resp.StatusCode, http.StatusMethodNotAllowed; got != want { + t.Errorf("status code: got %d, want %d", got, want) + } + + allow := resp.Header.Get("Allow") + if allow != "GET, POST" { + t.Errorf("Allow header: got %q, want %q", allow, "GET, POST") + } + }) + } +} diff --git a/mcp/streamable.go b/mcp/streamable.go index 167d6847..f2a28955 100644 --- a/mcp/streamable.go +++ b/mcp/streamable.go @@ -275,12 +275,27 @@ func (h *StreamableHTTPHandler) ServeHTTP(w http.ResponseWriter, req *http.Reque switch req.Method { case http.MethodPost, http.MethodGet: if req.Method == http.MethodGet && (h.opts.Stateless || sessionID == "") { - http.Error(w, "GET requires an active session", http.StatusMethodNotAllowed) + if h.opts.Stateless { + // Per MCP spec: server MUST return 405 if it doesn't offer SSE stream. + // In stateless mode, GET (SSE streaming) is not supported. + // RFC 9110 §15.5.6: 405 responses MUST include Allow header. + w.Header().Set("Allow", "POST") + http.Error(w, "Method Not Allowed", http.StatusMethodNotAllowed) + } else { + // In stateful mode, GET is supported but requires a session ID. + // This is a precondition error, similar to DELETE without session. + http.Error(w, "Bad Request: GET requires an Mcp-Session-Id header", http.StatusBadRequest) + } return } default: - w.Header().Set("Allow", "GET, POST, DELETE") - http.Error(w, "Method Not Allowed: streamable MCP servers support GET, POST, and DELETE requests", http.StatusMethodNotAllowed) + // RFC 9110 §15.5.6: 405 responses MUST include Allow header. + if h.opts.Stateless { + w.Header().Set("Allow", "POST") + } else { + w.Header().Set("Allow", "GET, POST, DELETE") + } + http.Error(w, "Method Not Allowed", http.StatusMethodNotAllowed) return } diff --git a/mcp/streamable_test.go b/mcp/streamable_test.go index b1c3f074..56c0a49b 100644 --- a/mcp/streamable_test.go +++ b/mcp/streamable_test.go @@ -1829,7 +1829,9 @@ func TestStreamableGET(t *testing.T) { if err != nil { t.Fatal(err) } - if got, want := resp.StatusCode, http.StatusMethodNotAllowed; got != want { + // GET without session should return 400 Bad Request (not 405) because + // GET is a valid method - it just requires a session ID. + if got, want := resp.StatusCode, http.StatusBadRequest; got != want { t.Errorf("initial GET: got status %d, want %d", got, want) } defer resp.Body.Close() @@ -1877,6 +1879,101 @@ func TestStreamableGET(t *testing.T) { } } +// TestStreamable405AllowHeader verifies RFC 9110 §15.5.6 compliance: +// 405 Method Not Allowed responses MUST include an Allow header. +func TestStreamable405AllowHeader(t *testing.T) { + server := NewServer(testImpl, nil) + + tests := []struct { + name string + stateless bool + method string + wantStatus int + wantAllow string + }{ + { + name: "unsupported method stateful", + stateless: false, + method: "PUT", + wantStatus: http.StatusMethodNotAllowed, + wantAllow: "GET, POST, DELETE", + }, + { + name: "GET in stateless mode", + stateless: true, + method: "GET", + wantStatus: http.StatusMethodNotAllowed, + wantAllow: "POST", + }, + { + // DELETE without session returns 400 Bad Request (not 405) + // because DELETE is a valid method, just requires a session ID. + name: "DELETE without session stateless", + stateless: true, + method: "DELETE", + wantStatus: http.StatusBadRequest, + wantAllow: "", // No Allow header for 400 responses + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + opts := &StreamableHTTPOptions{Stateless: tt.stateless} + handler := NewStreamableHTTPHandler(func(req *http.Request) *Server { return server }, opts) + httpServer := httptest.NewServer(mustNotPanic(t, handler)) + defer httpServer.Close() + + req, err := http.NewRequest(tt.method, httpServer.URL, nil) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Accept", "application/json, text/event-stream") + + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + + if got := resp.StatusCode; got != tt.wantStatus { + t.Errorf("status code: got %d, want %d", got, tt.wantStatus) + } + + allow := resp.Header.Get("Allow") + if allow != tt.wantAllow { + t.Errorf("Allow header: got %q, want %q", allow, tt.wantAllow) + } + }) + } +} + +// TestStreamableGETWithoutSession verifies that GET without session ID in stateful mode +// returns 400 Bad Request (not 405), since GET is a supported method that requires a session. +func TestStreamableGETWithoutSession(t *testing.T) { + server := NewServer(testImpl, nil) + handler := NewStreamableHTTPHandler(func(req *http.Request) *Server { return server }, nil) + httpServer := httptest.NewServer(mustNotPanic(t, handler)) + defer httpServer.Close() + + req, err := http.NewRequest("GET", httpServer.URL, nil) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Accept", "text/event-stream") + + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + + // GET without session should return 400 Bad Request, not 405 Method Not Allowed, + // because GET is a valid method - it just requires a session ID. + if got, want := resp.StatusCode, http.StatusBadRequest; got != want { + t.Errorf("status code: got %d, want %d", got, want) + } +} + func TestStreamableClientContextPropagation(t *testing.T) { type contextKey string const testKey = contextKey("test-key")