From e995f25851880a0865d9f5ecfc2be07ad7a2a38f Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Fri, 5 Jun 2026 11:11:46 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=93=9D=20CodeRabbit=20Chat:=20Implement?= =?UTF-8?q?=20requested=20code=20changes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/handler/static.go | 6 +- internal/handler/static_test.go | 115 ++++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 2 deletions(-) create mode 100644 internal/handler/static_test.go diff --git a/internal/handler/static.go b/internal/handler/static.go index b5fc6744..8eb011b3 100644 --- a/internal/handler/static.go +++ b/internal/handler/static.go @@ -197,8 +197,10 @@ func serveFromCache(w http.ResponseWriter, r *http.Request, cached *staticFileCa // Set content type w.Header().Set("Content-Type", cached.contentType) - // Always set Vary header to ensure caches differentiate by Accept-Encoding - w.Header().Set("Vary", "Accept-Encoding") + // Add Accept-Encoding to Vary so caches differentiate compressed from plain + // responses. Use Add (not Set) to preserve any Vary values already written + // by upstream middleware (e.g. the CORS middleware adds "Vary: Origin"). + w.Header().Add("Vary", "Accept-Encoding") // Check if client accepts gzip and we have gzipped content if cached.gzipped != nil && strings.Contains(r.Header.Get("Accept-Encoding"), "gzip") { diff --git a/internal/handler/static_test.go b/internal/handler/static_test.go new file mode 100644 index 00000000..1eb12fd7 --- /dev/null +++ b/internal/handler/static_test.go @@ -0,0 +1,115 @@ +package handler + +import ( + "compress/gzip" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// TestServeFromCache_GzipPathSetsContentEncoding ensures that when a gzip-capable +// client requests a file for which we have pre-compressed bytes, the response +// carries Content-Encoding: gzip and the body is the compressed bytes (not the +// raw content). This is a regression guard: a previous proposed fix accidentally +// dropped the Content-Encoding header while changing the Vary strategy. +func TestServeFromCache_GzipPathSetsContentEncoding(t *testing.T) { + // Build a cache entry whose gzipped field is non-nil. + // buildCacheEntry only pre-compresses content > 1024 bytes of a + // compressible MIME type, so use a large JS payload. + content := []byte(strings.Repeat("function hello(){return 42;}\n", 50)) + cached := buildCacheEntry("assets/app-abc123.js", content) + if cached.gzipped == nil { + t.Skip("pre-compression did not produce a gzipped entry; check buildCacheEntry thresholds") + } + + req := httptest.NewRequest(http.MethodGet, "/assets/app-abc123.js", nil) + req.Header.Set("Accept-Encoding", "gzip, deflate") + rec := httptest.NewRecorder() + serveFromCache(rec, req, cached) + + resp := rec.Result() + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %d, want 200", resp.StatusCode) + } + if got := resp.Header.Get("Content-Encoding"); got != "gzip" { + t.Fatalf("Content-Encoding = %q, want \"gzip\"", got) + } + + // The body must be valid gzip that decompresses to the original content. + gr, err := gzip.NewReader(resp.Body) + if err != nil { + t.Fatalf("response body is not valid gzip: %v", err) + } + defer gr.Close() + got, err := io.ReadAll(gr) + if err != nil { + t.Fatalf("reading decompressed body: %v", err) + } + if string(got) != string(content) { + t.Fatalf("decompressed body mismatch: got %d bytes, want %d bytes", len(got), len(content)) + } +} + +// TestServeFromCache_VaryPreservesExistingValues verifies that serveFromCache +// uses Add (not Set) for the Vary header so that values written by upstream +// middleware (such as the CORS middleware's "Vary: Origin") are not clobbered. +func TestServeFromCache_VaryPreservesExistingValues(t *testing.T) { + content := []byte("hi") + cached := buildCacheEntry("index.html", content) + + req := httptest.NewRequest(http.MethodGet, "/", nil) + rec := httptest.NewRecorder() + // Simulate CORS middleware having already written Vary: Origin before the + // static handler runs. + rec.Header().Add("Vary", "Origin") + + serveFromCache(rec, req, cached) + + vary := rec.Header().Values("Vary") + hasOrigin := false + hasEncoding := false + for _, v := range vary { + if v == "Origin" { + hasOrigin = true + } + if v == "Accept-Encoding" { + hasEncoding = true + } + } + if !hasOrigin { + t.Fatalf("Vary header lost upstream \"Origin\" value; got %v", vary) + } + if !hasEncoding { + t.Fatalf("Vary header missing \"Accept-Encoding\"; got %v", vary) + } +} + +// TestServeFromCache_NonGzipClientGetsUncompressed verifies that a client that +// does not advertise gzip support receives the uncompressed content without a +// Content-Encoding header, even when a pre-compressed version is available. +func TestServeFromCache_NonGzipClientGetsUncompressed(t *testing.T) { + content := []byte(strings.Repeat("function hello(){return 42;}\n", 50)) + cached := buildCacheEntry("assets/app-abc123.js", content) + if cached.gzipped == nil { + t.Skip("pre-compression not available; check buildCacheEntry thresholds") + } + + req := httptest.NewRequest(http.MethodGet, "/assets/app-abc123.js", nil) + // No Accept-Encoding: gzip + rec := httptest.NewRecorder() + serveFromCache(rec, req, cached) + + resp := rec.Result() + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %d, want 200", resp.StatusCode) + } + if got := resp.Header.Get("Content-Encoding"); got != "" { + t.Fatalf("Content-Encoding = %q, want empty for non-gzip client", got) + } + body, _ := io.ReadAll(resp.Body) + if string(body) != string(content) { + t.Fatalf("body mismatch: got %d bytes, want %d bytes", len(body), len(content)) + } +}