Skip to content

Commit 630e123

Browse files
committed
fix(middleware): gzip Write must return len(b), not the buffer length
gzipResponseWriter.Write returned the total buffered byte count (any previously buffered chunks plus the current slice) once a write crossed MinLength, so it could return n > len(b). That violates the io.Writer contract and makes standard-library consumers that validate the count panic -- notably io.Copy (used by Context.Stream), which panics with "bytes.Reader.WriteTo: invalid Write count". Flush the buffer as before but report only len(b) as written.
1 parent 25685e6 commit 630e123

2 files changed

Lines changed: 82 additions & 1 deletion

File tree

middleware/compress.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,15 @@ func (w *gzipResponseWriter) Write(b []byte) (int, error) {
168168
w.ResponseWriter.WriteHeader(w.code)
169169
}
170170

171-
return w.Writer.Write(w.buffer.Bytes())
171+
// The whole buffer (which already contains b) is flushed to the
172+
// underlying writer, but we must report only len(b) as written to
173+
// satisfy the io.Writer contract (0 <= n <= len(b)). Returning the
174+
// buffer length here would over-report and panic callers such as
175+
// io.Copy with "invalid write count".
176+
if _, err := w.Writer.Write(w.buffer.Bytes()); err != nil {
177+
return 0, err
178+
}
179+
return n, nil
172180
}
173181

174182
return n, err
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
package middleware
2+
3+
import (
4+
"bytes"
5+
"io"
6+
"net/http"
7+
"net/http/httptest"
8+
"testing"
9+
10+
"github.com/labstack/echo/v4"
11+
"github.com/stretchr/testify/assert"
12+
)
13+
14+
// TestGzipWriteReturnsCorrectCount verifies that gzipResponseWriter.Write honours
15+
// the io.Writer contract: it must return n == len(b) (never more) for the slice
16+
// passed to a single Write call. Before the fix, when a buffered write crossed the
17+
// MinLength threshold, Write returned the full buffer length (previous chunks + b),
18+
// over-reporting the count and panicking callers like io.Copy.
19+
func TestGzipWriteReturnsCorrectCount(t *testing.T) {
20+
e := echo.New()
21+
mw := GzipWithConfig(GzipConfig{MinLength: 100})
22+
23+
var n1, n2 int
24+
h := mw(func(c echo.Context) error {
25+
chunk1 := []byte("hello ") // 6 bytes, stays below MinLength
26+
chunk2 := bytes.Repeat([]byte("x"), 200) // crosses MinLength
27+
var err error
28+
if n1, err = c.Response().Write(chunk1); err != nil {
29+
return err
30+
}
31+
if n2, err = c.Response().Write(chunk2); err != nil {
32+
return err
33+
}
34+
return nil
35+
})
36+
37+
req := httptest.NewRequest(http.MethodGet, "/", nil)
38+
req.Header.Set(echo.HeaderAcceptEncoding, "gzip")
39+
rec := httptest.NewRecorder()
40+
assert.NoError(t, h(e.NewContext(req, rec)))
41+
42+
// Each Write must report exactly the length of the slice it was given.
43+
assert.Equal(t, 6, n1, "first Write should report len of its own slice")
44+
assert.Equal(t, 200, n2, "second Write must report len(b), not the buffered total")
45+
}
46+
47+
// TestGzipIoCopyDoesNotPanic reproduces the real-world failure: streaming through
48+
// the gzip response writer with io.Copy (as echo.Context#Stream does) panics with
49+
// "invalid write count" when Write over-reports the byte count.
50+
func TestGzipIoCopyDoesNotPanic(t *testing.T) {
51+
e := echo.New()
52+
mw := GzipWithConfig(GzipConfig{MinLength: 100})
53+
54+
h := mw(func(c echo.Context) error {
55+
// Small write keeps us below MinLength so the buffer holds previous bytes.
56+
if _, err := c.Response().Write([]byte("prefix")); err != nil {
57+
return err
58+
}
59+
// io.Copy validates that the returned write count never exceeds len(p);
60+
// an over-reported count makes bytes.Reader.WriteTo panic.
61+
src := bytes.NewReader(bytes.Repeat([]byte("y"), 200))
62+
_, err := io.Copy(c.Response(), src)
63+
return err
64+
})
65+
66+
req := httptest.NewRequest(http.MethodGet, "/", nil)
67+
req.Header.Set(echo.HeaderAcceptEncoding, "gzip")
68+
rec := httptest.NewRecorder()
69+
70+
assert.NotPanics(t, func() {
71+
assert.NoError(t, h(e.NewContext(req, rec)))
72+
})
73+
}

0 commit comments

Comments
 (0)