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.pull/7783/head
parent
86121c860f
commit
03e08ee6a9
|
|
@ -20,12 +20,12 @@
|
||||||
package encode
|
package encode
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"cmp"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
"math"
|
"math"
|
||||||
"net/http"
|
"net/http"
|
||||||
"slices"
|
"slices"
|
||||||
"sort"
|
|
||||||
"strconv"
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
"sync"
|
"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
|
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 preferences by descending q-factor first, then by preferOrder
|
||||||
sort.Slice(prefs, func(i, j int) bool {
|
slices.SortStableFunc(prefs, func(a, b encodingPreference) int {
|
||||||
if math.Abs(prefs[i].q-prefs[j].q) < 0.00001 {
|
if math.Abs(a.q-b.q) < 0.00001 {
|
||||||
return prefs[i].preferOrder > prefs[j].preferOrder
|
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))
|
prefEncNames := make([]string, len(prefs))
|
||||||
|
|
|
||||||
|
|
@ -1,10 +1,16 @@
|
||||||
package encode
|
package encode
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"context"
|
||||||
|
"io"
|
||||||
"net/http"
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
"slices"
|
"slices"
|
||||||
"sync"
|
"sync"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
|
"github.com/caddyserver/caddy/v2"
|
||||||
|
"github.com/caddyserver/caddy/v2/modules/caddyhttp"
|
||||||
)
|
)
|
||||||
|
|
||||||
func BenchmarkOpenResponseWriter(b *testing.B) {
|
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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue