fix: double compression.

middlewares/compress.go

@@ -1,13 +1,11 @@
 package middlewares
 
 import (
+	"compress/gzip"
 	"net/http"
 
 	"github.com/NYTimes/gziphandler"
-)
-
-const (
-	contentEncodingHeader = "Content-Encoding"
+	"github.com/containous/traefik/log"
 )
 
 // Compress is a middleware that allows redirection
@@ -15,17 +13,13 @@ type Compress struct{}
 
 // ServerHTTP is a function used by Negroni
 func (c *Compress) ServeHTTP(rw http.ResponseWriter, r *http.Request, next http.HandlerFunc) {
-	if isEncoded(r.Header) {
-		next.ServeHTTP(rw, r)
-	} else {
-		newGzipHandler := gziphandler.GzipHandler(next)
-		newGzipHandler.ServeHTTP(rw, r)
-	}
+	gzipHandler(next).ServeHTTP(rw, r)
 }
 
-func isEncoded(headers http.Header) bool {
-	header := headers.Get(contentEncodingHeader)
-	// According to https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Encoding,
-	// content is not encoded if the header 'Content-Encoding' is empty or equals to 'identity'.
-	return header != "" && header != "identity"
+func gzipHandler(h http.Handler) http.Handler {
+	wrapper, err := gziphandler.NewGzipHandler(gzip.DefaultCompression, gziphandler.DefaultMinSize, &gziphandler.GzipResponseWriterWrapper{})
+	if err != nil {
+		log.Error(err)
+	}
+	return wrapper(h)
 }

middlewares/compress_test.go

@@ -1,36 +1,39 @@
 package middlewares
 
 import (
+	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
 	"testing"
 
 	"github.com/NYTimes/gziphandler"
+	"github.com/codegangsta/negroni"
 	"github.com/containous/traefik/testhelpers"
 	"github.com/stretchr/testify/assert"
 )
 
 const (
-	acceptEncodingHeader = "Accept-Encoding"
-	varyHeader           = "Vary"
-	gzip                 = "gzip"
+	acceptEncodingHeader  = "Accept-Encoding"
+	contentEncodingHeader = "Content-Encoding"
+	varyHeader            = "Vary"
+	gzipValue             = "gzip"
 )
 
 func TestShouldCompressWhenNoContentEncodingHeader(t *testing.T) {
 	handler := &Compress{}
 
 	req := testhelpers.MustNewRequest(http.MethodGet, "http://localhost", nil)
-	req.Header.Add(acceptEncodingHeader, gzip)
+	req.Header.Add(acceptEncodingHeader, gzipValue)
 
 	baseBody := generateBytes(gziphandler.DefaultMinSize)
 	next := func(rw http.ResponseWriter, r *http.Request) {
 		rw.Write(baseBody)
 	}
-	rw := httptest.NewRecorder()
 
+	rw := httptest.NewRecorder()
 	handler.ServeHTTP(rw, req, next)
 
-	assert.Equal(t, gzip, rw.Header().Get(contentEncodingHeader))
+	assert.Equal(t, gzipValue, rw.Header().Get(contentEncodingHeader))
 	assert.Equal(t, acceptEncodingHeader, rw.Header().Get(varyHeader))
 
 	if assert.ObjectsAreEqualValues(rw.Body.Bytes(), baseBody) {
@@ -42,28 +45,105 @@ func TestShouldNotCompressWhenContentEncodingHeader(t *testing.T) {
 	handler := &Compress{}
 
 	req := testhelpers.MustNewRequest(http.MethodGet, "http://localhost", nil)
-	req.Header.Add(acceptEncodingHeader, gzip)
-	req.Header.Add(contentEncodingHeader, gzip)
-
-	baseBody := generateBytes(gziphandler.DefaultMinSize)
+	req.Header.Add(acceptEncodingHeader, gzipValue)
 
+	fakeCompressedBody := generateBytes(gziphandler.DefaultMinSize)
 	next := func(rw http.ResponseWriter, r *http.Request) {
-		rw.Write(baseBody)
+		rw.Header().Add(contentEncodingHeader, gzipValue)
+		rw.Header().Add(varyHeader, acceptEncodingHeader)
+		rw.Write(fakeCompressedBody)
 	}
 
 	rw := httptest.NewRecorder()
 	handler.ServeHTTP(rw, req, next)
 
-	assert.Equal(t, "", rw.Header().Get(contentEncodingHeader))
-	assert.Equal(t, "", rw.Header().Get(varyHeader))
+	assert.Equal(t, gzipValue, rw.Header().Get(contentEncodingHeader))
+	assert.Equal(t, acceptEncodingHeader, rw.Header().Get(varyHeader))
 
-	assert.EqualValues(t, rw.Body.Bytes(), baseBody)
+	assert.EqualValues(t, rw.Body.Bytes(), fakeCompressedBody)
+}
+
+func TestShouldNotCompressWhenNoAcceptEncodingHeader(t *testing.T) {
+	handler := &Compress{}
+
+	req := testhelpers.MustNewRequest(http.MethodGet, "http://localhost", nil)
+
+	fakeBody := generateBytes(gziphandler.DefaultMinSize)
+	next := func(rw http.ResponseWriter, r *http.Request) {
+		rw.Write(fakeBody)
+	}
+
+	rw := httptest.NewRecorder()
+	handler.ServeHTTP(rw, req, next)
+
+	assert.Empty(t, rw.Header().Get(contentEncodingHeader))
+	assert.EqualValues(t, rw.Body.Bytes(), fakeBody)
+}
+
+func TestIntegrationShouldNotCompressWhenContentAlreadyCompressed(t *testing.T) {
+	fakeCompressedBody := generateBytes(100000)
+
+	handler := func(rw http.ResponseWriter, r *http.Request) {
+		rw.Header().Add(contentEncodingHeader, gzipValue)
+		rw.Header().Add(varyHeader, acceptEncodingHeader)
+		rw.Write(fakeCompressedBody)
+	}
+
+	comp := &Compress{}
+
+	negro := negroni.New(comp)
+	negro.UseHandlerFunc(handler)
+	ts := httptest.NewServer(negro)
+	defer ts.Close()
+
+	client := &http.Client{}
+	req := testhelpers.MustNewRequest(http.MethodGet, ts.URL, nil)
+	req.Header.Add(acceptEncodingHeader, gzipValue)
+
+	resp, err := client.Do(req)
+	assert.NoError(t, err, "there should be no error")
+
+	assert.Equal(t, gzipValue, resp.Header.Get(contentEncodingHeader))
+	assert.Equal(t, acceptEncodingHeader, resp.Header.Get(varyHeader))
+
+	body, err := ioutil.ReadAll(resp.Body)
+	assert.EqualValues(t, fakeCompressedBody, body)
+}
+
+func TestIntegrationShouldCompressWhenAcceptEncodingHeaderIsPresent(t *testing.T) {
+	fakeBody := generateBytes(100000)
+
+	handler := func(rw http.ResponseWriter, r *http.Request) {
+		rw.Write(fakeBody)
+	}
+
+	comp := &Compress{}
+
+	negro := negroni.New(comp)
+	negro.UseHandlerFunc(handler)
+	ts := httptest.NewServer(negro)
+	defer ts.Close()
+
+	client := &http.Client{}
+	req := testhelpers.MustNewRequest(http.MethodGet, ts.URL, nil)
+	req.Header.Add(acceptEncodingHeader, gzipValue)
+
+	resp, err := client.Do(req)
+	assert.NoError(t, err, "there should be no error")
+
+	assert.Equal(t, gzipValue, resp.Header.Get(contentEncodingHeader))
+	assert.Equal(t, acceptEncodingHeader, resp.Header.Get(varyHeader))
+
+	body, err := ioutil.ReadAll(resp.Body)
+	if assert.ObjectsAreEqualValues(body, fakeBody) {
+		assert.Fail(t, "expected a compressed body", "got %v", body)
+	}
 }
 
 func generateBytes(len int) []byte {
 	var value []byte
 	for i := 0; i < len; i++ {
-		value = append(value, 0x61)
+		value = append(value, 0x61+byte(i))
 	}
 	return value
 }