From 1d8468afb3d80470b065320b9de9eef73b13a9cd Mon Sep 17 00:00:00 2001 From: Bart Venter <72999113+bartventer@users.noreply.github.com> Date: Thu, 12 Mar 2026 18:04:42 +0000 Subject: [PATCH 1/3] fix(cache): include all `Vary` header lines in normalized key Closes #32 --- internal/responsestorerer.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/responsestorerer.go b/internal/responsestorerer.go index eb92b1d..6ba21f1 100644 --- a/internal/responsestorerer.go +++ b/internal/responsestorerer.go @@ -18,6 +18,7 @@ import ( "maps" "net/http" "slices" + "strings" "time" ) @@ -59,7 +60,7 @@ func (r *responseStorer) StoreResponse( // Remove hop-by-hop headers as per RFC 9111 §3.1 removeHopByHopHeaders(resp) - vary := resp.Header.Get("Vary") + vary := strings.Join(resp.Header.Values("Vary"), ",") // Join multiple Vary headers into a single string varyResolved := maps.Collect( r.vhn.NormalizeVaryHeader(vary, req.Header), ) From a1d18b4043728931867b33faab8a35c3bef4caf0 Mon Sep 17 00:00:00 2001 From: Bart Venter <72999113+bartventer@users.noreply.github.com> Date: Thu, 12 Mar 2026 18:05:54 +0000 Subject: [PATCH 2/3] test: add regression test for handling multiple Vary headers Closes #32 --- roundtripper_test.go | 72 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/roundtripper_test.go b/roundtripper_test.go index 1756dff..7b9df85 100644 --- a/roundtripper_test.go +++ b/roundtripper_test.go @@ -16,6 +16,7 @@ package httpcache import ( "errors" + "fmt" "io" "log/slog" "net/http" @@ -957,3 +958,74 @@ func Test_transport_RevalidationUpdatesCache(t *testing.T) { }) } } + +// Test_transport_Vary_MultipleHeaders verifies that the transport correctly +// handles multiple Vary headers that are sent as separate header lines +// (instead of a single comma-separated line). This is a common scenario in +// practice, and there was a bug where only the first Vary header line was +// being processed, causing incorrect cache hits when subsequent Vary headers +// were not considered. +// Regression test for: https://github.com/bartventer/httpcache/issues/32 +func Test_transport_Vary_MultipleHeaders(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Simulate server returning multiple separate Vary header lines + // (not comma-separated, but as distinct headers) + w.Header().Add("Vary", "Accept-Language") + w.Header().Add("Vary", "X-Compatibility-Date") + w.Header().Set("Cache-Control", "max-age=60") + w.WriteHeader(http.StatusOK) + + lang := r.Header.Get("Accept-Language") + date := r.Header.Get("X-Compatibility-Date") + _, _ = fmt.Fprintf(w, "lang=%s date=%s", lang, date) + })) + defer server.Close() + + c := memcache.Open() + tr := newTransport(c, WithLogger(slog.New(slog.NewJSONHandler(os.Stderr, &slog.HandlerOptions{ + Level: slog.LevelDebug, + })))) + + makeReq := func(lang, date string) *http.Response { + req, _ := http.NewRequest(http.MethodGet, server.URL, nil) + req.Header.Set("Accept-Language", lang) + req.Header.Set("X-Compatibility-Date", date) + resp, err := tr.RoundTrip(req) + testutil.RequireNoError(t, err) + return resp + } + + // Request 1: MISS — first request for en-us + 2025-01-01 + resp := makeReq("en-us", "2025-01-01") + testutil.AssertEqual(t, http.StatusOK, resp.StatusCode) + testutil.AssertEqual(t, internal.CacheStatusMiss.Value, resp.Header.Get(internal.CacheStatusHeader)) + body, _ := io.ReadAll(resp.Body) + _ = resp.Body.Close() + testutil.AssertEqual(t, "lang=en-us date=2025-01-01", string(body)) + + // Request 2: HIT — same headers + resp = makeReq("en-us", "2025-01-01") + testutil.AssertEqual(t, http.StatusOK, resp.StatusCode) + testutil.AssertEqual(t, internal.CacheStatusHit.Value, resp.Header.Get(internal.CacheStatusHeader)) + body, _ = io.ReadAll(resp.Body) + _ = resp.Body.Close() + testutil.AssertEqual(t, "lang=en-us date=2025-01-01", string(body)) + + // Request 3: MISS — X-Compatibility-Date changed, must be a different cache entry + // This is the bug: without the fix, this incorrectly returns a HIT + resp = makeReq("en-us", "2026-01-01") + testutil.AssertEqual(t, http.StatusOK, resp.StatusCode) + testutil.AssertEqual(t, internal.CacheStatusMiss.Value, resp.Header.Get(internal.CacheStatusHeader), + "X-Compatibility-Date is a Vary header; different value must produce a cache MISS") + body, _ = io.ReadAll(resp.Body) + _ = resp.Body.Close() + testutil.AssertEqual(t, "lang=en-us date=2026-01-01", string(body)) + + // Request 4: HIT — same as request 3 + resp = makeReq("en-us", "2026-01-01") + testutil.AssertEqual(t, http.StatusOK, resp.StatusCode) + testutil.AssertEqual(t, internal.CacheStatusHit.Value, resp.Header.Get(internal.CacheStatusHeader)) + body, _ = io.ReadAll(resp.Body) + _ = resp.Body.Close() + testutil.AssertEqual(t, "lang=en-us date=2026-01-01", string(body)) +} From 0bc62ab8d45fa496e567d270b4ac947a9d889024 Mon Sep 17 00:00:00 2001 From: Bart Venter <72999113+bartventer@users.noreply.github.com> Date: Thu, 12 Mar 2026 18:13:19 +0000 Subject: [PATCH 3/3] chore: fix linting issues --- internal/responsestorerer.go | 3 ++- roundtripper_test.go | 26 +++++++++++++++++++++----- store/expapi/expapi.go | 12 ++++++++++-- 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/internal/responsestorerer.go b/internal/responsestorerer.go index 6ba21f1..d442a01 100644 --- a/internal/responsestorerer.go +++ b/internal/responsestorerer.go @@ -60,7 +60,8 @@ func (r *responseStorer) StoreResponse( // Remove hop-by-hop headers as per RFC 9111 §3.1 removeHopByHopHeaders(resp) - vary := strings.Join(resp.Header.Values("Vary"), ",") // Join multiple Vary headers into a single string + // Join multiple Vary headers into a single string + vary := strings.Join(resp.Header.Values("Vary"), ",") varyResolved := maps.Collect( r.vhn.NormalizeVaryHeader(vary, req.Header), ) diff --git a/roundtripper_test.go b/roundtripper_test.go index 7b9df85..7543431 100644 --- a/roundtripper_test.go +++ b/roundtripper_test.go @@ -998,7 +998,11 @@ func Test_transport_Vary_MultipleHeaders(t *testing.T) { // Request 1: MISS — first request for en-us + 2025-01-01 resp := makeReq("en-us", "2025-01-01") testutil.AssertEqual(t, http.StatusOK, resp.StatusCode) - testutil.AssertEqual(t, internal.CacheStatusMiss.Value, resp.Header.Get(internal.CacheStatusHeader)) + testutil.AssertEqual( + t, + internal.CacheStatusMiss.Value, + resp.Header.Get(internal.CacheStatusHeader), + ) body, _ := io.ReadAll(resp.Body) _ = resp.Body.Close() testutil.AssertEqual(t, "lang=en-us date=2025-01-01", string(body)) @@ -1006,7 +1010,11 @@ func Test_transport_Vary_MultipleHeaders(t *testing.T) { // Request 2: HIT — same headers resp = makeReq("en-us", "2025-01-01") testutil.AssertEqual(t, http.StatusOK, resp.StatusCode) - testutil.AssertEqual(t, internal.CacheStatusHit.Value, resp.Header.Get(internal.CacheStatusHeader)) + testutil.AssertEqual( + t, + internal.CacheStatusHit.Value, + resp.Header.Get(internal.CacheStatusHeader), + ) body, _ = io.ReadAll(resp.Body) _ = resp.Body.Close() testutil.AssertEqual(t, "lang=en-us date=2025-01-01", string(body)) @@ -1015,8 +1023,12 @@ func Test_transport_Vary_MultipleHeaders(t *testing.T) { // This is the bug: without the fix, this incorrectly returns a HIT resp = makeReq("en-us", "2026-01-01") testutil.AssertEqual(t, http.StatusOK, resp.StatusCode) - testutil.AssertEqual(t, internal.CacheStatusMiss.Value, resp.Header.Get(internal.CacheStatusHeader), - "X-Compatibility-Date is a Vary header; different value must produce a cache MISS") + testutil.AssertEqual( + t, + internal.CacheStatusMiss.Value, + resp.Header.Get(internal.CacheStatusHeader), + "X-Compatibility-Date is a Vary header; different value must produce a cache MISS", + ) body, _ = io.ReadAll(resp.Body) _ = resp.Body.Close() testutil.AssertEqual(t, "lang=en-us date=2026-01-01", string(body)) @@ -1024,7 +1036,11 @@ func Test_transport_Vary_MultipleHeaders(t *testing.T) { // Request 4: HIT — same as request 3 resp = makeReq("en-us", "2026-01-01") testutil.AssertEqual(t, http.StatusOK, resp.StatusCode) - testutil.AssertEqual(t, internal.CacheStatusHit.Value, resp.Header.Get(internal.CacheStatusHeader)) + testutil.AssertEqual( + t, + internal.CacheStatusHit.Value, + resp.Header.Get(internal.CacheStatusHeader), + ) body, _ = io.ReadAll(resp.Body) _ = resp.Body.Close() testutil.AssertEqual(t, "lang=en-us date=2026-01-01", string(body)) diff --git a/store/expapi/expapi.go b/store/expapi/expapi.go index 2a3d542..cc4cfb7 100644 --- a/store/expapi/expapi.go +++ b/store/expapi/expapi.go @@ -98,7 +98,11 @@ func retrieve(conn driver.Conn) http.Handler { if errors.Is(err, driver.ErrNotExist) { http.Error(w, fmt.Sprintf("key %q not found", key), http.StatusNotFound) } else { - http.Error(w, fmt.Sprintf("failed to get value for key %q: %v", key, err), http.StatusInternalServerError) + http.Error( + w, + fmt.Sprintf("failed to get value for key %q: %v", key, err), + http.StatusInternalServerError, + ) } return } @@ -121,7 +125,11 @@ func destroy(conn driver.Conn) http.Handler { if errors.Is(err, driver.ErrNotExist) { http.Error(w, fmt.Sprintf("key %q not found", key), http.StatusNotFound) } else { - http.Error(w, fmt.Sprintf("failed to delete value for key %q: %v", key, err), http.StatusInternalServerError) + http.Error( + w, + fmt.Sprintf("failed to delete value for key %q: %v", key, err), + http.StatusInternalServerError, + ) } return }