Set the request-body limit to 5 MiB at both layers
Bounds the total HTTP request, so allow modest headroom over the MCP SDK's 4 MiB default for JSON-RPC and tool-call envelope overhead rather than spending the whole budget on tool content. Because the limit now exceeds the SDK default, passing it to StreamableHTTPOptions is load-bearing: without it the SDK would cap requests at 4 MiB and the headroom would not exist. Covered by a test that sends a request between the two limits. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
+2
-2
@@ -250,8 +250,8 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
||||
return ghServer
|
||||
}, &mcp.StreamableHTTPOptions{
|
||||
Stateless: true,
|
||||
// Keep the SDK's own guard in step with the middleware, otherwise its
|
||||
// default would silently cap a larger configured limit.
|
||||
// Required, not just belt-and-braces: the effective limit exceeds the
|
||||
// SDK's own default, which would otherwise cap it.
|
||||
MaxRequestBodyBytes: h.maxRequestBodyBytes(),
|
||||
})
|
||||
|
||||
|
||||
@@ -15,6 +15,7 @@ import (
|
||||
ghcontext "github.com/github/github-mcp-server/pkg/context"
|
||||
"github.com/github/github-mcp-server/pkg/github"
|
||||
"github.com/github/github-mcp-server/pkg/http/headers"
|
||||
"github.com/github/github-mcp-server/pkg/http/middleware"
|
||||
"github.com/github/github-mcp-server/pkg/inventory"
|
||||
"github.com/github/github-mcp-server/pkg/scopes"
|
||||
"github.com/github/github-mcp-server/pkg/translations"
|
||||
@@ -1289,12 +1290,16 @@ func TestUIMetaStrippedWhenClientLacksCapability(t *testing.T) {
|
||||
require.NotNil(t, unknown[0].Tool.Meta["ui"], "_meta.ui should be preserved when capability is unknown and FF is on")
|
||||
}
|
||||
|
||||
// TestMaxRequestBodyBytes checks the effective limit tracks the MCP SDK
|
||||
// default and honours an operator override.
|
||||
// TestMaxRequestBodyBytes checks the effective limit and, critically, that it
|
||||
// sits above the MCP SDK default — which is why it must also be passed to
|
||||
// StreamableHTTPOptions rather than left to the SDK.
|
||||
func TestMaxRequestBodyBytes(t *testing.T) {
|
||||
t.Run("defaults to the MCP SDK limit", func(t *testing.T) {
|
||||
t.Run("default leaves headroom above the MCP SDK limit", func(t *testing.T) {
|
||||
h := &Handler{config: &ServerConfig{}}
|
||||
assert.Equal(t, int64(mcp.DefaultMaxRequestBodyBytes), h.maxRequestBodyBytes())
|
||||
|
||||
assert.Equal(t, int64(5<<20), h.maxRequestBodyBytes())
|
||||
assert.Greater(t, h.maxRequestBodyBytes(), int64(mcp.DefaultMaxRequestBodyBytes),
|
||||
"the default intentionally exceeds the SDK limit, so the SDK must be told about it")
|
||||
})
|
||||
|
||||
t.Run("configured value overrides the default", func(t *testing.T) {
|
||||
@@ -1320,11 +1325,11 @@ func TestMaxRequestBodySizeEnforcement(t *testing.T) {
|
||||
return strings.Replace(payload, "PADDING", "PADDING"+pad, 1)
|
||||
}
|
||||
|
||||
newHandler := func(t *testing.T, mcpServerFactoryCalled *bool) *Handler {
|
||||
newHandler := func(t *testing.T, maxBytes int64, mcpServerFactoryCalled *bool) *Handler {
|
||||
t.Helper()
|
||||
return NewHTTPMcpHandler(
|
||||
context.Background(),
|
||||
&ServerConfig{Version: "test", MaxRequestBodyBytes: limit},
|
||||
&ServerConfig{Version: "test", MaxRequestBodyBytes: maxBytes},
|
||||
nil,
|
||||
translations.NullTranslationHelper,
|
||||
slog.Default(),
|
||||
@@ -1359,7 +1364,7 @@ func TestMaxRequestBodySizeEnforcement(t *testing.T) {
|
||||
|
||||
t.Run("middleware rejects an oversized request before the MCP server is built", func(t *testing.T) {
|
||||
var mcpServerFactoryCalled bool
|
||||
r := newRouter(newHandler(t, &mcpServerFactoryCalled))
|
||||
r := newRouter(newHandler(t, limit, &mcpServerFactoryCalled))
|
||||
|
||||
body := buildBody(limit + 1)
|
||||
require.Greater(t, len(body), limit)
|
||||
@@ -1374,7 +1379,7 @@ func TestMaxRequestBodySizeEnforcement(t *testing.T) {
|
||||
|
||||
t.Run("request at the configured limit succeeds", func(t *testing.T) {
|
||||
var mcpServerFactoryCalled bool
|
||||
r := newRouter(newHandler(t, &mcpServerFactoryCalled))
|
||||
r := newRouter(newHandler(t, limit, &mcpServerFactoryCalled))
|
||||
|
||||
body := buildBody(limit)
|
||||
require.Len(t, body, limit)
|
||||
@@ -1387,7 +1392,7 @@ func TestMaxRequestBodySizeEnforcement(t *testing.T) {
|
||||
})
|
||||
|
||||
t.Run("SDK handler enforces the configured limit when the middleware is bypassed", func(t *testing.T) {
|
||||
h := newHandler(t, nil)
|
||||
h := newHandler(t, limit, nil)
|
||||
|
||||
body := buildBody(limit + 1)
|
||||
require.Greater(t, len(body), limit)
|
||||
@@ -1399,4 +1404,21 @@ func TestMaxRequestBodySizeEnforcement(t *testing.T) {
|
||||
assert.Contains(t, rr.Body.String(), fmt.Sprintf("request body exceeds %d bytes", limit),
|
||||
"the SDK should report the configured limit, not its own default")
|
||||
})
|
||||
|
||||
// The default headroom only exists if it reaches the SDK as well; leaving
|
||||
// the SDK on its own default would silently reject this request.
|
||||
t.Run("unconfigured handler accepts a request above the MCP SDK limit", func(t *testing.T) {
|
||||
var mcpServerFactoryCalled bool
|
||||
r := newRouter(newHandler(t, 0, &mcpServerFactoryCalled))
|
||||
|
||||
body := buildBody(mcp.DefaultMaxRequestBodyBytes + 1024)
|
||||
require.Greater(t, int64(len(body)), int64(mcp.DefaultMaxRequestBodyBytes))
|
||||
require.Less(t, int64(len(body)), middleware.DefaultMaxRequestBodyBytes)
|
||||
|
||||
rr := httptest.NewRecorder()
|
||||
r.ServeHTTP(rr, newRequest(body))
|
||||
|
||||
assert.Equal(t, http.StatusOK, rr.Code, "response body: %s", rr.Body.String())
|
||||
assert.True(t, mcpServerFactoryCalled, "the MCP server should be constructed for an allowed request")
|
||||
})
|
||||
}
|
||||
|
||||
@@ -3,13 +3,13 @@ package middleware
|
||||
import (
|
||||
"errors"
|
||||
"net/http"
|
||||
|
||||
"github.com/modelcontextprotocol/go-sdk/mcp"
|
||||
)
|
||||
|
||||
// DefaultMaxRequestBodyBytes tracks the MCP SDK's own request-body limit, so
|
||||
// enforcing it earlier in the chain does not change which requests are accepted.
|
||||
const DefaultMaxRequestBodyBytes int64 = mcp.DefaultMaxRequestBodyBytes
|
||||
// DefaultMaxRequestBodyBytes bounds the total HTTP request, not just the tool
|
||||
// payload within it. It sits modestly above the MCP SDK's own default to leave
|
||||
// room for JSON-RPC and tool-call envelope overhead; because it is the larger
|
||||
// of the two, callers must also pass it to the SDK or the SDK would cap it.
|
||||
const DefaultMaxRequestBodyBytes int64 = 5 << 20 // 5 MiB
|
||||
|
||||
// WithMaxBodySize bounds the size of the request body. It must be registered
|
||||
// before any middleware that reads or buffers the body (WithMCPParse,
|
||||
|
||||
Reference in New Issue
Block a user