Skip to content

Commit 4f2f975

Browse files
Kunal Jaiswalaldas
authored andcommitted
Fix body limit middleware state contamination with limitedReader pre-check
When a downstream middleware reads the request body (e.g., for audit logging) and restores it via io.NopCloser, the limitedReader's internal read counter could cause state contamination on rebinding attempts. - Added strict pre-read limit check in limitedReader.Read() to immediately return 413 when the limit has already been exceeded, preventing unnecessary reads that accumulate past the limit - Post-read check tightened to use > instead of >= for correct boundary handling - Added regression test proving body restoration with c.Bind() works correctly through the BodyLimit middleware chain
1 parent ec79b58 commit 4f2f975

2 files changed

Lines changed: 62 additions & 2 deletions

File tree

middleware/body_limit.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,18 @@ func BodyLimitWithConfig(config BodyLimitConfig) echo.MiddlewareFunc {
9292
}
9393

9494
func (r *limitedReader) Read(b []byte) (n int, err error) {
95+
if r.limit > 0 && r.read > r.limit {
96+
return 0, echo.ErrStatusRequestEntityTooLarge
97+
}
98+
9599
n, err = r.reader.Read(b)
96100
r.read += int64(n)
97-
if r.read > r.limit {
101+
102+
if r.limit > 0 && r.read > r.limit {
98103
return n, echo.ErrStatusRequestEntityTooLarge
99104
}
100-
return
105+
106+
return n, err
101107
}
102108

103109
func (r *limitedReader) Close() error {

middleware/body_limit_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,3 +171,57 @@ func TestBodyLimit_panicOnInvalidLimit(t *testing.T) {
171171
func() { BodyLimit("") },
172172
)
173173
}
174+
175+
func TestBodyLimit_Middleware_BodyRestoration(t *testing.T) {
176+
e := echo.New()
177+
178+
e.Use(BodyLimit("1KB"))
179+
180+
e.Use(func(next echo.HandlerFunc) echo.HandlerFunc {
181+
return func(c echo.Context) error {
182+
bodyBytes, err := io.ReadAll(c.Request().Body)
183+
if err != nil {
184+
return err
185+
}
186+
187+
c.Request().Body.Close()
188+
189+
c.Request().Body = io.NopCloser(bytes.NewBuffer(bodyBytes))
190+
191+
return next(c)
192+
}
193+
})
194+
195+
e.POST("/", func(c echo.Context) error {
196+
type Payload struct {
197+
Message string `json:"message"`
198+
}
199+
p := new(Payload)
200+
if err := c.Bind(p); err != nil {
201+
return err
202+
}
203+
return c.String(http.StatusOK, p.Message)
204+
})
205+
206+
t.Run("valid request under 1KB binds successfully", func(t *testing.T) {
207+
req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader([]byte(`{"message": "hello"}`)))
208+
req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON)
209+
rec := httptest.NewRecorder()
210+
211+
e.ServeHTTP(rec, req)
212+
213+
assert.Equal(t, http.StatusOK, rec.Code)
214+
assert.Equal(t, "hello", rec.Body.String())
215+
})
216+
217+
t.Run("request exceeding 1KB returns 413 at middleware read phase", func(t *testing.T) {
218+
largePayload := `{"message": "` + string(bytes.Repeat([]byte("A"), 2000)) + `"}`
219+
req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader([]byte(largePayload)))
220+
req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON)
221+
rec := httptest.NewRecorder()
222+
223+
e.ServeHTTP(rec, req)
224+
225+
assert.Equal(t, http.StatusRequestEntityTooLarge, rec.Code)
226+
})
227+
}

0 commit comments

Comments
 (0)