From 0dace696da6455a6340a6aa88667795742d75f1f Mon Sep 17 00:00:00 2001 From: Justin Chang Date: Tue, 5 May 2026 19:01:31 -0700 Subject: [PATCH] mcp: write SSE comment on standalone stream so HTTP/2 reverse proxies flush HEADERS frame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #937. The standalone SSE GET stream (s.id == "") writes response headers, calls Flush(), and then waits indefinitely for server-pushed events. Issue #410 / PR #413 (refactored in PR #870 to use http.NewResponseController) ensure Flush() is called, which is correct for HTTP/1.1. On HTTP/2, headers and body travel as separate frames (HEADERS and DATA). Reverse proxies (Envoy, Caddy, net/http/httputil, etc.) commonly buffer the HEADERS frame until they have a DATA frame to coalesce it with — there is no HTTP/2 equivalent of HTTP/1.1's Transfer-Encoding: chunked signal that says "this is streaming, send headers now". Calling Flush() pushes the in-process buffer to the proxy, but the proxy still holds the HEADERS frame waiting for body data. Real-world impact: an MCP server behind Envoy hangs ~31s on session startup (matching the configured request timeout) because the client never sees the response headers until the proxy tears down the stream on timeout. Behavior reproduces with net/http/httputil in HTTP/2 mode as well; it is not Envoy-specific. This change writes an SSE comment (": ok\n\n") immediately after WriteHeader, before Flush. SSE comments are explicitly defined by the spec as lines starting with ":" that clients ignore, so the change is behavior-preserving for SSE clients. The point is the DATA frame this produces, which forces HTTP/2 reverse proxies to forward both frames together. Also adds a regression test that exercises the standalone SSE GET stream and asserts the first body bytes start with ":". The test times out at 2s without the fix and completes in <1ms with it. Refs: https://github.com/golang/go/issues/31125 https://github.com/caddyserver/caddy/issues/4247 --- mcp/streamable.go | 16 +++++++ mcp/streamable_test.go | 96 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+) diff --git a/mcp/streamable.go b/mcp/streamable.go index 06531715..7b053737 100644 --- a/mcp/streamable.go +++ b/mcp/streamable.go @@ -1061,7 +1061,23 @@ func (c *streamableServerConn) acquireStream(ctx context.Context, w http.Respons if s.id == "" { // Issue #410: the standalone SSE stream is likely not to receive messages // for a long time. Ensure that headers are flushed. + // + // On HTTP/2, headers and body travel as separate frames (HEADERS and + // DATA). Reverse proxies (e.g. Envoy, Caddy, net/http/httputil) + // commonly buffer the HEADERS frame until they have a DATA frame to + // coalesce it with — there is no HTTP/2 equivalent of HTTP/1.1's + // Transfer-Encoding: chunked signal that says "this is streaming, send + // headers now". Calling Flush() alone is not sufficient: it pushes + // the kernel buffer to the proxy, but the proxy still holds the + // HEADERS frame. + // + // Write an SSE comment (lines starting with ":" are ignored by + // clients per RFC) so a DATA frame is produced, which forces the + // proxy to forward both frames. See: + // https://github.com/golang/go/issues/31125 + // https://github.com/caddyserver/caddy/issues/4247 w.WriteHeader(http.StatusOK) + fmt.Fprint(w, ": ok\n\n") rc := http.NewResponseController(w) // Ignore returned error as flushing is best-effort. _ = rc.Flush() diff --git a/mcp/streamable_test.go b/mcp/streamable_test.go index 80fb4baa..118d05df 100644 --- a/mcp/streamable_test.go +++ b/mcp/streamable_test.go @@ -2904,3 +2904,99 @@ func TestStreamableOriginProtection(t *testing.T) { }) } } + +// TestStandaloneSSEEmitsCommentForHTTP2Flush is a regression test for the +// HTTP/2 reverse-proxy header-buffering bug. The standalone SSE GET stream +// must emit at least one body byte (an SSE comment) immediately after the +// response headers. Without a DATA frame, HTTP/2 reverse proxies (Envoy, +// Caddy, net/http/httputil, etc.) buffer the HEADERS frame indefinitely +// because they have nothing to coalesce it with — calling Flush() alone is +// not sufficient. +// +// SSE explicitly defines lines starting with ":" as comments that clients +// ignore, so this is behavior-preserving for the SSE protocol while +// producing the DATA frame that HTTP/2 proxies need. +func TestStandaloneSSEEmitsCommentForHTTP2Flush(t *testing.T) { + server := NewServer(testImpl, nil) + handler := NewStreamableHTTPHandler(func(*http.Request) *Server { return server }, nil) + httpServer := httptest.NewServer(mustNotPanic(t, handler)) + defer httpServer.Close() + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + // Initialize a session so we have a valid Mcp-Session-Id to use on the + // standalone SSE GET. + initBody := strings.NewReader(`{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-06-18","capabilities":{},"clientInfo":{"name":"test","version":"1.0"}}}`) + initReq, err := http.NewRequestWithContext(ctx, http.MethodPost, httpServer.URL, initBody) + if err != nil { + t.Fatal(err) + } + initReq.Header.Set("Content-Type", "application/json") + initReq.Header.Set("Accept", "application/json, text/event-stream") + initResp, err := http.DefaultClient.Do(initReq) + if err != nil { + t.Fatal(err) + } + io.Copy(io.Discard, initResp.Body) + initResp.Body.Close() + sessionID := initResp.Header.Get(sessionIDHeader) + if sessionID == "" { + t.Fatalf("initialize response missing %s header", sessionIDHeader) + } + + // Open the standalone SSE stream. + getReq, err := http.NewRequestWithContext(ctx, http.MethodGet, httpServer.URL, nil) + if err != nil { + t.Fatal(err) + } + getReq.Header.Set("Accept", "text/event-stream") + getReq.Header.Set(sessionIDHeader, sessionID) + + resp, err := http.DefaultClient.Do(getReq) + if err != nil { + t.Fatalf("GET standalone SSE: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %d; want 200", resp.StatusCode) + } + if got := resp.Header.Get("Content-Type"); !strings.HasPrefix(got, "text/event-stream") { + t.Fatalf("Content-Type = %q; want text/event-stream", got) + } + + // Read the first chunk. With the fix, an SSE comment (": ok\n\n") is + // written immediately. Without the fix, the body has no bytes to read + // until the server pushes an event, which won't happen in this test. + type readResult struct { + n int + err error + buf []byte + } + ch := make(chan readResult, 1) + go func() { + buf := make([]byte, 64) + n, err := resp.Body.Read(buf) + ch <- readResult{n: n, err: err, buf: buf} + }() + + select { + case r := <-ch: + if r.err != nil && r.err != io.EOF { + t.Fatalf("reading first SSE chunk: %v", r.err) + } + if r.n == 0 { + t.Fatal("first SSE chunk was empty; expected an SSE comment to flush HTTP/2 proxy HEADERS frame") + } + got := string(r.buf[:r.n]) + // SSE spec: lines starting with ":" are comments, ignored by clients. + // We don't pin the exact comment text; just require the first byte to + // be the SSE comment marker so HTTP/2 proxies have a DATA frame. + if !strings.HasPrefix(got, ":") { + t.Errorf("first SSE chunk = %q; want it to start with %q (SSE comment marker, so HTTP/2 proxies receive a DATA frame and forward HEADERS)", got, ":") + } + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for first SSE bytes; the standalone SSE stream must emit a DATA frame immediately so HTTP/2 reverse proxies don't buffer the HEADERS frame") + } +}