Skip to content

Commit 8bdd579

Browse files
Align request-body limit with the MCP SDK default
The middleware default was an arbitrary 10 MiB, above the 4 MiB the SDK already enforces, so it never changed which requests were accepted. Alias mcp.DefaultMaxRequestBodyBytes instead, making the earlier enforcement point behaviour-preserving by construction. Also pass the effective limit to StreamableHTTPOptions. Previously the SDK kept its own 4 MiB default, so a larger configured MaxRequestBodyBytes was silently capped; both layers now agree. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 8597b1a commit 8bdd579

7 files changed

Lines changed: 81 additions & 73 deletions

File tree

pkg/http/handler.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,7 @@ func NewHTTPMcpHandler(
132132

133133
func (h *Handler) RegisterMiddleware(r chi.Router) {
134134
r.Use(
135-
// Must run first: bounds the request body before any other
136-
// middleware (or the MCP SDK) reads or buffers it.
135+
// Must run first: bounds the body before anything downstream reads it.
137136
middleware.WithMaxBodySize(h.maxRequestBodyBytes()),
138137
middleware.ExtractUserToken(h.oauthCfg),
139138
middleware.WithRequestConfig,
@@ -146,8 +145,8 @@ func (h *Handler) RegisterMiddleware(r chi.Router) {
146145
}
147146
}
148147

149-
// maxRequestBodyBytes returns the configured request-body size limit, or
150-
// middleware.DefaultMaxRequestBodyBytes if none was configured.
148+
// maxRequestBodyBytes returns the effective request-body size limit, applied
149+
// both by the early middleware and by the MCP SDK handler.
151150
func (h *Handler) maxRequestBodyBytes() int64 {
152151
if h.config != nil && h.config.MaxRequestBodyBytes > 0 {
153152
return h.config.MaxRequestBodyBytes
@@ -251,6 +250,9 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
251250
return ghServer
252251
}, &mcp.StreamableHTTPOptions{
253252
Stateless: true,
253+
// Keep the SDK's own guard in step with the middleware, otherwise its
254+
// default would silently cap a larger configured limit.
255+
MaxRequestBodyBytes: h.maxRequestBodyBytes(),
254256
})
255257

256258
mcpHandler.ServeHTTP(w, r)

pkg/http/handler_test.go

Lines changed: 52 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package http
33
import (
44
"context"
55
"encoding/json"
6+
"fmt"
67
"log/slog"
78
"net/http"
89
"net/http/httptest"
@@ -1288,11 +1289,23 @@ func TestUIMetaStrippedWhenClientLacksCapability(t *testing.T) {
12881289
require.NotNil(t, unknown[0].Tool.Meta["ui"], "_meta.ui should be preserved when capability is unknown and FF is on")
12891290
}
12901291

1291-
// TestRegisterMiddleware_MaxRequestBodySize verifies that RegisterMiddleware
1292-
// wires the body-size limit ahead of the body-consuming middleware, so an
1293-
// oversized request never reaches the MCP server, and that requests within
1294-
// the configured limit (including exactly at the boundary) still succeed.
1295-
func TestRegisterMiddleware_MaxRequestBodySize(t *testing.T) {
1292+
// TestMaxRequestBodyBytes checks the effective limit tracks the MCP SDK
1293+
// default and honours an operator override.
1294+
func TestMaxRequestBodyBytes(t *testing.T) {
1295+
t.Run("defaults to the MCP SDK limit", func(t *testing.T) {
1296+
h := &Handler{config: &ServerConfig{}}
1297+
assert.Equal(t, int64(mcp.DefaultMaxRequestBodyBytes), h.maxRequestBodyBytes())
1298+
})
1299+
1300+
t.Run("configured value overrides the default", func(t *testing.T) {
1301+
h := &Handler{config: &ServerConfig{MaxRequestBodyBytes: 1234}}
1302+
assert.Equal(t, int64(1234), h.maxRequestBodyBytes())
1303+
})
1304+
}
1305+
1306+
// TestMaxRequestBodySizeEnforcement exercises both layers the limit is applied
1307+
// at: the early middleware, and the MCP SDK handler the request is delegated to.
1308+
func TestMaxRequestBodySizeEnforcement(t *testing.T) {
12961309
const limit = 256
12971310

12981311
apiHost, err := utils.NewAPIHost("https://api.github.com")
@@ -1307,9 +1320,9 @@ func TestRegisterMiddleware_MaxRequestBodySize(t *testing.T) {
13071320
return strings.Replace(payload, "PADDING", "PADDING"+pad, 1)
13081321
}
13091322

1310-
newHandler := func(t *testing.T, mcpServerFactoryCalled *bool) http.Handler {
1323+
newHandler := func(t *testing.T, mcpServerFactoryCalled *bool) *Handler {
13111324
t.Helper()
1312-
handler := NewHTTPMcpHandler(
1325+
return NewHTTPMcpHandler(
13131326
context.Background(),
13141327
&ServerConfig{Version: "test", MaxRequestBodyBytes: limit},
13151328
nil,
@@ -1327,47 +1340,63 @@ func TestRegisterMiddleware_MaxRequestBodySize(t *testing.T) {
13271340
}),
13281341
WithScopeFetcher(allScopesFetcher{}),
13291342
)
1343+
}
13301344

1345+
newRouter := func(h *Handler) http.Handler {
13311346
r := chi.NewRouter()
1332-
handler.RegisterMiddleware(r)
1333-
handler.RegisterRoutes(r)
1347+
h.RegisterMiddleware(r)
1348+
h.RegisterRoutes(r)
13341349
return r
13351350
}
13361351

1337-
t.Run("oversized request is rejected before reaching the MCP server", func(t *testing.T) {
1352+
newRequest := func(body string) *http.Request {
1353+
req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body))
1354+
req.Header.Set(headers.ContentTypeHeader, headers.ContentTypeJSON)
1355+
req.Header.Set(headers.AcceptHeader, strings.Join([]string{headers.ContentTypeJSON, headers.ContentTypeEventStream}, ", "))
1356+
req.Header.Set(headers.AuthorizationHeader, strings.Join([]string{"ghs", "test-token"}, "_"))
1357+
return req
1358+
}
1359+
1360+
t.Run("middleware rejects an oversized request before the MCP server is built", func(t *testing.T) {
13381361
var mcpServerFactoryCalled bool
1339-
r := newHandler(t, &mcpServerFactoryCalled)
1362+
r := newRouter(newHandler(t, &mcpServerFactoryCalled))
13401363

13411364
body := buildBody(limit + 1)
13421365
require.Greater(t, len(body), limit)
13431366

1344-
req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body))
1345-
req.Header.Set(headers.AuthorizationHeader, strings.Join([]string{"ghs", "test-token"}, "_"))
1346-
13471367
rr := httptest.NewRecorder()
1348-
r.ServeHTTP(rr, req)
1368+
r.ServeHTTP(rr, newRequest(body))
13491369

13501370
assert.Equal(t, http.StatusRequestEntityTooLarge, rr.Code)
13511371
assert.Contains(t, rr.Body.String(), "request body too large")
13521372
assert.False(t, mcpServerFactoryCalled, "the MCP server should never be constructed for an oversized request")
13531373
})
13541374

1355-
t.Run("boundary-size request at the configured limit succeeds", func(t *testing.T) {
1375+
t.Run("request at the configured limit succeeds", func(t *testing.T) {
13561376
var mcpServerFactoryCalled bool
1357-
r := newHandler(t, &mcpServerFactoryCalled)
1377+
r := newRouter(newHandler(t, &mcpServerFactoryCalled))
13581378

13591379
body := buildBody(limit)
13601380
require.Len(t, body, limit)
13611381

1362-
req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body))
1363-
req.Header.Set(headers.ContentTypeHeader, headers.ContentTypeJSON)
1364-
req.Header.Set(headers.AcceptHeader, strings.Join([]string{headers.ContentTypeJSON, headers.ContentTypeEventStream}, ", "))
1365-
req.Header.Set(headers.AuthorizationHeader, strings.Join([]string{"ghs", "test-token"}, "_"))
1366-
13671382
rr := httptest.NewRecorder()
1368-
r.ServeHTTP(rr, req)
1383+
r.ServeHTTP(rr, newRequest(body))
13691384

13701385
assert.Equal(t, http.StatusOK, rr.Code, "response body: %s", rr.Body.String())
13711386
assert.True(t, mcpServerFactoryCalled, "the MCP server should be constructed for an allowed request")
13721387
})
1388+
1389+
t.Run("SDK handler enforces the configured limit when the middleware is bypassed", func(t *testing.T) {
1390+
h := newHandler(t, nil)
1391+
1392+
body := buildBody(limit + 1)
1393+
require.Greater(t, len(body), limit)
1394+
1395+
rr := httptest.NewRecorder()
1396+
h.ServeHTTP(rr, newRequest(body))
1397+
1398+
assert.Equal(t, http.StatusRequestEntityTooLarge, rr.Code)
1399+
assert.Contains(t, rr.Body.String(), fmt.Sprintf("request body exceeds %d bytes", limit),
1400+
"the SDK should report the configured limit, not its own default")
1401+
})
13731402
}

pkg/http/middleware/body_limit.go

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,24 +3,22 @@ package middleware
33
import (
44
"errors"
55
"net/http"
6+
7+
"github.com/modelcontextprotocol/go-sdk/mcp"
68
)
79

8-
// DefaultMaxRequestBodyBytes bounds the size of HTTP request bodies accepted
9-
// by the MCP endpoints when no explicit limit is configured.
10-
const DefaultMaxRequestBodyBytes int64 = 10 << 20 // 10 MiB
10+
// DefaultMaxRequestBodyBytes tracks the MCP SDK's own request-body limit, so
11+
// enforcing it earlier in the chain does not change which requests are accepted.
12+
const DefaultMaxRequestBodyBytes int64 = mcp.DefaultMaxRequestBodyBytes
1113

12-
// WithMaxBodySize returns middleware that bounds the size of the request
13-
// body. It must be registered before any middleware that reads or buffers
14-
// the body (e.g. WithMCPParse, WithScopeChallenge) so that an oversized
15-
// payload is rejected before it is ever fully buffered in memory, rather than
16-
// relying on a size guard applied later by the MCP SDK or a downstream
17-
// handler.
14+
// WithMaxBodySize bounds the size of the request body. It must be registered
15+
// before any middleware that reads or buffers the body (WithMCPParse,
16+
// WithScopeChallenge), so an oversized payload is rejected before it is
17+
// buffered in memory rather than by a later guard in the MCP SDK.
1818
//
19-
// When Content-Length is known and already exceeds maxBytes, the request is
20-
// rejected immediately without touching the body. Otherwise the body is
21-
// wrapped with http.MaxBytesReader, so any subsequent read (including
22-
// chunked or unknown-length bodies) fails with a *http.MaxBytesError once
23-
// maxBytes have been consumed.
19+
// A body of unknown length (chunked, HTTP/2) cannot be rejected upfront, so
20+
// the limit is instead enforced on read and surfaces as a *http.MaxBytesError
21+
// to whichever middleware reads the body first.
2422
func WithMaxBodySize(maxBytes int64) func(http.Handler) http.Handler {
2523
return func(next http.Handler) http.Handler {
2624
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -38,16 +36,10 @@ func WithMaxBodySize(maxBytes int64) func(http.Handler) http.Handler {
3836
}
3937
}
4038

41-
// writeRequestTooLarge writes the standard "request body too large" response.
42-
// Every middleware that reads the request body should use this so oversized
43-
// requests get a consistent, clear response regardless of which layer
44-
// detects the overflow.
4539
func writeRequestTooLarge(w http.ResponseWriter) {
4640
http.Error(w, "request body too large", http.StatusRequestEntityTooLarge)
4741
}
4842

49-
// isMaxBytesError reports whether err resulted from a body exceeding the
50-
// limit applied by WithMaxBodySize, as opposed to some other read failure.
5143
func isMaxBytesError(err error) bool {
5244
var maxBytesErr *http.MaxBytesError
5345
return errors.As(err, &maxBytesErr)

pkg/http/middleware/body_limit_test.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,8 @@ import (
1111
"github.com/stretchr/testify/require"
1212
)
1313

14-
// unknownLengthBody wraps a reader without exposing a Len method, so
15-
// httptest.NewRequest cannot infer Content-Length from it. This mirrors a
16-
// chunked-transfer-encoded request, where the body size is unknown upfront.
14+
// unknownLengthBody hides Len from httptest.NewRequest so ContentLength is -1,
15+
// as it is for a chunked or HTTP/2 request.
1716
func unknownLengthBody(s string) io.Reader {
1817
return io.NopCloser(strings.NewReader(s))
1918
}

pkg/http/middleware/mcp_parse_test.go

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -190,11 +190,8 @@ func TestWithMCPParse_BodyRestoration(t *testing.T) {
190190
assert.Equal(t, originalBody, capturedBody, "body should be restored for downstream handlers")
191191
}
192192

193-
// TestWithMCPParse_WithMaxBodySize composes the body-size limit with
194-
// WithMCPParse, mirroring the production middleware ordering where
195-
// WithMaxBodySize runs first. It verifies that an oversized body is rejected
196-
// with a clear 413 before parsing runs, while requests within the limit
197-
// (including exactly at the boundary) still parse and preserve the body.
193+
// TestWithMCPParse_WithMaxBodySize mirrors the production middleware ordering,
194+
// where WithMaxBodySize runs ahead of WithMCPParse.
198195
func TestWithMCPParse_WithMaxBodySize(t *testing.T) {
199196
const limit = 128
200197

@@ -203,7 +200,6 @@ func TestWithMCPParse_WithMaxBodySize(t *testing.T) {
203200
if len(payload) >= size {
204201
return payload
205202
}
206-
// Pad the JSON with a longer string value so we can hit an exact byte size.
207203
pad := strings.Repeat("x", size-len(payload))
208204
return strings.Replace(payload, "PADDING", "PADDING"+pad, 1)
209205
}
@@ -219,11 +215,8 @@ func TestWithMCPParse_WithMaxBodySize(t *testing.T) {
219215

220216
handler := WithMaxBodySize(limit)(WithMCPParse()(nextHandler))
221217

222-
// Use an unknown-length body so WithMaxBodySize can't reject the
223-
// request via its known-Content-Length fast path (already covered by
224-
// body_limit_test.go). This forces the request through to
225-
// WithMCPParse's own io.ReadAll call, exercising its *http.MaxBytesError
226-
// handling.
218+
// An unknown length skips WithMaxBodySize's Content-Length fast path,
219+
// so the overflow surfaces from WithMCPParse's own read.
227220
req := httptest.NewRequest(http.MethodPost, "/mcp", unknownLengthBody(body))
228221
require.Equal(t, int64(-1), req.ContentLength, "test setup: Content-Length should be unknown")
229222

pkg/http/middleware/scope_challenge_test.go

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,8 @@ import (
1414
"github.com/stretchr/testify/require"
1515
)
1616

17-
// TestWithScopeChallenge_MaxBodySize verifies the fallback body-parsing path
18-
// (used when WithMCPParse has not already populated MCPMethodInfo in
19-
// context) respects the request-body size limit and returns a clear 413
20-
// instead of silently continuing, when composed with WithMaxBodySize as it
21-
// is in production.
17+
// TestWithScopeChallenge_MaxBodySize covers the fallback body-parsing path,
18+
// used when WithMCPParse has not already populated MCPMethodInfo in context.
2219
func TestWithScopeChallenge_MaxBodySize(t *testing.T) {
2320
const limit = 64
2421
oauthCfg := &oauth.Config{}
@@ -48,11 +45,8 @@ func TestWithScopeChallenge_MaxBodySize(t *testing.T) {
4845

4946
handler := WithMaxBodySize(limit)(WithScopeChallenge(oauthCfg, fetcher)(next))
5047

51-
// Use an unknown-length body so WithMaxBodySize can't reject the
52-
// request via its known-Content-Length fast path (already covered by
53-
// body_limit_test.go). This forces the request through to
54-
// WithScopeChallenge's fallback io.ReadAll call, exercising its
55-
// *http.MaxBytesError handling.
48+
// An unknown length skips WithMaxBodySize's Content-Length fast path,
49+
// so the overflow surfaces from the fallback read.
5650
req := newRequestWithBody(unknownLengthBody(body))
5751
require.Equal(t, int64(-1), req.ContentLength, "test setup: Content-Length should be unknown")
5852

pkg/http/server.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,7 @@ type ServerConfig struct {
109109
MRTRStateKey string
110110

111111
// MaxRequestBodyBytes bounds the size of HTTP request bodies accepted by
112-
// the MCP endpoints, enforced before any middleware reads or buffers the
113-
// body. When zero, middleware.DefaultMaxRequestBodyBytes is used.
112+
// the MCP endpoints. When zero, middleware.DefaultMaxRequestBodyBytes is used.
114113
MaxRequestBodyBytes int64
115114

116115
disableDeleteRepository bool

0 commit comments

Comments
 (0)