`middleware.BodyDump`: data race — error response is written into a dump buffer already returned to `sync.Pool`
Maintainer antworten meist innerhalb von 1 Tag
Bewertung
- Schwierigkeit
- 3/5
- Geschätzter Aufwand
- Ein halber Tag
- Anfängerfreundlichkeit
- 62/100
Rechercherichtung
Start in middleware/body_dump.go, where the response writer is installed with c.SetResponse and the dump buffer is returned to bodyDumpBufferPool, then check echo.go around serveHTTP to see when HTTPErrorHandler runs after the chain returns. Add a regression test based on the reproduction in the issue and run it with go test -race. Done means the race is gone, the writer is restored before BodyDump returns, and the error response body is unchanged.
Vom Indexierungsmodell aus dem Issue-Text verfasst.
Beschreibung
Echo version: github.com/labstack/echo/v5 v5.4.0
Go version: 1.25
Summary
BodyDump installs a response-writer wrapper on the Context via c.SetResponse and never restores it. It also returns its dump buffer to bodyDumpBufferPool when the middleware returns.
Because Echo.serveHTTP invokes the global HTTPErrorHandler after the middleware chain has returned, the error response is written through the still-installed wrapper — landing in a *bytes.Buffer that has already been handed back to the pool.
Root cause
middleware/body_dump.go:
resBuf := bodyDumpBufferPool.Get().(*bytes.Buffer) // L116
resBuf.Reset()
defer bodyDumpBufferPool.Put(resBuf) // L118 <- returned to pool
...
writer := &bodyDumpResponseWriter{...}
c.SetResponse(writer) // L134 <- never restored
err := next(c) // L136
config.Handler(c, reqBody, resBuf.Bytes(), err) // L139
return err // L141 <- deferred Put runs here
echo.go:
err = e.preChain(c) // L841
...
if err != nil {
e.HTTPErrorHandler(c, err) // L845 <- runs after the chain returned
}
So the ordering is: buffer returned to pool → error handler writes JSON → limitedWriter.Write (L193) → dumpBuf.Write on the pooled buffer.
Reproduction
package middleware_test
import (
"bytes"
"net/http"
"net/http/httptest"
"sync"
"testing"
"github.com/labstack/echo/v5"
"github.com/labstack/echo/v5/middleware"
)
func TestBodyDumpErrorHandlerWritesToReturnedBuffer(t *testing.T) {
e := echo.New()
e.HTTPErrorHandler = func(c *echo.Context, err error) {
_ = c.JSON(http.StatusOK, map[string]any{"error": err.Error()})
}
e.Pre(middleware.BodyDump(func(c *echo.Context, reqBody, rspBody []byte, err error) {}))
e.POST("/err", func(c *echo.Context) error {
return echo.NewHTTPError(http.StatusBadRequest, "boom")
})
var wg sync.WaitGroup
for i := 0; i < 16; i++ {
wg.Add(1)
go func() {
defer wg.Done()
for j := 0; j < 500; j++ {
req := httptest.NewRequest(http.MethodPost, "/err", bytes.NewReader(make([]byte, 256)))
e.ServeHTTP(httptest.NewRecorder(), req)
}
}()
}
wg.Wait()
}
$ go test -race -run TestBodyDumpErrorHandlerWritesToReturnedBuffer ./middleware/
WARNING: DATA RACE
Write at 0x00c00028e0e0 by goroutine 11:
bytes.(*Buffer).Write()
middleware.(*limitedWriter).Write()
middleware/body_dump.go:193
middleware.(*bodyDumpResponseWriter).Write()
middleware/body_dump.go:151
echo.(*delayedStatusWriter).Write()
response.go:157
encoding/json.(*Encoder).Encode()
echo.(*Context).JSON()
main.TestBodyDumpErrorHandlerWritesToReturnedBuffer.func1()
echo.(*Echo).serveHTTP()
echo.go:845
The conflicting access is another request's reqBuf.Reset() at body_dump.go:94. Reproduced on every run (35–43 races per run with 16 goroutines × 500 requests).
Note this needs e.Pre(...) or e.Use(...) — either works, since HTTPErrorHandler is always called after the chain returns.
Impact
- Genuine data race on
bytes.Bufferinternals — undefined behaviour per the Go memory model, flagged by-race. - The pool is shared by
reqBuf(L93) andresBuf(L116), so the stale write can land in another request'sreqBuf. BodyDump rebuildsc.Request().Bodyfrom that buffer (L111-113), so a concurrent request can have its request body corrupted before it reaches the handler — not just polluted dump output. - Client-visible responses are not affected:
limitedWriter.Write(L184) writes to the real writer before mirroring into the dump buffer. - Intermittent by nature:
sync.Poolmay drop the object at GC, so it can go unnoticed in short runs.
Suggested fix
Restore the previous response writer before the middleware returns, so that writes occurring after the chain has unwound do not go through the wrapper:
prev := c.Response()
c.SetResponse(writer)
defer c.SetResponse(prev)
Either defer ordering works — the requirement is only that c.Response() is back to the previous writer by the time BodyDump returns.
Workaround
Wrap BodyDump and restore the writer after next returns:
inner := middleware.BodyDumpWithConfig(cfg)
return func(c *echo.Context) error {
orig := c.Response()
err := inner(next)(c)
c.SetResponse(orig)
return err
}
Or restore it in the global error handler:
if r, uerr := echo.UnwrapResponse(c.Response()); uerr == nil && r != nil {
c.SetResponse(r)
}
Both verified to eliminate the race (-race clean, error response body unchanged).
- Vorherrschende Sprache
- Go
- Sterne
- 32.8k
- Forks
- 5.1k
- Ø Merge
- 15 Std. 18 Min.
- Gemergte PRs (30 T.)
- 31
Entwicklungsumgebung
Dieses Projekt bietet weder Dev-Container noch Dockerfile noch Beitragsleitfaden – die Einrichtung liegt bei Ihnen. Beginnen Sie mit der README; die allgemeinen Schritte stehen in unserem Leitfaden für den ersten Beitrag.
Erste Schritte
- Lesen Sie das ganze Issue und danach den Beitragsleitfaden des Projekts.
- Schreiben Sie ins Issue, dass Sie es übernehmen — das erspart doppelte Arbeit.
- Forken Sie das Repository und arbeiten Sie in einem Branch.
- Öffnen Sie einen Pull Request, der die Issue-Nummer nennt.
Mehr aus labstack/echo
-
ExtractIPFromXFFHeader ignores X-Forwarded-For when the peer address has an IPv6 zoneEvtl. vergeben @TheNetherWatcher hat das heute übernommen. Offen
Schwierigkeit 2/5 1-3 Stunden Anfängerfreundlichkeit 82/100
Maintainer antworten meist innerhalb von 1 Tag
-
BindHeaders panics on nil or empty header value slicesEvtl. vergeben @SashaMIT hat das vor 6 Tagen übernommen. Offen
Schwierigkeit 2/5 1-3 Stunden Anfängerfreundlichkeit 73/100
Maintainer antworten meist innerhalb von 1 Tag
-
Group routes does not work without leading slashEvtl. vergeben @team-humaki hat das vor 26 Tagen übernommen. Offenenhancement router v5
Schwierigkeit 2/5 1-3 Stunden Anfängerfreundlichkeit 74/100
labstack/echo#3099 · 2 Kommentare ·
Maintainer antworten meist innerhalb von 1 Tag
-
enhancement v5
Schwierigkeit 5/5 Über eine Woche Anfängerfreundlichkeit 35/100
labstack/echo#3066 · 1 Kommentar ·
Maintainer antworten meist innerhalb von 1 Tag
-
Rate limit middleware doesn't set headers or provide metadata via interface.Evtl. vergeben @AdamMagued hat das vor 8 Tagen übernommen. Offenenhancement middleware
Schwierigkeit 4/5 3-5 Tage Anfängerfreundlichkeit 48/100
labstack/echo#2961 · 2 Kommentare ·
Maintainer antworten meist innerhalb von 1 Tag
Ähnliche Issues
-
Schwierigkeit 2/5 1-3 Stunden Anfängerfreundlichkeit 81/100
apache/rocketmq-dashboard#6895 ·
Maintainer antworten meist innerhalb von 3 Tagen
-
agent-butler-finding chore
Schwierigkeit 1/5 Unter einer Stunde Anfängerfreundlichkeit 88/100
jordansmall/spindrift#5466 ·
Maintainer antworten meist innerhalb von 1 Tag
-
Schwierigkeit 1/5 Unter einer Stunde Anfängerfreundlichkeit 87/100
777genius/agent-notifications#490 ·
Maintainer antworten meist innerhalb von 1 Tag
-
bug
Schwierigkeit 2/5 1-3 Stunden Anfängerfreundlichkeit 78/100
devantler-tech/ksail#7836 ·
Maintainer antworten meist innerhalb von 1 Tag
-
Schwierigkeit 2/5 1-3 Stunden Anfängerfreundlichkeit 76/100
keyxmakerx/Chronicle#1469 ·
Maintainer antworten meist innerhalb von 1 Tag