diff --git a/middleware/body_limit.go b/middleware/body_limit.go index 4f1963e18..917eeb8f4 100644 --- a/middleware/body_limit.go +++ b/middleware/body_limit.go @@ -24,6 +24,7 @@ type limitedReader struct { BodyLimitConfig reader io.ReadCloser read int64 + err error } // BodyLimit returns a BodyLimit middleware. @@ -81,10 +82,27 @@ func (config BodyLimitConfig) ToMiddleware() (echo.MiddlewareFunc, error) { } func (r *limitedReader) Read(b []byte) (n int, err error) { + // Once the limit has been exceeded, make it sticky — always return 0 bytes and the error. + // This prevents callers (like encoding/json.Decoder, which processes n>0 before checking err) + // from reading arbitrarily far past the limit by repeatedly calling Read. + if r.err != nil { + return 0, r.err + } + + // Bound the read size so a single Read call can never return enough bytes to both + // cross the limit and complete a value (e.g. a JSON object). + // We allow one byte past the limit so the caller gets the error alongside the data + // that caused the overflow, which matches standard io.Reader semantics. + remaining := r.LimitBytes - r.read + if remaining < int64(len(b))-1 { + b = b[:remaining+1] + } + n, err = r.reader.Read(b) r.read += int64(n) if r.read > r.LimitBytes { - return n, echo.ErrStatusRequestEntityTooLarge + r.err = echo.ErrStatusRequestEntityTooLarge + return n, r.err } return } @@ -96,4 +114,5 @@ func (r *limitedReader) Close() error { func (r *limitedReader) Reset(reader io.ReadCloser) { r.reader = reader r.read = 0 + r.err = nil } diff --git a/middleware/body_limit_test.go b/middleware/body_limit_test.go index 68d904da8..60cca0453 100644 --- a/middleware/body_limit_test.go +++ b/middleware/body_limit_test.go @@ -189,3 +189,108 @@ func TestBodyLimit(t *testing.T) { assert.Equal(t, http.StatusOK, rec.Code) assert.Equal(t, hw, rec.Body.Bytes()) } + +func TestBodyLimitReader_stickyError(t *testing.T) { + // Verifies that once the limit is exceeded, subsequent Read calls always + // return (0, err) — no more data is leaked to the caller. + // This is the fix for issue #3071 where callers following the standard + // io.Reader pattern (process n>0 before checking err) could read arbitrarily + // far past the limit. + hw := []byte("Hello, World!") + + config := BodyLimitConfig{ + Skipper: DefaultSkipper, + LimitBytes: 5, + } + reader := &limitedReader{ + BodyLimitConfig: config, + reader: io.NopCloser(bytes.NewReader(hw)), + } + + buf := make([]byte, 1) + + // Read 5 bytes one at a time — should all succeed with no error + for i := 0; i < 5; i++ { + n, err := reader.Read(buf) + assert.Equal(t, 1, n) + assert.NoError(t, err) + } + + // 6th read should cross the limit and return the error alongside 1 byte + n, err := reader.Read(buf) + assert.Equal(t, 1, n) + he := err.(echo.HTTPStatusCoder) + assert.Equal(t, http.StatusRequestEntityTooLarge, he.StatusCode()) + + // All subsequent reads must return (0, err) — never more data + for i := 0; i < 10; i++ { + n, err = reader.Read(buf) + assert.Equal(t, 0, n, "read after limit exceeded must return 0 bytes") + he = err.(echo.HTTPStatusCoder) + assert.Equal(t, http.StatusRequestEntityTooLarge, he.StatusCode()) + } +} + +func TestBodyLimitReader_boundedRead(t *testing.T) { + // Verifies that a single large Read buffer can never return enough bytes to + // both cross the limit and complete a value — only remaining+1 bytes max. + // This prevents a single oversized Read from silently bypassing the limit. + hw := bytes.Repeat([]byte("x"), 100) + + config := BodyLimitConfig{ + Skipper: DefaultSkipper, + LimitBytes: 10, + } + reader := &limitedReader{ + BodyLimitConfig: config, + reader: io.NopCloser(bytes.NewReader(hw)), + } + + // A large buffer read should return at most remaining+1 = 11 bytes, not the full 100 + buf := make([]byte, 100) + n, err := reader.Read(buf) + assert.LessOrEqual(t, n, 11, "single read must not return more than remaining+1 bytes") + he := err.(echo.HTTPStatusCoder) + assert.Equal(t, http.StatusRequestEntityTooLarge, he.StatusCode()) +} + +func TestBodyLimit_readAfterLimitNoLeak(t *testing.T) { + // Integration test: verifies that BodyLimit middleware correctly enforces the + // limit even when the handler reads with a large buffer and processes n>0 + // bytes before checking err (standard io.Reader semantics). + e := echo.New() + + const limit = 5 + body := bytes.Repeat([]byte("x"), 10*limit) // 50 bytes, well over the limit + + req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(body)) + // Force chunked encoding to bypass the Content-Length fast path + req.ContentLength = -1 + req.TransferEncoding = []string{"chunked"} + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + mw := BodyLimit(limit) + h := func(c *echo.Context) error { + // Simulate a handler that follows io.Reader semantics exactly: + // process n>0 bytes before considering err fatal. + // (This is exactly what encoding/json.Decoder does.) + buf := make([]byte, 64) + var total int + for { + n, err := c.Request().Body.Read(buf) + total += n + if n == 0 { + break + } + _ = err // process data first, per io.Reader docs + } + // The handler must never be able to read more than limit+1 bytes total. + assert.LessOrEqual(t, total, limit+1, "handler must not read past limit+1") + return nil + } + + err := mw(h)(c) + he := err.(echo.HTTPStatusCoder) + assert.Equal(t, http.StatusRequestEntityTooLarge, he.StatusCode()) +}