Skip to content

Commit 4c0c353

Browse files
authored
fix(header): disable gzip compression for error responses (#120)
* fix: handle gzip compression correctly for error responses - Prevent compression and remove gzip-related headers for error (4xx, 5xx) responses - Ensure gzip writer properly tracks the response status code - Only close the gzip writer when compression was used and nothing was written, or for error responses - Add tests verifying that 404 responses are not compressed and do not include gzip-specific headers fix #98 Signed-off-by: appleboy <appleboy.tw@gmail.com> * feat: extend gzipWriter with header and response handling - Add methods to gzipWriter to expose HTTP status, response size, and written state - Implement WriteHeaderNow to force HTTP header writing through gzipWriter Signed-off-by: appleboy <appleboy.tw@gmail.com> --------- Signed-off-by: appleboy <appleboy.tw@gmail.com>
1 parent 05c6a0d commit 4c0c353

File tree

3 files changed

+116
-5
lines changed

3 files changed

+116
-5
lines changed

gzip.go

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,26 +24,77 @@ func Gzip(level int, options ...Option) gin.HandlerFunc {
2424

2525
type gzipWriter struct {
2626
gin.ResponseWriter
27-
writer *gzip.Writer
27+
writer *gzip.Writer
28+
statusWritten bool
29+
status int
2830
}
2931

3032
func (g *gzipWriter) WriteString(s string) (int, error) {
31-
g.Header().Del("Content-Length")
32-
return g.writer.Write([]byte(s))
33+
return g.Write([]byte(s))
3334
}
3435

3536
func (g *gzipWriter) Write(data []byte) (int, error) {
3637
g.Header().Del("Content-Length")
38+
39+
// Check status from ResponseWriter if not set via WriteHeader
40+
if !g.statusWritten {
41+
g.status = g.ResponseWriter.Status()
42+
}
43+
44+
// For error responses (4xx, 5xx), don't compress
45+
if g.status >= 400 {
46+
g.removeGzipHeaders()
47+
return g.ResponseWriter.Write(data)
48+
}
49+
3750
return g.writer.Write(data)
3851
}
3952

53+
// Status returns the HTTP response status code
54+
func (g *gzipWriter) Status() int {
55+
if g.statusWritten {
56+
return g.status
57+
}
58+
return g.ResponseWriter.Status()
59+
}
60+
61+
// Size returns the number of bytes already written into the response http body
62+
func (g *gzipWriter) Size() int {
63+
return g.ResponseWriter.Size()
64+
}
65+
66+
// Written returns true if the response body was already written
67+
func (g *gzipWriter) Written() bool {
68+
return g.ResponseWriter.Written()
69+
}
70+
71+
// WriteHeaderNow forces to write the http header
72+
func (g *gzipWriter) WriteHeaderNow() {
73+
g.ResponseWriter.WriteHeaderNow()
74+
}
75+
76+
// removeGzipHeaders removes compression-related headers for error responses
77+
func (g *gzipWriter) removeGzipHeaders() {
78+
g.Header().Del("Content-Encoding")
79+
g.Header().Del("Vary")
80+
g.Header().Del("ETag")
81+
}
82+
4083
func (g *gzipWriter) Flush() {
4184
_ = g.writer.Flush()
4285
g.ResponseWriter.Flush()
4386
}
4487

4588
// Fix: https://github.com/mholt/caddy/issues/38
4689
func (g *gzipWriter) WriteHeader(code int) {
90+
g.status = code
91+
g.statusWritten = true
92+
93+
// Remove gzip headers for error responses
94+
if code >= 400 {
95+
g.removeGzipHeaders()
96+
}
97+
4798
g.Header().Del("Content-Length")
4899
g.ResponseWriter.WriteHeader(code)
49100
}

handler.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,13 @@ func (g *gzipHandler) Handle(c *gin.Context) {
8484
if originalEtag != "" && !strings.HasPrefix(originalEtag, "W/") {
8585
c.Header("ETag", "W/"+originalEtag)
8686
}
87-
c.Writer = &gzipWriter{c.Writer, gz}
87+
gw := &gzipWriter{ResponseWriter: c.Writer, writer: gz}
88+
c.Writer = gw
8889
defer func() {
89-
if c.Writer.Size() < 0 {
90+
// Only close gzip writer if it was actually used (not for error responses)
91+
if gw.status >= 400 {
92+
gz.Reset(io.Discard)
93+
} else if c.Writer.Size() < 0 {
9094
// do not write gzip footer when nothing is written to the response body
9195
gz.Reset(io.Discard)
9296
}

handler_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,3 +99,59 @@ func TestHandleDecompressGzip(t *testing.T) {
9999
assert.Equal(t, http.StatusOK, w.Code)
100100
assert.Equal(t, "ok", w.Body.String())
101101
}
102+
103+
func TestHandle404NoCompression(t *testing.T) {
104+
gin.SetMode(gin.TestMode)
105+
106+
tests := []struct {
107+
name string
108+
acceptEncoding string
109+
expectedGzip bool
110+
}{
111+
{
112+
name: "404 with gzip accept-encoding should not compress",
113+
acceptEncoding: "gzip",
114+
expectedGzip: false,
115+
},
116+
{
117+
name: "404 without gzip accept-encoding",
118+
acceptEncoding: "",
119+
expectedGzip: false,
120+
},
121+
}
122+
123+
for _, tt := range tests {
124+
t.Run(tt.name, func(t *testing.T) {
125+
router := gin.New()
126+
router.Use(Gzip(DefaultCompression))
127+
// Register a route to get proper 404 for unmatched paths
128+
router.NoRoute(func(c *gin.Context) {
129+
c.String(http.StatusNotFound, "404 page not found")
130+
})
131+
132+
req, _ := http.NewRequestWithContext(context.Background(), "GET", "/nonexistent", nil)
133+
if tt.acceptEncoding != "" {
134+
req.Header.Set(headerAcceptEncoding, tt.acceptEncoding)
135+
}
136+
137+
w := httptest.NewRecorder()
138+
router.ServeHTTP(w, req)
139+
140+
assert.Equal(t, http.StatusNotFound, w.Code)
141+
142+
// Check that Content-Encoding header is not set for 404 responses
143+
contentEncoding := w.Header().Get("Content-Encoding")
144+
if tt.expectedGzip {
145+
assert.Equal(t, "gzip", contentEncoding)
146+
} else {
147+
assert.Empty(t, contentEncoding, "404 responses should not have Content-Encoding: gzip")
148+
}
149+
150+
// Verify that Vary header is also not set for uncompressed 404 responses
151+
if !tt.expectedGzip {
152+
vary := w.Header().Get("Vary")
153+
assert.Empty(t, vary, "404 responses should not have Vary header when not compressed")
154+
}
155+
})
156+
}
157+
}

0 commit comments

Comments
 (0)