Revert compress middleware algorithms priority to v2 behavior

Co-authored-by: Kevin Pollet <pollet.kevin@gmail.com>

docs/content/middlewares/http/compress.md

@@ -264,10 +264,10 @@ http:
 
 ### `encodings`
 
-_Optional, Default="zstd, br, gzip"_
+_Optional, Default="gzip, br, zstd"_
 
 `encodings` specifies the list of supported compression encodings.
-At least one encoding value must be specified, and valid entries are `zstd` (Zstandard), `br` (Brotli), and `gzip` (Gzip).
+At least one encoding value must be specified, and valid entries are `gzip` (Gzip), `br` (Brotli), and `zstd` (Zstandard).
 The order of the list also sets the priority, the top entry has the highest priority.
 
 ```yaml tab="Docker & Swarm"

docs/content/migration/v3.md

@@ -187,3 +187,13 @@ and will be removed in the next major version.
 
 In `v3.3.4`, the OpenTelemetry Request Duration metric (named `traefik_(entrypoint|router|service)_request_duration_seconds`) unit has been changed from milliseconds to seconds.
 To be consistent with the naming and other metrics providers, the metric now reports the duration in seconds.
+
+## v3.3.5
+
+### Compress Middleware
+
+In `v3.3.5`, the compress middleware `encodings` option default value is now `gzip, br, zstd`. 
+This change helps the algorithm selection to favor the `gzip` algorithm over the other algorithms.
+
+It impacts requests that do not specify their preferred algorithm,
+or has no order preference, in the `Accept-Encoding` header.

pkg/config/dynamic/middlewares.go

@@ -187,7 +187,7 @@ type Compress struct {
 }
 
 func (c *Compress) SetDefaults() {
-	c.Encodings = []string{"zstd", "br", "gzip"}
+	c.Encodings = []string{"gzip", "br", "zstd"}
 }
 
 // +k8s:deepcopy-gen=true

pkg/middlewares/compress/acceptencoding.go

@@ -23,57 +23,44 @@ type Encoding struct {
 	Weight float64
 }
 
-func getCompressionEncoding(acceptEncoding []string, defaultEncoding string, supportedEncodings []string) string {
+func (c *compress) getCompressionEncoding(acceptEncoding []string) string {
-	if defaultEncoding == "" {
+	// RFC says: An Accept-Encoding header field with a field value that is empty implies that the user agent does not want any content coding in response.
-		if slices.Contains(supportedEncodings, brotliName) {
+	// https://datatracker.ietf.org/doc/html/rfc9110#name-accept-encoding
-			// Keeps the pre-existing default inside Traefik if brotli is a supported encoding.
+	if len(acceptEncoding) == 1 && acceptEncoding[0] == "" {
-			defaultEncoding = brotliName
+		return identityName
-		} else if len(supportedEncodings) > 0 {
-			// Otherwise use the first supported encoding.
-			defaultEncoding = supportedEncodings[0]
-		}
 	}
 
-	encodings, hasWeight := parseAcceptEncoding(acceptEncoding, supportedEncodings)
+	acceptableEncodings := parseAcceptableEncodings(acceptEncoding, c.supportedEncodings)
 
-	if hasWeight {
+	// An empty Accept-Encoding header field would have been handled earlier.
-		if len(encodings) == 0 {
+	// If empty, it means no encoding is supported, we do not encode.
-			return identityName
+	if len(acceptableEncodings) == 0 {
-		}
+		// TODO: return 415 status code instead of deactivating the compression, if the backend was not to compress as well.
-
+		return notAcceptable
-		encoding := encodings[0]
-
-		if encoding.Type == identityName && encoding.Weight == 0 {
-			return notAcceptable
-		}
-
-		if encoding.Type == wildcardName && encoding.Weight == 0 {
-			return notAcceptable
-		}
-
-		if encoding.Type == wildcardName {
-			return defaultEncoding
-		}
-
-		return encoding.Type
 	}
 
-	for _, dt := range supportedEncodings {
+	slices.SortFunc(acceptableEncodings, func(a, b Encoding) int {
-		if slices.ContainsFunc(encodings, func(e Encoding) bool { return e.Type == dt }) {
+		if a.Weight == b.Weight {
-			return dt
+			// At same weight, we want to prioritize based on the encoding priority.
+			// the lower the index, the higher the priority.
+			return cmp.Compare(c.supportedEncodings[a.Type], c.supportedEncodings[b.Type])
 		}
+		return cmp.Compare(b.Weight, a.Weight)
+	})
+
+	if acceptableEncodings[0].Type == wildcardName {
+		if c.defaultEncoding == "" {
+			return c.encodings[0]
+		}
+
+		return c.defaultEncoding
 	}
 
-	if slices.ContainsFunc(encodings, func(e Encoding) bool { return e.Type == wildcardName }) {
+	return acceptableEncodings[0].Type
-		return defaultEncoding
-	}
-
-	return identityName
 }
 
-func parseAcceptEncoding(acceptEncoding, supportedEncodings []string) ([]Encoding, bool) {
+func parseAcceptableEncodings(acceptEncoding []string, supportedEncodings map[string]int) []Encoding {
 	var encodings []Encoding
-	var hasWeight bool
 
 	for _, line := range acceptEncoding {
 		for _, item := range strings.Split(strings.ReplaceAll(line, " ", ""), ",") {
@@ -82,9 +69,7 @@ func parseAcceptEncoding(acceptEncoding, supportedEncodings []string) ([]Encodin
 				continue
 			}
 
-			if !slices.Contains(supportedEncodings, parsed[0]) &&
+			if _, ok := supportedEncodings[parsed[0]]; !ok {
-				parsed[0] != identityName &&
-				parsed[0] != wildcardName {
 				continue
 			}
 
@@ -94,8 +79,13 @@ func parseAcceptEncoding(acceptEncoding, supportedEncodings []string) ([]Encodin
 			if len(parsed) > 1 && strings.HasPrefix(parsed[1], "q=") {
 				w, _ := strconv.ParseFloat(strings.TrimPrefix(parsed[1], "q="), 64)
 
+				// If the weight is 0, the encoding is not acceptable.
+				// We can skip the encoding.
+				if w == 0 {
+					continue
+				}
+
 				weight = w
-				hasWeight = true
 			}
 
 			encodings = append(encodings, Encoding{
@@ -105,9 +95,5 @@ func parseAcceptEncoding(acceptEncoding, supportedEncodings []string) ([]Encodin
 		}
 	}
 
-	slices.SortFunc(encodings, func(a, b Encoding) int {
+	return encodings
-		return cmp.Compare(b.Weight, a.Weight)
-	})
-
-	return encodings, hasWeight
 }

pkg/middlewares/compress/acceptencoding_test.go

@@ -1,9 +1,12 @@
 package compress
 
 import (
+	"context"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/traefik/traefik/v3/pkg/config/dynamic"
 )
 
 func Test_getCompressionEncoding(t *testing.T) {
@@ -15,14 +18,19 @@ func Test_getCompressionEncoding(t *testing.T) {
 		expected           string
 	}{
 		{
-			desc:           "br > gzip (no weight)",
+			desc:           "Empty Accept-Encoding",
-			acceptEncoding: []string{"gzip, br"},
+			acceptEncoding: []string{""},
-			expected:       brotliName,
+			expected:       identityName,
 		},
 		{
-			desc:           "zstd > br > gzip (no weight)",
+			desc:           "gzip > br (no weight)",
-			acceptEncoding: []string{"zstd, gzip, br"},
+			acceptEncoding: []string{"gzip, br"},
-			expected:       zstdName,
+			expected:       gzipName,
+		},
+		{
+			desc:           "gzip > br > zstd (no weight)",
+			acceptEncoding: []string{"gzip, br, zstd"},
+			expected:       gzipName,
 		},
 		{
 			desc:           "known compression encoding (no weight)",
@@ -32,24 +40,34 @@ func Test_getCompressionEncoding(t *testing.T) {
 		{
 			desc:           "unknown compression encoding (no weight), no encoding",
 			acceptEncoding: []string{"compress, rar"},
-			expected:       identityName,
+			expected:       notAcceptable,
 		},
 		{
-			desc:           "wildcard return the default compression encoding",
+			desc:           "wildcard returns the default compression encoding",
 			acceptEncoding: []string{"*"},
-			expected:       brotliName,
+			expected:       gzipName,
 		},
 		{
-			desc:            "wildcard return the custom default compression encoding",
+			desc:            "wildcard returns the custom default compression encoding",
 			acceptEncoding:  []string{"*"},
-			defaultEncoding: "foo",
+			defaultEncoding: brotliName,
-			expected:        "foo",
+			expected:        brotliName,
 		},
 		{
 			desc:           "follows weight",
 			acceptEncoding: []string{"br;q=0.8, gzip;q=1.0, *;q=0.1"},
 			expected:       gzipName,
 		},
+		{
+			desc:           "identity with higher weight is preferred",
+			acceptEncoding: []string{"br;q=0.8, identity;q=1.0"},
+			expected:       identityName,
+		},
+		{
+			desc:           "identity with equal weight is not preferred",
+			acceptEncoding: []string{"br;q=0.8, identity;q=0.8"},
+			expected:       brotliName,
+		},
 		{
 			desc:           "ignore unknown compression encoding",
 			acceptEncoding: []string{"compress;q=1.0, gzip;q=0.5"},
@@ -93,6 +111,33 @@ func Test_getCompressionEncoding(t *testing.T) {
 			supportedEncodings: []string{gzipName, brotliName},
 			expected:           gzipName,
 		},
+		{
+			desc:           "Zero weights, no compression",
+			acceptEncoding: []string{"br;q=0, gzip;q=0, zstd;q=0"},
+			expected:       notAcceptable,
+		},
+		{
+			desc:            "Zero weights, default encoding, no compression",
+			acceptEncoding:  []string{"br;q=0, gzip;q=0, zstd;q=0"},
+			defaultEncoding: "br",
+			expected:        notAcceptable,
+		},
+		{
+			desc:           "Same weight, first supported encoding",
+			acceptEncoding: []string{"br;q=1.0, gzip;q=1.0, zstd;q=1.0"},
+			expected:       gzipName,
+		},
+		{
+			desc:           "Same weight, first supported encoding, order has no effect",
+			acceptEncoding: []string{"br;q=1.0, zstd;q=1.0, gzip;q=1.0"},
+			expected:       gzipName,
+		},
+		{
+			desc:            "Same weight, first supported encoding, defaultEncoding has no effect",
+			acceptEncoding:  []string{"br;q=1.0, zstd;q=1.0, gzip;q=1.0"},
+			defaultEncoding: "br",
+			expected:        gzipName,
+		},
 	}
 
 	for _, test := range testCases {
@@ -103,7 +148,18 @@ func Test_getCompressionEncoding(t *testing.T) {
 				test.supportedEncodings = defaultSupportedEncodings
 			}
 
-			encoding := getCompressionEncoding(test.acceptEncoding, test.defaultEncoding, test.supportedEncodings)
+			conf := dynamic.Compress{
+				Encodings:       test.supportedEncodings,
+				DefaultEncoding: test.defaultEncoding,
+			}
+
+			h, err := New(context.Background(), nil, conf, "test")
+			require.NoError(t, err)
+
+			c, ok := h.(*compress)
+			require.True(t, ok)
+
+			encoding := c.getCompressionEncoding(test.acceptEncoding)
 
 			assert.Equal(t, test.expected, encoding)
 		})
@@ -147,7 +203,6 @@ func Test_parseAcceptEncoding(t *testing.T) {
 				{Type: zstdName, Weight: 1},
 				{Type: gzipName, Weight: 1},
 				{Type: brotliName, Weight: 1},
-				{Type: wildcardName, Weight: 0},
 			},
 			assertWeight: assert.True,
 		},
@@ -157,7 +212,6 @@ func Test_parseAcceptEncoding(t *testing.T) {
 			supportedEncodings: []string{zstdName},
 			expected: []Encoding{
 				{Type: zstdName, Weight: 1},
-				{Type: wildcardName, Weight: 0},
 			},
 			assertWeight: assert.True,
 		},
@@ -188,7 +242,6 @@ func Test_parseAcceptEncoding(t *testing.T) {
 			expected: []Encoding{
 				{Type: gzipName, Weight: 1},
 				{Type: identityName, Weight: 0.5},
-				{Type: wildcardName, Weight: 0},
 			},
 			assertWeight: assert.True,
 		},
@@ -198,7 +251,6 @@ func Test_parseAcceptEncoding(t *testing.T) {
 			supportedEncodings: []string{"br"},
 			expected: []Encoding{
 				{Type: identityName, Weight: 0.5},
-				{Type: wildcardName, Weight: 0},
 			},
 			assertWeight: assert.True,
 		},
@@ -212,10 +264,11 @@ func Test_parseAcceptEncoding(t *testing.T) {
 				test.supportedEncodings = defaultSupportedEncodings
 			}
 
-			aes, hasWeight := parseAcceptEncoding(test.values, test.supportedEncodings)
+			supportedEncodings := buildSupportedEncodings(test.supportedEncodings)
+
+			aes := parseAcceptableEncodings(test.values, supportedEncodings)
 
 			assert.Equal(t, test.expected, aes)
-			test.assertWeight(t, hasWeight)
 		})
 	}
 }

pkg/middlewares/compress/compress.go

@@ -22,7 +22,7 @@ const typeName = "Compress"
 // See https://github.com/klauspost/compress/blob/9559b037e79ad673c71f6ef7c732c00949014cd2/gzhttp/compress.go#L47.
 const defaultMinSize = 1024
 
-var defaultSupportedEncodings = []string{zstdName, brotliName, gzipName}
+var defaultSupportedEncodings = []string{gzipName, brotliName, zstdName}
 
 // Compress is a middleware that allows to compress the response.
 type compress struct {
@@ -33,6 +33,8 @@ type compress struct {
 	minSize         int
 	encodings       []string
 	defaultEncoding string
+	// supportedEncodings is a map of supported encodings and their priority.
+	supportedEncodings map[string]int
 
 	brotliHandler http.Handler
 	gzipHandler   http.Handler
@@ -85,13 +87,14 @@ func New(ctx context.Context, next http.Handler, conf dynamic.Compress, name str
 	}
 
 	c := &compress{
-		next:            next,
+		next:               next,
-		name:            name,
+		name:               name,
-		excludes:        excludes,
+		excludes:           excludes,
-		includes:        includes,
+		includes:           includes,
-		minSize:         minSize,
+		minSize:            minSize,
-		encodings:       conf.Encodings,
+		encodings:          conf.Encodings,
-		defaultEncoding: conf.DefaultEncoding,
+		defaultEncoding:    conf.DefaultEncoding,
+		supportedEncodings: buildSupportedEncodings(conf.Encodings),
 	}
 
 	var err error
@@ -114,6 +117,19 @@ func New(ctx context.Context, next http.Handler, conf dynamic.Compress, name str
 	return c, nil
 }
 
+func buildSupportedEncodings(encodings []string) map[string]int {
+	supportedEncodings := map[string]int{
+		// the most permissive first.
+		wildcardName: -1,
+		// the less permissive last.
+		identityName: len(encodings),
+	}
+	for i, encoding := range encodings {
+		supportedEncodings[encoding] = i
+	}
+	return supportedEncodings
+}
+
 func (c *compress) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
 	logger := middlewares.GetLogger(req.Context(), c.name, typeName)
 
@@ -149,7 +165,7 @@ func (c *compress) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
 		return
 	}
 
-	c.chooseHandler(getCompressionEncoding(acceptEncoding, c.defaultEncoding, c.encodings), rw, req)
+	c.chooseHandler(c.getCompressionEncoding(acceptEncoding), rw, req)
 }
 
 func (c *compress) chooseHandler(typ string, rw http.ResponseWriter, req *http.Request) {

pkg/middlewares/compress/compress_test.go

@@ -39,9 +39,14 @@ func TestNegotiation(t *testing.T) {
 			expEncoding:     "",
 		},
 		{
+			// In this test, the default encodings are defaulted to gzip, brotli, and zstd,
+			// which make gzip the default encoding, and will be selected.
+			// However, the klauspost/compress gzhttp handler does not compress when Accept-Encoding: * is set.
+			// Until klauspost/compress gzhttp package supports the asterisk,
+			// we will not support it when selecting the gzip encoding.
 			desc:            "accept any header",
 			acceptEncHeader: "*",
-			expEncoding:     brotliName,
+			expEncoding:     "",
 		},
 		{
 			desc:            "gzip accept header",
@@ -66,7 +71,7 @@ func TestNegotiation(t *testing.T) {
 		{
 			desc:            "multi accept header list, prefer br",
 			acceptEncHeader: "gzip, br",
-			expEncoding:     brotliName,
+			expEncoding:     gzipName,
 		},
 		{
 			desc:            "zstd accept header",
@@ -78,15 +83,20 @@ func TestNegotiation(t *testing.T) {
 			acceptEncHeader: "zstd;q=0.9, br;q=0.8, gzip;q=0.6",
 			expEncoding:     zstdName,
 		},
+		{
+			desc:            "multi accept header, prefer brotli",
+			acceptEncHeader: "gzip;q=0.8, br;q=1.0, zstd;q=0.7",
+			expEncoding:     brotliName,
+		},
 		{
 			desc:            "multi accept header, prefer gzip",
 			acceptEncHeader: "gzip;q=1.0, br;q=0.8, zstd;q=0.7",
 			expEncoding:     gzipName,
 		},
 		{
-			desc:            "multi accept header list, prefer zstd",
+			desc:            "multi accept header list, prefer gzip",
 			acceptEncHeader: "gzip, br, zstd",
-			expEncoding:     zstdName,
+			expEncoding:     gzipName,
 		},
 	}
 
@@ -190,6 +200,28 @@ func TestShouldNotCompressWhenNoAcceptEncodingHeader(t *testing.T) {
 	assert.EqualValues(t, rw.Body.Bytes(), fakeBody)
 }
 
+func TestEmptyAcceptEncoding(t *testing.T) {
+	req := testhelpers.MustNewRequest(http.MethodGet, "http://localhost", nil)
+	req.Header.Add(acceptEncodingHeader, "")
+
+	fakeBody := generateBytes(gzhttp.DefaultMinSize)
+	next := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
+		_, err := rw.Write(fakeBody)
+		if err != nil {
+			http.Error(rw, err.Error(), http.StatusInternalServerError)
+		}
+	})
+	handler, err := New(context.Background(), next, dynamic.Compress{Encodings: defaultSupportedEncodings}, "testing")
+	require.NoError(t, err)
+
+	rw := httptest.NewRecorder()
+	handler.ServeHTTP(rw, req)
+
+	assert.Empty(t, rw.Header().Get(contentEncodingHeader))
+	assert.Empty(t, rw.Header().Get(varyHeader))
+	assert.EqualValues(t, rw.Body.Bytes(), fakeBody)
+}
+
 func TestShouldNotCompressWhenIdentityAcceptEncodingHeader(t *testing.T) {
 	req := testhelpers.MustNewRequest(http.MethodGet, "http://localhost", nil)
 	req.Header.Set(acceptEncodingHeader, "identity")