From 463eb4c68a46e8bce59e26d1fe7b649c9e6c5d85 Mon Sep 17 00:00:00 2001 From: Will Bicks Date: Mon, 16 Jan 2023 04:55:41 -0600 Subject: [PATCH] Add SuffixETag() and DropETag() options to prevent ETag collisions on compressed responses (#740) --- gzhttp/compress.go | 51 +++++++++++++++++++++++ gzhttp/compress_test.go | 90 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+) diff --git a/gzhttp/compress.go b/gzhttp/compress.go index eae0bba07c..56941d2090 100644 --- a/gzhttp/compress.go +++ b/gzhttp/compress.go @@ -29,6 +29,7 @@ const ( acceptRanges = "Accept-Ranges" contentType = "Content-Type" contentLength = "Content-Length" + eTag = "ETag" ) type codings map[string]float64 @@ -64,6 +65,8 @@ type GzipResponseWriter struct { ignore bool // If true, then we immediately passthru writes to the underlying ResponseWriter. keepAcceptRanges bool // Keep "Accept-Ranges" header. setContentType bool // Add content type, if missing and detected. + suffixETag string // Suffix to add to ETag header if response is compressed. + dropETag bool // Drop ETag header if response is compressed (supersedes suffixETag). contentTypeFilter func(ct string) bool // Only compress if the response is one of these content-types. All are accepted if empty. } @@ -168,6 +171,21 @@ func (w *GzipResponseWriter) startGzip() error { w.Header().Del(acceptRanges) } + // Suffix ETag. + if w.suffixETag != "" && !w.dropETag && w.Header().Get(eTag) != "" { + orig := w.Header().Get(eTag) + insertPoint := strings.LastIndex(orig, `"`) + if insertPoint == -1 { + insertPoint = len(orig) + } + w.Header().Set(eTag, orig[:insertPoint]+w.suffixETag+orig[insertPoint:]) + } + + // Delete ETag. + if w.dropETag { + w.Header().Del(eTag) + } + // Write the header to gzip response. if w.code != 0 { w.ResponseWriter.WriteHeader(w.code) @@ -370,6 +388,8 @@ func NewWrapper(opts ...option) (func(http.Handler) http.HandlerFunc, error) { minSize: c.minSize, contentTypeFilter: c.contentTypes, keepAcceptRanges: c.keepAcceptRanges, + dropETag: c.dropETag, + suffixETag: c.suffixETag, buf: gw.buf, setContentType: c.setContentType, } @@ -433,6 +453,8 @@ type config struct { contentTypes func(ct string) bool keepAcceptRanges bool setContentType bool + suffixETag string + dropETag bool } func (c *config) validate() error { @@ -574,6 +596,35 @@ func ContentTypeFilter(compress func(ct string) bool) option { } } +// SuffixETag adds the specified suffix to the ETag header (if it exists) of +// responses which are compressed. +// +// Per [RFC 7232 Section 2.3.3](https://www.rfc-editor.org/rfc/rfc7232#section-2.3.3), +// the ETag of a compressed response must differ from it's uncompressed version. +// +// A suffix such as "-gzip" is sometimes used as a workaround for generating a +// unique new ETag (see https://bz.apache.org/bugzilla/show_bug.cgi?id=39727). +func SuffixETag(suffix string) option { + return func(c *config) { + c.suffixETag = suffix + } +} + +// DropETag removes the ETag of responses which are compressed. If DropETag is +// specified in conjunction with SuffixETag, this option will take precedence +// and the ETag will be dropped. +// +// Per [RFC 7232 Section 2.3.3](https://www.rfc-editor.org/rfc/rfc7232#section-2.3.3), +// the ETag of a compressed response must differ from it's uncompressed version. +// +// This workaround eliminates ETag conflicts between the compressed and +// uncompressed versions by removing the ETag from the compressed version. +func DropETag() option { + return func(c *config) { + c.dropETag = true + } +} + // acceptsGzip returns true if the given HTTP request indicates that it will // accept a gzipped response. func acceptsGzip(r *http.Request) bool { diff --git a/gzhttp/compress_test.go b/gzhttp/compress_test.go index 262ee52495..6b7c92fd30 100644 --- a/gzhttp/compress_test.go +++ b/gzhttp/compress_test.go @@ -180,6 +180,96 @@ func TestGzipHandlerKeepAcceptRange(t *testing.T) { assertEqual(t, testBody, got) } +func TestGzipHandlerSuffixETag(t *testing.T) { + wrapper, err := NewWrapper(SuffixETag("-gzip")) + assertNil(t, err) + + handlerWithETag := wrapper( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("ETag", `W/"1234"`) + w.WriteHeader(http.StatusOK) + w.Write([]byte(testBody)) + })) + handlerWithoutETag := wrapper( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(testBody)) + })) + + req, _ := http.NewRequest("GET", "/gzipped", nil) + req.Header.Set("Accept-Encoding", "gzip") + + respWithEtag := httptest.NewRecorder() + respWithoutEtag := httptest.NewRecorder() + handlerWithETag.ServeHTTP(respWithEtag, req) + handlerWithoutETag.ServeHTTP(respWithoutEtag, req) + + resWithEtag := respWithEtag.Result() + assertEqual(t, 200, resWithEtag.StatusCode) + assertEqual(t, "gzip", resWithEtag.Header.Get("Content-Encoding")) + assertEqual(t, `W/"1234-gzip"`, resWithEtag.Header.Get("ETag")) + zr, err := gzip.NewReader(resWithEtag.Body) + assertNil(t, err) + got, err := io.ReadAll(zr) + assertNil(t, err) + assertEqual(t, testBody, got) + + resWithoutEtag := respWithoutEtag.Result() + assertEqual(t, 200, resWithoutEtag.StatusCode) + assertEqual(t, "gzip", resWithoutEtag.Header.Get("Content-Encoding")) + assertEqual(t, "", resWithoutEtag.Header.Get("ETag")) + zr, err = gzip.NewReader(resWithoutEtag.Body) + assertNil(t, err) + got, err = io.ReadAll(zr) + assertNil(t, err) + assertEqual(t, testBody, got) +} + +func TestGzipHandlerDropETag(t *testing.T) { + wrapper, err := NewWrapper(DropETag()) + assertNil(t, err) + + handlerCompressed := wrapper( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("ETag", `W/"1234"`) + w.WriteHeader(http.StatusOK) + w.Write([]byte(testBody)) + })) + handlerUncompressed := wrapper( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("ETag", `W/"1234"`) + w.Header().Set(HeaderNoCompression, "true") + w.WriteHeader(http.StatusOK) + w.Write([]byte(testBody)) + })) + + req, _ := http.NewRequest("GET", "/gzipped", nil) + req.Header.Set("Accept-Encoding", "gzip") + + respCompressed := httptest.NewRecorder() + respUncompressed := httptest.NewRecorder() + handlerCompressed.ServeHTTP(respCompressed, req) + handlerUncompressed.ServeHTTP(respUncompressed, req) + + resCompressed := respCompressed.Result() + assertEqual(t, 200, resCompressed.StatusCode) + assertEqual(t, "gzip", resCompressed.Header.Get("Content-Encoding")) + assertEqual(t, "", resCompressed.Header.Get("ETag")) + zr, err := gzip.NewReader(resCompressed.Body) + assertNil(t, err) + got, err := io.ReadAll(zr) + assertNil(t, err) + assertEqual(t, testBody, got) + + resUncompressed := respUncompressed.Result() + assertEqual(t, 200, resUncompressed.StatusCode) + assertEqual(t, "", resUncompressed.Header.Get("Content-Encoding")) + assertEqual(t, `W/"1234"`, resUncompressed.Header.Get("ETag")) + got, err = io.ReadAll(resUncompressed.Body) + assertNil(t, err) + assertEqual(t, testBody, got) +} + func TestNewGzipLevelHandler(t *testing.T) { handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK)