From 546ded1d9c35ae43bff90c80a655a795d4ab0cf4 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Thu, 13 Aug 2026 03:10:21 +0100 Subject: [PATCH] fix(pools): return pooled gzip reader on Reset error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GetGzipReader got a reader from the pool, Reset it, and on error returned a fresh gzip.NewReader while never returning the pooled reader to the pool — one leaked reader per reset error (non-gzip input), silently draining the pool. Return the pooled reader without Close (Close panics because the decompressor is nil until a header parses); the next caller's Reset reinitialises it. Test: TestGetGzipReader_ResetErrorPath (error path leaves the pool usable for subsequent valid gzip reads). --- pkg/pools/buffer.go | 7 ++++++- pkg/pools/buffer_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/pkg/pools/buffer.go b/pkg/pools/buffer.go index 49b1b4f..db39b85 100644 --- a/pkg/pools/buffer.go +++ b/pkg/pools/buffer.go @@ -78,7 +78,12 @@ func PutGzipWriter(gz *gzip.Writer) { func GetGzipReader(r io.Reader) (*gzip.Reader, error) { gr := gzipReaderPool.Get().(*gzip.Reader) if err := gr.Reset(r); err != nil { - // If reset fails, create a new reader + // Reset failed (e.g. non-gzip data): return the pooled reader so it + // isn't leaked, and hand back a fresh one for this call. We must not + // call gr.Close() here — the decompressor is nil until a header + // parses, so Close would panic. The next GetGzipReader's Reset + // reinitialises it. + gzipReaderPool.Put(gr) return gzip.NewReader(r) } return gr, nil diff --git a/pkg/pools/buffer_test.go b/pkg/pools/buffer_test.go index ca507a6..d9c1328 100644 --- a/pkg/pools/buffer_test.go +++ b/pkg/pools/buffer_test.go @@ -415,3 +415,31 @@ func BenchmarkWithoutPool(b *testing.B) { } }) } + +// TestGetGzipReader_ResetErrorPath exercises the path where the pooled reader +// fails to Reset (non-gzip input). The pooled reader must be returned to the +// pool and a usable reader handed back; a subsequent valid gzip read must +// still succeed so the pool is not left in a broken state. +func (suite *BufferPoolTestSuite) TestGetGzipReader_ResetErrorPath() { + t := suite.T() + // Non-gzip input -> Reset fails -> fresh reader returned (with error). + gr1, err := GetGzipReader(strings.NewReader("not gzip data at all")) + assert.Error(t, err, "expected error decoding non-gzip data") + if gr1 != nil { + PutGzipReader(gr1) + } + + // Valid gzip input afterwards must still work, proving the pool is usable. + var buf bytes.Buffer + zw := gzip.NewWriter(&buf) + _, werr := zw.Write([]byte("hello world")) + assert.NoError(t, werr) + assert.NoError(t, zw.Close()) + + gr2, err := GetGzipReader(bytes.NewReader(buf.Bytes())) + assert.NoError(t, err) + defer PutGzipReader(gr2) + decoded, rerr := io.ReadAll(gr2) + assert.NoError(t, rerr) + assert.Equal(t, "hello world", string(decoded)) +}