Skip to content

Refactor: use global header constants instead of hardcoded strings #128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Refactor: use global header constants instead of hardcoded strings #128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions gzip.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func (g *gzipWriter) WriteString(s string) (int, error) {
}

func (g *gzipWriter) Write(data []byte) (int, error) {
g.Header().Del("Content-Length")
g.Header().Del(headerContentLength)

// Check status from ResponseWriter if not set via WriteHeader
if !g.statusWritten {
Expand All @@ -51,7 +51,7 @@ func (g *gzipWriter) Write(data []byte) (int, error) {

// Check if response is already gzip-compressed by looking at Content-Encoding header
// If upstream handler already set gzip encoding, pass through without double compression
if contentEncoding := g.Header().Get("Content-Encoding"); contentEncoding != "" && contentEncoding != gzipEncoding {
if contentEncoding := g.Header().Get(headerContentEncoding); contentEncoding != "" && contentEncoding != gzipEncoding {
// Different encoding, remove our gzip headers and pass through
g.removeGzipHeaders()
return g.ResponseWriter.Write(data)
Expand Down Expand Up @@ -92,9 +92,9 @@ func (g *gzipWriter) WriteHeaderNow() {

// removeGzipHeaders removes compression-related headers for error responses
func (g *gzipWriter) removeGzipHeaders() {
g.Header().Del("Content-Encoding")
g.Header().Del("Vary")
g.Header().Del("ETag")
g.Header().Del(headerContentEncoding)
g.Header().Del(headerVary)
g.Header().Del(headerETag)
}

func (g *gzipWriter) Flush() {
Expand All @@ -111,7 +111,7 @@ func (g *gzipWriter) WriteHeader(code int) {
// because some handlers (like static file server) may call WriteHeader multiple times
// We'll check the status in Write() method when content is actually written

g.Header().Del("Content-Length")
g.Header().Del(headerContentLength)
g.ResponseWriter.WriteHeader(code)
}

Expand Down
44 changes: 22 additions & 22 deletions gzip_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ func newServer() *gin.Engine {
router := gin.New()
router.Use(Gzip(DefaultCompression))
router.GET("/", func(c *gin.Context) {
c.Header("Content-Length", strconv.Itoa(len(testResponse)))
c.Header(headerContentLength, strconv.Itoa(len(testResponse)))
c.String(200, testResponse)
})
router.GET("/ping", func(c *gin.Context) {
c.Writer.Header().Add("Vary", "Origin")
c.Writer.Header().Add(headerVary, "Origin")
}, func(c *gin.Context) {
c.Header("Content-Length", strconv.Itoa(len(testResponse)))
c.Header(headerContentLength, strconv.Itoa(len(testResponse)))
c.String(200, testResponse)
})
router.Any("/reverse", func(c *gin.Context) {
Expand All @@ -82,9 +82,9 @@ func TestVaryHeader(t *testing.T) {
assert.Equal(t, 200, w.Code)
assert.Equal(t, "gzip", w.Header().Get(headerContentEncoding))
assert.Equal(t, []string{headerAcceptEncoding, "Origin"}, w.Header().Values(headerVary))
assert.NotEqual(t, "0", w.Header().Get("Content-Length"))
assert.NotEqual(t, "0", w.Header().Get(headerContentLength))
assert.NotEqual(t, 19, w.Body.Len())
assert.Equal(t, w.Header().Get("Content-Length"), fmt.Sprint(w.Body.Len()))
assert.Equal(t, w.Header().Get(headerContentLength), fmt.Sprint(w.Body.Len()))

gr, err := gzip.NewReader(w.Body)
assert.NoError(t, err)
Expand All @@ -105,9 +105,9 @@ func TestGzip(t *testing.T) {
assert.Equal(t, w.Code, 200)
assert.Equal(t, w.Header().Get(headerContentEncoding), "gzip")
assert.Equal(t, w.Header().Get(headerVary), headerAcceptEncoding)
assert.NotEqual(t, w.Header().Get("Content-Length"), "0")
assert.NotEqual(t, w.Header().Get(headerContentLength), "0")
assert.NotEqual(t, w.Body.Len(), 19)
assert.Equal(t, fmt.Sprint(w.Body.Len()), w.Header().Get("Content-Length"))
assert.Equal(t, fmt.Sprint(w.Body.Len()), w.Header().Get(headerContentLength))

gr, err := gzip.NewReader(w.Body)
assert.NoError(t, err)
Expand Down Expand Up @@ -166,7 +166,7 @@ func TestExcludedPathsAndExtensions(t *testing.T) {
assert.Equal(t, tt.expectedContentEncoding, w.Header().Get(headerContentEncoding))
assert.Equal(t, tt.expectedVary, w.Header().Get(headerVary))
assert.Equal(t, tt.expectedBody, w.Body.String())
assert.Equal(t, tt.expectedContentLength, w.Header().Get("Content-Length"))
assert.Equal(t, tt.expectedContentLength, w.Header().Get(headerContentLength))
}
}

Expand All @@ -179,7 +179,7 @@ func TestNoGzip(t *testing.T) {

assert.Equal(t, w.Code, 200)
assert.Equal(t, w.Header().Get(headerContentEncoding), "")
assert.Equal(t, w.Header().Get("Content-Length"), "19")
assert.Equal(t, w.Header().Get(headerContentLength), "19")
assert.Equal(t, w.Body.String(), testResponse)
}

Expand All @@ -194,9 +194,9 @@ func TestGzipWithReverseProxy(t *testing.T) {
assert.Equal(t, w.Code, 200)
assert.Equal(t, w.Header().Get(headerContentEncoding), "gzip")
assert.Equal(t, w.Header().Get(headerVary), headerAcceptEncoding)
assert.NotEqual(t, w.Header().Get("Content-Length"), "0")
assert.NotEqual(t, w.Header().Get(headerContentLength), "0")
assert.NotEqual(t, w.Body.Len(), 19)
assert.Equal(t, fmt.Sprint(w.Body.Len()), w.Header().Get("Content-Length"))
assert.Equal(t, fmt.Sprint(w.Body.Len()), w.Header().Get(headerContentLength))

gr, err := gzip.NewReader(w.Body)
assert.NoError(t, err)
Expand Down Expand Up @@ -224,7 +224,7 @@ func TestDecompressGzip(t *testing.T) {
if v := c.Request.Header.Get(headerContentEncoding); v != "" {
t.Errorf("unexpected `Content-Encoding`: %s header", v)
}
if v := c.Request.Header.Get("Content-Length"); v != "" {
if v := c.Request.Header.Get(headerContentLength); v != "" {
t.Errorf("unexpected `Content-Length`: %s header", v)
}
data, err := c.GetRawData()
Expand All @@ -241,7 +241,7 @@ func TestDecompressGzip(t *testing.T) {
assert.Equal(t, "", w.Header().Get(headerContentEncoding))
assert.Equal(t, "", w.Header().Get(headerVary))
assert.Equal(t, testResponse, w.Body.String())
assert.Equal(t, "", w.Header().Get("Content-Length"))
assert.Equal(t, "", w.Header().Get(headerContentLength))
}

func TestDecompressGzipWithEmptyBody(t *testing.T) {
Expand All @@ -261,7 +261,7 @@ func TestDecompressGzipWithEmptyBody(t *testing.T) {
assert.Equal(t, "", w.Header().Get(headerContentEncoding))
assert.Equal(t, "", w.Header().Get(headerVary))
assert.Equal(t, "ok", w.Body.String())
assert.Equal(t, "", w.Header().Get("Content-Length"))
assert.Equal(t, "", w.Header().Get(headerContentLength))
}

func TestDecompressGzipWithIncorrectData(t *testing.T) {
Expand Down Expand Up @@ -298,7 +298,7 @@ func TestDecompressOnly(t *testing.T) {
if v := c.Request.Header.Get(headerContentEncoding); v != "" {
t.Errorf("unexpected `Content-Encoding`: %s header", v)
}
if v := c.Request.Header.Get("Content-Length"); v != "" {
if v := c.Request.Header.Get(headerContentLength); v != "" {
t.Errorf("unexpected `Content-Length`: %s header", v)
}
data, err := c.GetRawData()
Expand All @@ -315,7 +315,7 @@ func TestDecompressOnly(t *testing.T) {
assert.Equal(t, "", w.Header().Get(headerContentEncoding))
assert.Equal(t, "", w.Header().Get(headerVary))
assert.Equal(t, testResponse, w.Body.String())
assert.Equal(t, "", w.Header().Get("Content-Length"))
assert.Equal(t, "", w.Header().Get(headerContentLength))
}

func TestGzipWithDecompressOnly(t *testing.T) {
Expand All @@ -335,7 +335,7 @@ func TestGzipWithDecompressOnly(t *testing.T) {
r.Use(Gzip(NoCompression, WithDecompressOnly(), WithDecompressFn(DefaultDecompressHandle)))
r.POST("/", func(c *gin.Context) {
assert.Equal(t, c.Request.Header.Get(headerContentEncoding), "")
assert.Equal(t, c.Request.Header.Get("Content-Length"), "")
assert.Equal(t, c.Request.Header.Get(headerContentLength), "")
body, err := c.GetRawData()
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -364,7 +364,7 @@ func TestCustomShouldCompressFn(t *testing.T) {
}),
))
router.GET("/", func(c *gin.Context) {
c.Header("Content-Length", strconv.Itoa(len(testResponse)))
c.Header(headerContentLength, strconv.Itoa(len(testResponse)))
c.String(200, testResponse)
})

Expand All @@ -373,7 +373,7 @@ func TestCustomShouldCompressFn(t *testing.T) {

assert.Equal(t, 200, w.Code)
assert.Equal(t, "", w.Header().Get(headerContentEncoding))
assert.Equal(t, "19", w.Header().Get("Content-Length"))
assert.Equal(t, "19", w.Header().Get(headerContentLength))
assert.Equal(t, testResponse, w.Body.String())
}

Expand Down Expand Up @@ -414,7 +414,7 @@ func TestResponseWriterHijack(t *testing.T) {
c.Next()
}))
router.GET("/", func(c *gin.Context) {
c.Header("Content-Length", strconv.Itoa(len(testResponse)))
c.Header(headerContentLength, strconv.Itoa(len(testResponse)))
c.String(200, testResponse)
})

Expand All @@ -435,7 +435,7 @@ func TestDoubleGzipCompression(t *testing.T) {

// Set gzip headers to simulate already compressed content
w.Header().Set(headerContentEncoding, "gzip")
w.Header().Set("Content-Length", strconv.Itoa(buf.Len()))
w.Header().Set(headerContentLength, strconv.Itoa(buf.Len()))
w.WriteHeader(200)
_, err = w.Write(buf.Bytes())
require.NoError(t, err)
Expand Down Expand Up @@ -507,7 +507,7 @@ http_requests_total{method="get",status="400"} 3 1395066363000`

w.Header().Set(headerContentEncoding, "gzip")
w.Header().Set("Content-Type", "text/plain; version=0.0.4; charset=utf-8")
w.Header().Set("Content-Length", strconv.Itoa(buf.Len()))
w.Header().Set(headerContentLength, strconv.Itoa(buf.Len()))
w.WriteHeader(200)
_, err = w.Write(buf.Bytes())
require.NoError(t, err)
Expand Down
10 changes: 6 additions & 4 deletions handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ import (
const (
headerAcceptEncoding = "Accept-Encoding"
headerContentEncoding = "Content-Encoding"
headerContentLength = "Content-Length"
headerVary = "Vary"
headerETag = "ETag"
)

type gzipHandler struct {
Expand Down Expand Up @@ -64,7 +66,7 @@ func newGzipHandler(level int, opts ...Option) *gzipHandler {
// and wraps the response writer with a gzipWriter. After the request is processed, it ensures the gzip.Writer
// is properly closed and the "Content-Length" header is set based on the response size.
func (g *gzipHandler) Handle(c *gin.Context) {
if fn := g.decompressFn; fn != nil && strings.Contains(c.Request.Header.Get("Content-Encoding"), "gzip") {
if fn := g.decompressFn; fn != nil && strings.Contains(c.Request.Header.Get(headerContentEncoding), "gzip") {
fn(c)
}

Expand All @@ -80,9 +82,9 @@ func (g *gzipHandler) Handle(c *gin.Context) {
c.Header(headerContentEncoding, "gzip")
c.Writer.Header().Add(headerVary, headerAcceptEncoding)
// check ETag Header
originalEtag := c.GetHeader("ETag")
originalEtag := c.GetHeader(headerETag)
if originalEtag != "" && !strings.HasPrefix(originalEtag, "W/") {
c.Header("ETag", "W/"+originalEtag)
c.Header(headerETag, "W/"+originalEtag)
}
gw := &gzipWriter{ResponseWriter: c.Writer, writer: gz}
c.Writer = gw
Expand All @@ -98,7 +100,7 @@ func (g *gzipHandler) Handle(c *gin.Context) {
}
_ = gz.Close()
if c.Writer.Size() > -1 {
c.Header("Content-Length", strconv.Itoa(c.Writer.Size()))
c.Header(headerContentLength, strconv.Itoa(c.Writer.Size()))
}
g.gzPool.Put(gz)
}()
Expand Down
6 changes: 3 additions & 3 deletions handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func TestHandleGzip(t *testing.T) {
router.ServeHTTP(w, req)

assert.Equal(t, http.StatusOK, w.Code)
assert.Equal(t, tt.expectedContentEncoding, w.Header().Get("Content-Encoding"))
assert.Equal(t, tt.expectedContentEncoding, w.Header().Get(headerContentEncoding))

if tt.expectedContentEncoding == gzipEncoding {
gr, err := gzip.NewReader(w.Body)
Expand Down Expand Up @@ -140,7 +140,7 @@ func TestHandle404NoCompression(t *testing.T) {
assert.Equal(t, http.StatusNotFound, w.Code)

// Check that Content-Encoding header is not set for 404 responses
contentEncoding := w.Header().Get("Content-Encoding")
contentEncoding := w.Header().Get(headerContentEncoding)
if tt.expectedGzip {
assert.Equal(t, gzipEncoding, contentEncoding)
} else {
Expand All @@ -149,7 +149,7 @@ func TestHandle404NoCompression(t *testing.T) {

// Verify that Vary header is also not set for uncompressed 404 responses
if !tt.expectedGzip {
vary := w.Header().Get("Vary")
vary := w.Header().Get(headerVary)
assert.Empty(t, vary, "404 responses should not have Vary header when not compressed")
}
})
Expand Down
6 changes: 3 additions & 3 deletions options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ func DefaultDecompressHandle(c *gin.Context) {
return
}

contentEncodingField := strings.Split(strings.ToLower(c.GetHeader("Content-Encoding")), ",")
contentEncodingField := strings.Split(strings.ToLower(c.GetHeader(headerContentEncoding)), ",")
if len(contentEncodingField) == 0 { // nothing to decompress
c.Next()

Expand Down Expand Up @@ -263,8 +263,8 @@ func DefaultDecompressHandle(c *gin.Context) {
c.Request.Body = r
}

c.Request.Header.Del("Content-Encoding")
c.Request.Header.Del("Content-Length")
c.Request.Header.Del(headerContentEncoding)
c.Request.Header.Del(headerContentLength)

c.Next()
}
2 changes: 1 addition & 1 deletion static_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ func TestStaticFileGzipHeadersBug(t *testing.T) {
t.Logf("Response Status: %d", w.Code)
t.Logf("Content-Encoding: %s", w.Header().Get(headerContentEncoding))
t.Logf("Vary: %s", w.Header().Get(headerVary))
t.Logf("Content-Length: %s", w.Header().Get("Content-Length"))
t.Logf("Content-Length: %s", w.Header().Get(headerContentLength))
t.Logf("Body Length: %d", w.Body.Len())

// This test will currently fail due to the bug described in issue #122
Expand Down