Add SuffixETag() and DropETag() options to prevent ETag collisions on compressed responses (#740)

Author: willbicks

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 {

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)