From 03e08ee6a9cfbebed591c765c210c5488b9aefc2 Mon Sep 17 00:00:00 2001 From: "Muhammad Syafri, S.Kom" <105954036+Jualhosting@users.noreply.github.com> Date: Fri, 29 May 2026 02:26:19 +0700 Subject: [PATCH] encode: prioritize zstd and br over gzip in content negotiation (#7772) * fix(encode): prioritize zstd and br over gzip in content negotiation * test(encode): update unit tests to reflect new default priority ties * fix(encode): move default preferences to dynamic encode handler and restore generic negotiation helper * test(encode): call real Provision function in served-response test * test(encode): rename served-response test to TestServeHTTPDefaultEncodingPreference * refactor(encode): use slices.SortStableFunc and httptest.NewRecorder as recommended * refactor(encode): simplify sorting with cmp.Compare and check request error in test * test(encode): fix variable redeclaration in TestServeHTTPDefaultEncodingPreference Fix 'no new variables on left side of :=' error by changing 'err :=' to 'err =' on line 347, since err was already declared on line 332. This fixes the build failure in the encode module tests. --- modules/caddyhttp/encode/encode.go | 18 ++++-- modules/caddyhttp/encode/encode_test.go | 75 +++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 5 deletions(-) diff --git a/modules/caddyhttp/encode/encode.go b/modules/caddyhttp/encode/encode.go index 3b6522745..22c9e7abf 100644 --- a/modules/caddyhttp/encode/encode.go +++ b/modules/caddyhttp/encode/encode.go @@ -20,12 +20,12 @@ package encode import ( + "cmp" "fmt" "io" "math" "net/http" "slices" - "sort" "strconv" "strings" "sync" @@ -127,6 +127,14 @@ func (enc *Encode) Provision(ctx caddy.Context) error { } } + if len(enc.Prefer) == 0 { + for _, encName := range []string{"zstd", "br", "gzip"} { + if _, ok := enc.writerPools[encName]; ok { + enc.Prefer = append(enc.Prefer, encName) + } + } + } + return nil } @@ -538,11 +546,11 @@ func AcceptedEncodings(r *http.Request, preferredOrder []string) []string { } // sort preferences by descending q-factor first, then by preferOrder - sort.Slice(prefs, func(i, j int) bool { - if math.Abs(prefs[i].q-prefs[j].q) < 0.00001 { - return prefs[i].preferOrder > prefs[j].preferOrder + slices.SortStableFunc(prefs, func(a, b encodingPreference) int { + if math.Abs(a.q-b.q) < 0.00001 { + return cmp.Compare(b.preferOrder, a.preferOrder) } - return prefs[i].q > prefs[j].q + return cmp.Compare(b.q, a.q) }) prefEncNames := make([]string, len(prefs)) diff --git a/modules/caddyhttp/encode/encode_test.go b/modules/caddyhttp/encode/encode_test.go index 818f76745..0f306777f 100644 --- a/modules/caddyhttp/encode/encode_test.go +++ b/modules/caddyhttp/encode/encode_test.go @@ -1,10 +1,16 @@ package encode import ( + "context" + "io" "net/http" + "net/http/httptest" "slices" "sync" "testing" + + "github.com/caddyserver/caddy/v2" + "github.com/caddyserver/caddy/v2/modules/caddyhttp" ) func BenchmarkOpenResponseWriter(b *testing.B) { @@ -295,3 +301,72 @@ func TestIsEncodeAllowed(t *testing.T) { }) } } + +type mockEncoder struct{} + +func (mockEncoder) Write(p []byte) (n int, err error) { return len(p), nil } +func (mockEncoder) Close() error { return nil } +func (mockEncoder) Reset(w io.Writer) {} +func (mockEncoder) Flush() error { return nil } + +func TestServeHTTPDefaultEncodingPreference(t *testing.T) { + enc := new(Encode) + enc.MinLength = 1 // compress everything + enc.writerPools = map[string]*sync.Pool{ + "gzip": { + New: func() any { return mockEncoder{} }, + }, + "zstd": { + New: func() any { return mockEncoder{} }, + }, + } + + // Call Provision() with a valid caddy.Context to exercise the real path + ctx, cancel := caddy.NewContext(caddy.Context{Context: context.Background()}) + defer cancel() + if err := enc.Provision(ctx); err != nil { + t.Fatalf("Provision failed: %v", err) + } + + // Test default preference: zstd preferred over gzip + r, err := http.NewRequest("GET", "/", nil) + if err != nil { + t.Fatalf("error creating request: %v", err) + } + r.Header.Set("Accept-Encoding", "gzip, deflate, br, zstd") + + w := httptest.NewRecorder() + w.Header().Set("Content-Type", "text/plain") + + next := caddyhttp.HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { + w.WriteHeader(http.StatusOK) + _, err := w.Write([]byte("Hello, world! This is a long enough string to satisfy min length if it wasn't 1.")) + return err + }) + + err = enc.ServeHTTP(w, r, next) + if err != nil { + t.Fatalf("ServeHTTP returned error: %v", err) + } + + // ETag suffix or Content-Encoding header should reflect zstd + contentEncoding := w.Header().Get("Content-Encoding") + if contentEncoding != "zstd" { + t.Errorf("Expected Content-Encoding to be 'zstd' by default, got '%s'", contentEncoding) + } + + // Test explicit user preference: gzip over zstd + enc.Prefer = []string{"gzip", "zstd"} + + w2 := httptest.NewRecorder() + w2.Header().Set("Content-Type", "text/plain") + err = enc.ServeHTTP(w2, r, next) + if err != nil { + t.Fatalf("ServeHTTP returned error: %v", err) + } + + contentEncoding2 := w2.Header().Get("Content-Encoding") + if contentEncoding2 != "gzip" { + t.Errorf("Expected Content-Encoding to be 'gzip' when explicitly preferred, got '%s'", contentEncoding2) + } +}