From fb34af025ce6f344cea21384d5a7b3099096fb76 Mon Sep 17 00:00:00 2001 From: Aditya <205600203+Rohilalala@users.noreply.github.com> Date: Tue, 25 Aug 2026 14:49:50 +0530 Subject: [PATCH] fix(middleware): stop BodyLimit handing out more than the limit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit limitedReader.Read passed the caller's buffer to the source untouched and only looked at the running total afterwards, and the refusal did not stick. io.Reader asks callers to process the n>0 bytes of a read before treating its error as fatal, so a caller following that advice — encoding/json's Decoder among them — kept getting real data on every call after the limit had already been passed. With a 5 byte limit and a 50 byte body, 50 bytes came through. The read is now capped at one byte past the limit, which is all it takes to know the body is too large; that byte is not handed to the caller; and once the limit is passed the reader stays refused without touching the source again. Fixes #3071 --- middleware/body_limit.go | 21 +++++++++ middleware/body_limit_test.go | 83 +++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+) diff --git a/middleware/body_limit.go b/middleware/body_limit.go index 4f1963e18..08fef7f56 100644 --- a/middleware/body_limit.go +++ b/middleware/body_limit.go @@ -81,9 +81,30 @@ func (config BodyLimitConfig) ToMiddleware() (echo.MiddlewareFunc, error) { } func (r *limitedReader) Read(b []byte) (n int, err error) { + // Once the limit is known to be exceeded, stay refused. io.Reader's + // contract invites callers to process the n>0 bytes of a failed read and + // carry on, so a reader that keeps serving data after the first refusal + // hands out the whole body to anyone following that advice. + if r.read > r.LimitBytes { + return 0, echo.ErrStatusRequestEntityTooLarge + } + + // Never read further than one byte past the limit: that byte is what + // proves the body is too large, and anything beyond it is data the caller + // asked for but is not allowed to have. + if max := r.LimitBytes - r.read + 1; int64(len(b)) > max { + b = b[:max] + } + n, err = r.reader.Read(b) r.read += int64(n) if r.read > r.LimitBytes { + // Hand back only what fits. The byte past the limit was read to prove + // the body is too large, not to be delivered, and a caller that + // processes n>0 before the error must not receive it. + if over := int(r.read - r.LimitBytes); over <= n { + n -= over + } return n, echo.ErrStatusRequestEntityTooLarge } return diff --git a/middleware/body_limit_test.go b/middleware/body_limit_test.go index 68d904da8..fe24333e5 100644 --- a/middleware/body_limit_test.go +++ b/middleware/body_limit_test.go @@ -93,6 +93,89 @@ func TestBodyLimitAfterDecompressUsesDecodedSize(t *testing.T) { assert.Equal(t, body, rec.Body.String()) } +func TestBodyLimitReaderStopsAtTheLimit(t *testing.T) { + const limit = 5 + body := bytes.Repeat([]byte("x"), 10*limit) + + reader := &limitedReader{ + BodyLimitConfig: BodyLimitConfig{Skipper: DefaultSkipper, LimitBytes: limit}, + reader: io.NopCloser(bytes.NewReader(body)), + } + + // io.Reader asks callers to process the n>0 bytes of a read before + // treating its error as fatal, so keep reading the way such a caller + // would. No more than the limit may be handed over however long it goes on. + buf := make([]byte, 64) + var total int + for range 20 { + n, err := reader.Read(buf) + total += n + if n == 0 && err != nil { + break + } + } + + assert.Equal(t, limit, total) +} + +// countingReader records how much was asked of the source, which is not +// visible from what the caller receives. +type countingReader struct { + io.Reader + read int64 + calls int +} + +func (c *countingReader) Read(b []byte) (int, error) { + c.calls++ + n, err := c.Reader.Read(b) + c.read += int64(n) + return n, err +} + +func (c *countingReader) Close() error { return nil } + +func TestBodyLimitReaderDoesNotOverdrawTheSource(t *testing.T) { + const limit = 5 + src := &countingReader{Reader: bytes.NewReader(bytes.Repeat([]byte("x"), 1<<20))} + + reader := &limitedReader{ + BodyLimitConfig: BodyLimitConfig{Skipper: DefaultSkipper, LimitBytes: limit}, + reader: src, + } + + buf := make([]byte, 64*1024) + _, _ = reader.Read(buf) + callsAtRefusal := src.calls + + // One byte past the limit is enough to know the body is too large; a + // megabyte of it should never be pulled off the wire to find that out. + assert.LessOrEqual(t, src.read, int64(limit+1)) + + // And once refused, the source must not be touched again. + for range 5 { + _, _ = reader.Read(buf) + } + assert.Equal(t, callsAtRefusal, src.calls) +} + +func TestBodyLimitReaderStaysRefused(t *testing.T) { + reader := &limitedReader{ + BodyLimitConfig: BodyLimitConfig{Skipper: DefaultSkipper, LimitBytes: 2}, + reader: io.NopCloser(bytes.NewReader([]byte("Hello, World!"))), + } + + _, err := io.ReadAll(reader) + he := err.(echo.HTTPStatusCoder) + assert.Equal(t, http.StatusRequestEntityTooLarge, he.StatusCode()) + + // Reading on after the refusal must not produce more of the body. + n, err := reader.Read(make([]byte, 8)) + assert.Equal(t, 0, n) + he = err.(echo.HTTPStatusCoder) + assert.Equal(t, http.StatusRequestEntityTooLarge, he.StatusCode()) +} + func TestBodyLimitReader(t *testing.T) { hw := []byte("Hello, World!")