From 3f0c48fdbbca74d46671a0cbf699041f04cee60c Mon Sep 17 00:00:00 2001 From: Jake Wang Date: Thu, 17 Sep 2026 16:19:50 -0400 Subject: [PATCH] fix(middleware): preserve request bodies beyond the dump limit --- middleware/body_dump.go | 15 ++++++----- middleware/body_dump_test.go | 52 ++++++++++++++++++++++++++++++++++-- 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/middleware/body_dump.go b/middleware/body_dump.go index 0443a67ab..ad575ce62 100644 --- a/middleware/body_dump.go +++ b/middleware/body_dump.go @@ -27,6 +27,7 @@ type BodyDumpConfig struct { // MaxRequestBytes limits how much of the request body to dump. // If the request body exceeds this limit, only the first MaxRequestBytes // are dumped. The handler callback receives truncated data. + // The next handler still receives the full request body. // Default: 5 * MB (5,242,880 bytes) // Set to -1 to disable limits (not recommended in production). MaxRequestBytes int64 @@ -102,15 +103,15 @@ func (config BodyDumpConfig) ToMiddleware() (echo.MiddlewareFunc, error) { if readErr != nil && readErr != io.EOF { return readErr } - if config.MaxRequestBytes > 0 { - // Drain any remaining body data to prevent connection issues - _, _ = io.Copy(io.Discard, c.Request().Body) - _ = c.Request().Body.Close() - } - reqBody := make([]byte, reqBuf.Len()) copy(reqBody, reqBuf.Bytes()) - c.Request().Body = io.NopCloser(bytes.NewReader(reqBody)) + c.Request().Body = struct { + io.Reader + io.Closer + }{ + Reader: io.MultiReader(bytes.NewReader(reqBody), c.Request().Body), + Closer: c.Request().Body, + } // response part resBuf := bodyDumpBufferPool.Get().(*bytes.Buffer) diff --git a/middleware/body_dump_test.go b/middleware/body_dump_test.go index e5f64541a..2b7ea7c10 100644 --- a/middleware/body_dump_test.go +++ b/middleware/body_dump_test.go @@ -5,11 +5,13 @@ package middleware import ( "errors" + "fmt" "io" "net/http" "net/http/httptest" "strings" "testing" + "testing/iotest" "github.com/labstack/echo/v5" "github.com/stretchr/testify/assert" @@ -280,8 +282,8 @@ func TestBodyDump_RequestExceedsLimit(t *testing.T) { assert.NoError(t, err) assert.Equal(t, int(limit), len(requestBodyDumped), "Dumped request should be truncated to limit") assert.Equal(t, strings.Repeat("A", 1024), requestBodyDumped, "Dumped data should match first N bytes") - // Handler should receive truncated data (what was dumped) - assert.Equal(t, strings.Repeat("A", 1024), rec.Body.String()) + // Dump limits must not truncate the request passed to the handler. + assert.Equal(t, largeData, rec.Body.String()) } func TestBodyDump_RequestAtExactLimit(t *testing.T) { @@ -579,3 +581,49 @@ func BenchmarkBodyDump_BufferPooling(b *testing.B) { mw(h)(c) } } + +func TestBodyDump_RequestRemainder(t *testing.T) { + for _, readError := range []error{nil, errors.New("read failed")} { + t.Run(fmt.Sprint(readError), func(t *testing.T) { + const payload = "abcdef" + const limit = 3 + reader := io.Reader(strings.NewReader(payload)) + if readError != nil { + reader = io.MultiReader(reader, iotest.ErrReader(readError)) + } + body := &bodyDumpTrackingReadCloser{Reader: reader} + req := httptest.NewRequest(http.MethodPost, "/", nil) + req.Body = body + e := echo.New() + c := e.NewContext(req, httptest.NewRecorder()) + var dumped []byte + mw := BodyDumpWithConfig(BodyDumpConfig{ + MaxRequestBytes: limit, + Handler: func(_ *echo.Context, reqBody, _ []byte, _ error) { + dumped = reqBody + }, + }) + err := mw(func(c *echo.Context) error { + assert.False(t, body.closed, "request body must remain open for the handler") + data, err := io.ReadAll(c.Request().Body) + assert.Equal(t, payload, string(data)) + assert.Equal(t, readError, err) + assert.NoError(t, c.Request().Body.Close()) + assert.True(t, body.closed, "Close must reach the original request body") + return nil + })(c) + assert.NoError(t, err) + assert.Equal(t, payload[:limit], string(dumped)) + }) + } +} + +type bodyDumpTrackingReadCloser struct { + io.Reader + closed bool +} + +func (r *bodyDumpTrackingReadCloser) Close() error { + r.closed = true + return nil +}