amadan
edge: the cache reads every header line, and remembers only the latest Vary
RFC 9110 §5.3 lets one header field arrive on several lines, meaning exactly what the comma-joined single line means — and that is what http.Header.Add produces, so layered middleware emits it constantly: compression adds Vary: Accept-Encoding, the session layer separately adds Vary: Cookie. The edge read both Vary and Cache-Control with Header.Get, which returns only the FIRST line. Everything after it was invisible. So Vary: Cookie on a second line never fired the refusal, and the entry was keyed on Accept-Encoding alone: visitor bob was served visitor alice's cookie-scoped page. And Cache-Control: private on a second line never fired either — the private response was cached at the shared edge, the very leak the private rule exists to close. One joinHeader helper now answers every Cache-Control and Vary question in the file, the one joined value feeding both of each field's consumers; varyValue reads the request's lines the same way. The third bug was the cache remembering a URL's Vary names write-once. Nothing cleared the record when a later response for that URL carried no Vary at all, so such a response was stored under the empty variant while every lookup kept resolving against names nothing was stored under. Any app that varies conditionally — CORS emitting Vary: Origin only for Origin-bearing requests — could be poisoned permanently by ONE unauthenticated request on a cold URL: 10/10 ordinary requests missed afterwards, and it never self-healed, because the unreachable entry was re-put on every miss and so kept the URL's group non-empty. On a hibernating route every miss is a wake, which makes it a remote wake-storm — precisely the denial this whole feature exists to prevent. Put now takes the response's Vary value explicitly (a parameter, because VaryValue == "" cannot stand in for "the response had no Vary", and because the string that keys the entry must be the same string that gets remembered). It records the latest scheme, clears it when a URL stops varying, and drops the variants stored under the old scheme — unreachable by construction once the scheme changes, and otherwise a slow leak holding LRU space forever. Each of the three had a test that should have caught it and didn't. The new ones assert what the old ones couldn't: variant COUNTS rather than an empty-key Get that misses either way, one visitor's body never reaching another, and backend invocation counts across ten requests.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
6 files changed,
+305
−32
internal/activator/activator_test.go+1 −1internal/edge/cache.go+49 −14internal/edge/cache_test.go+58 −10internal/edge/edge.go+29 −4internal/edge/edge_test.go+166 −1internal/edge/purge_test.go+2 −2
diff --git a/internal/activator/activator_test.go b/internal/activator/activator_test.go| index 36fb7eebe..8b48b26c0 100644 |
| --- a/internal/activator/activator_test.go |
| +++ b/internal/activator/activator_test.go |
| @@ -2074,7 +2074,7 @@ func TestExecBackendStartPurgesHostCacheOnSpawn(t *testing.T) { |
| stale := edge.CacheKey{Host: "deploy.example.com", Method: "GET", Path: "/event"} |
| otherHost := edge.CacheKey{Host: "bystander.example.com", Method: "GET", Path: "/event"} |
| for _, k := range []edge.CacheKey{stale, otherHost} { |
| - cache.Put(k, &edge.CachedResponse{ |
| + cache.Put(k, "", &edge.CachedResponse{ |
| StatusCode: 200, Header: http.Header{}, Body: []byte("old html"), |
| StoredAt: time.Now(), TTL: time.Minute, |
| }) |
diff --git a/internal/edge/cache.go b/internal/edge/cache.go| index 189ec4952..ed4f07c47 100644 |
| --- a/internal/edge/cache.go |
| +++ b/internal/edge/cache.go |
| @@ -60,12 +60,22 @@ type Cache struct { |
| entries map[cacheFullKey]*list.Element |
| byURL map[cacheURLKey]map[cacheFullKey]*list.Element // urlKey -> its Vary variants |
| byPath map[cachePathKey]map[cacheFullKey]*list.Element // pathKey -> every entry under that path |
| - // varyNames records, per URL, the raw Vary header value the most recent |
| - // Vary-carrying response for that URL asked to vary by. It is what makes |
| - // the two-phase lookup possible: at request time the response's Vary |
| - // header isn't known yet, so GetFor replays the last one seen to compute |
| - // the same VaryValue Put stored the entry under. Dropped when a URL's last |
| - // entry goes away, so it can't grow without bound. |
| + // varyNames records, per URL, the raw Vary header value of the most recent |
| + // response stored for that URL. It is what makes the two-phase lookup |
| + // possible: at request time the response's Vary header isn't known yet, so |
| + // GetFor replays the last one seen to compute the same VaryValue Put |
| + // stored the entry under. Dropped when a URL's last entry goes away, so it |
| + // can't grow without bound. |
| + // |
| + // "Most recent", emphatically, not "first ever". A URL whose responses |
| + // vary conditionally (CORS middleware emitting Vary: Origin only for |
| + // Origin-bearing requests) would otherwise be poisoned by a single |
| + // request: the remembered names outlive the response that set them, later |
| + // non-varying responses are stored under the empty variant, and every |
| + // lookup keeps resolving against names nothing is stored under — a |
| + // permanent miss on that URL, which on a hibernating route is a permanent |
| + // wake per request. Put rewrites this record on every store, so a URL that |
| + // stops varying stops being resolved against stale names. |
| varyNames map[cacheURLKey]string |
| } |
| @@ -142,8 +152,9 @@ func (c *Cache) Get(key CacheKey) (*CachedResponse, bool) { |
| // left empty. |
| // |
| // The Vary header names live on the RESPONSE, so at lookup time the only |
| -// honest source for them is what the last Vary-carrying response for this URL |
| -// said (c.varyNames). With none on record the lookup uses an empty VaryValue, |
| +// honest source for them is what the last response stored for this URL said |
| +// (c.varyNames). With none on record — no response yet, or the last one |
| +// carried no Vary — the lookup uses an empty VaryValue, |
| // which is exactly the no-Vary case and keeps that path byte-for-byte as it |
| // was. Both this and maybeStore compute the variant through varyValue, so |
| // store and lookup can never drift apart — the bug this method exists to fix |
| @@ -164,9 +175,18 @@ func (c *Cache) GetFor(baseKey CacheKey, r *http.Request) (*CachedResponse, bool |
| // maxEntries, and enforces the per-URL and per-path admission caps so no |
| // single URL or path can flood the cache (see the cap constants above). |
| // |
| -// A resp whose Header carries Vary also records those header names for this |
| -// URL, so GetFor can resolve the same variant on the next request. |
| -func (c *Cache) Put(key CacheKey, resp *CachedResponse) { |
| +// vary is the response's Vary field value — every line of it, comma-joined — |
| +// and "" when the response carried no Vary header at all. It is a separate |
| +// parameter rather than something read back off resp.Header because the caller |
| +// must have computed key.VaryValue from this exact string (edge.go's maybeStore |
| +// does, via varyValue), and because key.VaryValue == "" cannot stand in for |
| +// "no Vary": a response may vary on a header the request never sent, which |
| +// computes an empty value from a very real Vary header. |
| +// |
| +// Put records vary as this URL's Vary scheme so GetFor can resolve the same |
| +// variant on the next request, and — this is the load-bearing half — clears |
| +// or replaces that record when vary changes, including changing to "". |
| +func (c *Cache) Put(key CacheKey, vary string, resp *CachedResponse) { |
| if len(resp.Body) > c.maxBodyBytes { |
| return |
| } |
| @@ -174,9 +194,24 @@ func (c *Cache) Put(key CacheKey, resp *CachedResponse) { |
| defer c.mu.Unlock() |
| fk := key.fullKey() |
| uk := fk.cacheURLKey |
| - if resp.Header != nil { |
| - if names := resp.Header.Get("Vary"); names != "" { |
| - c.varyNames[uk] = names |
| + if vary != c.varyNames[uk] { |
| + // This URL's Vary scheme changed: it stopped varying, started, or |
| + // started varying by different headers. Every entry already stored for |
| + // it was keyed under the OLD scheme, while GetFor will now resolve |
| + // against the new one — none of them can ever be reached again, so |
| + // they are pure garbage holding LRU and per-URL-cap space. Dropping |
| + // them is what keeps a conditionally-varying URL from slowly filling |
| + // the cache with unreachable variants. |
| + // |
| + // The sweep runs before the record is rewritten because removeLocked |
| + // deletes varyNames[uk] once the URL's last entry goes. |
| + for _, el := range elementsOf(c.byURL[uk]) { |
| + c.removeLocked(el) |
| + } |
| + if vary == "" { |
| + delete(c.varyNames, uk) |
| + } else { |
| + c.varyNames[uk] = vary |
| } |
| } |
| if el, ok := c.entries[fk]; ok { |
diff --git a/internal/edge/cache_test.go b/internal/edge/cache_test.go| index 71bf388e8..a3b52d633 100644 |
| --- a/internal/edge/cache_test.go |
| +++ b/internal/edge/cache_test.go |
| @@ -18,7 +18,7 @@ func TestCachePutThenGetHits(t *testing.T) { |
| c := NewCache(10, 1<<20) |
| key := CacheKey{Host: "a", Method: "GET", Path: "/event"} |
| resp := &CachedResponse{StatusCode: 200, Header: http.Header{}, Body: []byte("hi"), StoredAt: time.Now(), TTL: time.Minute} |
| - c.Put(key, resp) |
| + c.Put(key, "", resp) |
| got, ok := c.Get(key) |
| if !ok { |
| t.Fatal("expected hit after put") |
| @@ -32,7 +32,7 @@ func TestCacheEntryExpiresAfterTTL(t *testing.T) { |
| c := NewCache(10, 1<<20) |
| key := CacheKey{Host: "a", Method: "GET", Path: "/event"} |
| resp := &CachedResponse{StatusCode: 200, Header: http.Header{}, Body: []byte("hi"), StoredAt: time.Now().Add(-2 * time.Minute), TTL: time.Minute} |
| - c.Put(key, resp) |
| + c.Put(key, "", resp) |
| if _, ok := c.Get(key); ok { |
| t.Fatal("expected miss on expired entry") |
| } |
| @@ -42,7 +42,7 @@ func TestCachePutOverMaxBodyBytesIsDropped(t *testing.T) { |
| c := NewCache(10, 4) // 4-byte cap |
| key := CacheKey{Host: "a", Method: "GET", Path: "/event"} |
| resp := &CachedResponse{StatusCode: 200, Header: http.Header{}, Body: []byte("way too big"), StoredAt: time.Now(), TTL: time.Minute} |
| - c.Put(key, resp) |
| + c.Put(key, "", resp) |
| if _, ok := c.Get(key); ok { |
| t.Fatal("expected oversized body to never be cached") |
| } |
| @@ -52,7 +52,7 @@ func TestCacheEvictsLeastRecentlyUsedAtCapacity(t *testing.T) { |
| c := NewCache(2, 1<<20) |
| mk := func(p string) CacheKey { return CacheKey{Host: "a", Method: "GET", Path: p} } |
| put := func(p string) { |
| - c.Put(mk(p), &CachedResponse{StatusCode: 200, Header: http.Header{}, Body: []byte(p), StoredAt: time.Now(), TTL: time.Minute}) |
| + c.Put(mk(p), "", &CachedResponse{StatusCode: 200, Header: http.Header{}, Body: []byte(p), StoredAt: time.Now(), TTL: time.Minute}) |
| } |
| put("/one") |
| put("/two") |
| @@ -69,6 +69,54 @@ func TestCacheEvictsLeastRecentlyUsedAtCapacity(t *testing.T) { |
| } |
| } |
| +// varyOf reports the Vary field value the response behind k must have carried: |
| +// a key only has a VaryValue because its response varied by something, and Put |
| +// needs to be told which something. |
| +func varyOf(k CacheKey) string { |
| + if k.VaryValue == "" { |
| + return "" |
| + } |
| + return "Accept-Language" |
| +} |
| + |
| +// Put records the Vary scheme of the LATEST response for a URL, not the first |
| +// one ever seen. When a URL stops varying, the old per-variant entries are |
| +// unreachable — GetFor resolves against the new (empty) scheme — so they must |
| +// go rather than sit in the LRU forever, and the remembered names must go with |
| +// them or every later lookup resolves against names nothing is stored under. |
| +func TestCachePutWithoutVaryClearsRememberedNamesAndStaleVariants(t *testing.T) { |
| + c := NewCache(10, 1<<20) |
| + uk := CacheKey{Host: "a", Method: "GET", Path: "/page"}.urlKey() |
| + mkResp := func(b string) *CachedResponse { |
| + return &CachedResponse{StatusCode: 200, Header: http.Header{}, Body: []byte(b), StoredAt: time.Now(), TTL: time.Minute} |
| + } |
| + |
| + varied := CacheKey{Host: "a", Method: "GET", Path: "/page", VaryValue: "Origin=https://x;"} |
| + c.Put(varied, "Origin", mkResp("cors")) |
| + c.mu.Lock() |
| + names := c.varyNames[uk] |
| + c.mu.Unlock() |
| + if names != "Origin" { |
| + t.Fatalf("varyNames = %q after a Vary-carrying store, want %q", names, "Origin") |
| + } |
| + |
| + plain := CacheKey{Host: "a", Method: "GET", Path: "/page"} |
| + c.Put(plain, "", mkResp("plain")) |
| + c.mu.Lock() |
| + names, remembered := c.varyNames[uk] |
| + variants := len(c.byURL[uk]) |
| + c.mu.Unlock() |
| + if remembered { |
| + t.Fatalf("varyNames still holds %q after a response with no Vary — every later lookup would resolve against names nothing is stored under", names) |
| + } |
| + if variants != 1 { |
| + t.Fatalf("URL holds %d entries, want 1 — the old Origin variant is unreachable once the URL stops varying and must not linger in the LRU", variants) |
| + } |
| + if got, ok := c.Get(plain); !ok || string(got.Body) != "plain" { |
| + t.Fatalf("Get(no-Vary key) = %v, %v; want the freshly stored body", got, ok) |
| + } |
| +} |
| + |
| // A deploy replaces the code behind a host, so every cached page for it is |
| // suspect — PurgeHost drops all of them, across paths and Vary variants, and |
| // nothing belonging to any other host. |
| @@ -85,10 +133,10 @@ func TestCachePurgeHostRemovesEveryEntryForThatHost(t *testing.T) { |
| {Host: "a", Method: "GET", Path: "/one", RawQuery: "page=2"}, |
| } |
| for _, k := range keys { |
| - c.Put(k, mkResp(k.Path)) |
| + c.Put(k, varyOf(k), mkResp(k.Path)) |
| } |
| other := CacheKey{Host: "b", Method: "GET", Path: "/one"} |
| - c.Put(other, mkResp("b/one")) |
| + c.Put(other, "", mkResp("b/one")) |
| if n := c.PurgeHost("a"); n != len(keys) { |
| t.Fatalf("PurgeHost removed %d entries, want %d", n, len(keys)) |
| @@ -113,7 +161,7 @@ func TestCacheQueryStringFloodIsBoundedPerPath(t *testing.T) { |
| c := NewCache(1000, 1<<20) |
| for i := 0; i < 10*maxEntriesPerPath; i++ { |
| k := CacheKey{Host: "a", Method: "GET", Path: "/event", RawQuery: fmt.Sprintf("x=%d", i)} |
| - c.Put(k, &CachedResponse{StatusCode: 200, Header: http.Header{}, Body: []byte("x"), StoredAt: time.Now(), TTL: time.Minute}) |
| + c.Put(k, "", &CachedResponse{StatusCode: 200, Header: http.Header{}, Body: []byte("x"), StoredAt: time.Now(), TTL: time.Minute}) |
| } |
| c.mu.Lock() |
| n := len(c.byPath[cachePathKey{Host: "a", Method: "GET", Path: "/event"}]) |
| @@ -135,9 +183,9 @@ func TestCachePurgeURLRemovesAllVaryVariants(t *testing.T) { |
| mkResp := func(b string) *CachedResponse { |
| return &CachedResponse{StatusCode: 200, Header: http.Header{}, Body: []byte(b), StoredAt: time.Now(), TTL: time.Minute} |
| } |
| - c.Put(en, mkResp("en")) |
| - c.Put(fr, mkResp("fr")) |
| - c.Put(other, mkResp("other")) |
| + c.Put(en, "Accept-Language", mkResp("en")) |
| + c.Put(fr, "Accept-Language", mkResp("fr")) |
| + c.Put(other, "", mkResp("other")) |
| n := c.PurgeURL("a", "/event", "") |
| if n != 2 { |
| t.Fatalf("purged %d entries, want 2", n) |
diff --git a/internal/edge/edge.go b/internal/edge/edge.go| index 6e63ec092..50cb0090a 100644 |
| --- a/internal/edge/edge.go |
| +++ b/internal/edge/edge.go |
| @@ -374,7 +374,7 @@ func (e *Edge) maybeStore(key CacheKey, rec *recordingResponseWriter) { |
| if rec.status < 200 || rec.status >= 300 { |
| return |
| } |
| - cc := rec.Header().Get("Cache-Control") |
| + cc := joinHeader(rec.Header(), "Cache-Control") |
| ttl, ok := parseMaxAge(cc) |
| if !ok || ttl <= 0 { |
| return |
| @@ -388,7 +388,7 @@ func (e *Edge) maybeStore(key CacheKey, rec *recordingResponseWriter) { |
| if ttl > DefaultCacheMaxTTL { |
| ttl = DefaultCacheMaxTTL |
| } |
| - vary := rec.Header().Get("Vary") |
| + vary := joinHeader(rec.Header(), "Vary") |
| if vary != "" { |
| for _, h := range strings.Split(vary, ",") { |
| if strings.EqualFold(strings.TrimSpace(h), "Cookie") { |
| @@ -399,7 +399,10 @@ func (e *Edge) maybeStore(key CacheKey, rec *recordingResponseWriter) { |
| // varyValue so the store key and the lookup key can never drift. |
| key.VaryValue = varyValue(rec.request, vary) |
| } |
| - e.cache.Put(key, &CachedResponse{ |
| + // vary is handed to Put as well as consumed here: it is what Put records |
| + // (or, when empty, clears) as this URL's Vary scheme, so the next request's |
| + // GetFor resolves the very variant this store used. |
| + e.cache.Put(key, vary, &CachedResponse{ |
| StatusCode: rec.status, |
| Header: rec.Header().Clone(), |
| Body: rec.body, |
| @@ -408,17 +411,39 @@ func (e *Edge) maybeStore(key CacheKey, rec *recordingResponseWriter) { |
| }) |
| } |
| +// joinHeader returns every line of one header field as a single comma-joined |
| +// value. RFC 9110 §5.3: a field may legally appear on multiple lines and that |
| +// is identical in meaning to one comma-joined line — which is exactly what |
| +// http.Header.Add produces, and exactly how layered middleware emits |
| +// list-valued fields (compression adds Vary: Accept-Encoding, the session |
| +// layer separately adds Vary: Cookie; one handler sets Cache-Control: |
| +// max-age=600 and an auth wrapper adds Cache-Control: private). |
| +// |
| +// Every Cache-Control and Vary question this file asks goes through here |
| +// because http.Header.Get returns ONLY the first line: reading either field |
| +// with Get made every later line invisible, so a second line saying "private" |
| +// or "Cookie" silently failed to refuse the response. Joining with ", " keeps |
| +// the result a well-formed field value for the comma-splitting parsers below. |
| +func joinHeader(h http.Header, name string) string { |
| + return strings.Join(h.Values(name), ", ") |
| +} |
| + |
| // varyValue concatenates r's values for each header named in the response's |
| // Vary list, in the order Vary names them — the same raw string always |
| // produces the same bucket, which is all correctness requires (standard HTTP |
| // Vary semantics; RFC 7234 §4.1). |
| +// |
| +// vary itself may be a comma-joined multi-line field (see joinHeader); the |
| +// split-and-trim below handles that form identically to a single line. The |
| +// request side reads all of a header's lines too, so two requests that differ |
| +// only in how they split a varied-on header still land in different buckets. |
| func varyValue(r *http.Request, vary string) string { |
| var b strings.Builder |
| for _, h := range strings.Split(vary, ",") { |
| h = strings.TrimSpace(h) |
| b.WriteString(h) |
| b.WriteByte('=') |
| - b.WriteString(r.Header.Get(h)) |
| + b.WriteString(joinHeader(r.Header, h)) |
| b.WriteByte(';') |
| } |
| return b.String() |
diff --git a/internal/edge/edge_test.go b/internal/edge/edge_test.go| index 1ad2320c6..c09526e08 100644 |
| --- a/internal/edge/edge_test.go |
| +++ b/internal/edge/edge_test.go |
| @@ -503,7 +503,7 @@ func TestServeHTTPCacheHitNeverInvokesActivator(t *testing.T) { |
| } |
| e.EnableCache(NewCache(DefaultCacheMaxEntries, DefaultCacheMaxBodyBytes)) |
| key := CacheKey{Host: "cached.example.com", Method: "GET", Path: "/event"} |
| - e.cache.Put(key, &CachedResponse{ |
| + e.cache.Put(key, "", &CachedResponse{ |
| StatusCode: 200, |
| Header: http.Header{"Content-Type": {"text/html"}}, |
| Body: []byte("cached page"), |
| @@ -842,6 +842,171 @@ func TestServeHTTPRefusesVaryCookie(t *testing.T) { |
| } |
| } |
| +// cachedVariants counts every cached variant of one URL, whatever VaryValue |
| +// each was stored under. A "must not be cached" assertion has to ask this |
| +// question rather than Get with an empty VaryValue: a response that varies is |
| +// stored under a computed variant key, so the empty-key Get misses and the |
| +// assertion passes whether or not the refusal actually fired. |
| +func cachedVariants(e *Edge, host, path string) int { |
| + uk := CacheKey{Host: host, Method: "GET", Path: path}.urlKey() |
| + e.cache.mu.Lock() |
| + defer e.cache.mu.Unlock() |
| + return len(e.cache.byURL[uk]) |
| +} |
| + |
| +// getWith issues a GET carrying one extra header and returns the recorder. |
| +func getWith(t *testing.T, e *Edge, url, name, value string) *httptest.ResponseRecorder { |
| + t.Helper() |
| + req := httptest.NewRequest("GET", url, nil) |
| + if name != "" { |
| + req.Header.Set(name, value) |
| + } |
| + rec := httptest.NewRecorder() |
| + e.ServeHTTP(rec, req) |
| + return rec |
| +} |
| + |
| +// RFC 9110 §5.3: one header field may arrive as several lines, and that is |
| +// semantically identical to a single comma-joined line — which is exactly what |
| +// http.Header.Add produces and what layered middleware emits in practice |
| +// (compression adds Vary: Accept-Encoding, the session layer separately adds |
| +// Vary: Cookie). http.Header.Get returns only the FIRST line, so reading Vary |
| +// that way made everything after it invisible: the Vary: Cookie refusal never |
| +// fired and the entry was keyed on Accept-Encoding alone, so one visitor's |
| +// cookie-scoped response was replayed to the next visitor. |
| +func TestServeHTTPRefusesVaryCookieSplitAcrossHeaderLines(t *testing.T) { |
| + backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| + w.Header().Set("Cache-Control", "max-age=60") |
| + // Two separate Add calls, the standard shape for a list-valued header. |
| + w.Header().Add("Vary", "Accept-Encoding") |
| + w.Header().Add("Vary", "Cookie") |
| + w.WriteHeader(200) |
| + w.Write([]byte("dashboard for " + r.Header.Get("Cookie"))) |
| + })) |
| + t.Cleanup(backend.Close) |
| + |
| + reg := openReg(t) |
| + if err := reg.Upsert(registry.Route{Host: "split.example.com", Kind: "instance", Addr: backend.Listener.Addr().String(), Status: "ready"}); err != nil { |
| + t.Fatalf("upsert: %v", err) |
| + } |
| + e, err := New(reg, nil) |
| + if err != nil { |
| + t.Fatalf("New: %v", err) |
| + } |
| + e.EnableCache(NewCache(DefaultCacheMaxEntries, DefaultCacheMaxBodyBytes)) |
| + const url = "http://split.example.com/dashboard" |
| + |
| + alice := getWith(t, e, url, "Cookie", "sid=alice").Body.String() |
| + if alice != "dashboard for sid=alice" { |
| + t.Fatalf("alice body = %q, want %q", alice, "dashboard for sid=alice") |
| + } |
| + if n := cachedVariants(e, "split.example.com", "/dashboard"); n != 0 { |
| + // Errorf, not Fatalf: the replay assertion below is the live |
| + // consequence and is worth reporting in the same run. |
| + t.Errorf("%d variants cached for a response that varies on Cookie, want 0 — a multi-line Vary is still a Vary", n) |
| + } |
| + |
| + bob := getWith(t, e, url, "Cookie", "sid=bob").Body.String() |
| + if bob != "dashboard for sid=bob" { |
| + t.Fatalf("bob was served %q — one visitor's cookie-scoped response was replayed to another", bob) |
| + } |
| +} |
| + |
| +// The same root cause on Cache-Control: with max-age=600 on one line and |
| +// private on the next, Header.Get saw only max-age and the private response |
| +// was cached at the shared edge — the cross-visitor leak the private rule |
| +// exists to prevent. |
| +func TestServeHTTPRefusesCacheControlPrivateSplitAcrossHeaderLines(t *testing.T) { |
| + e := cacheProbeEdge(t, "splitcc.example.com", func(path string, h http.Header) { |
| + if path == "/private" { |
| + h.Add("Cache-Control", "max-age=600") |
| + h.Add("Cache-Control", "private") |
| + return |
| + } |
| + h.Set("Cache-Control", "max-age=600") |
| + }) |
| + e.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "http://splitcc.example.com/private", nil)) |
| + e.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "http://splitcc.example.com/shared", nil)) |
| + |
| + if n := cachedVariants(e, "splitcc.example.com", "/private"); n != 0 { |
| + t.Fatalf("%d entries cached for a response whose second Cache-Control line says private, want 0", n) |
| + } |
| + if !cachedAt(e, "splitcc.example.com", "/shared") { |
| + t.Fatal("an ordinary max-age response must still be cached") |
| + } |
| +} |
| + |
| +// The same root cause once more, on the Authorization rule: a response to an |
| +// Authorization-bearing request is private per RFC 9111 §3.5 unless |
| +// Cache-Control says public — and "public" on a second header line counts. |
| +func TestServeHTTPAuthorizedPublicSplitAcrossHeaderLines(t *testing.T) { |
| + e := cacheProbeEdge(t, "splitauth.example.com", func(path string, h http.Header) { |
| + h.Add("Cache-Control", "max-age=600") |
| + h.Add("Cache-Control", "public") |
| + }) |
| + req := httptest.NewRequest("GET", "http://splitauth.example.com/public", nil) |
| + req.Header.Set("Authorization", "Bearer secret-token") |
| + e.ServeHTTP(httptest.NewRecorder(), req) |
| + |
| + if !cachedAt(e, "splitauth.example.com", "/public") { |
| + t.Fatal("Cache-Control: public on a second header line still overrides the Authorization concern") |
| + } |
| +} |
| + |
| +// An app that emits Vary only for some requests — CORS middleware adding |
| +// Vary: Origin only when the request carries Origin, content negotiation |
| +// adding Vary: Accept only when it must — used to poison its own URL |
| +// permanently: the first Vary-carrying response recorded the Vary names for |
| +// that URL and nothing ever cleared them, so later non-Vary responses were |
| +// stored under the empty variant while every lookup kept resolving against the |
| +// remembered names. Store and lookup drifted apart, every ordinary request |
| +// missed, and on a hibernating route every miss is a wake — a remote, |
| +// unauthenticated wake-storm from one request. |
| +func TestServeHTTPConditionalVaryDoesNotPoisonURL(t *testing.T) { |
| + var hits atomic.Int64 |
| + backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| + hits.Add(1) |
| + w.Header().Set("Cache-Control", "max-age=60") |
| + if origin := r.Header.Get("Origin"); origin != "" { |
| + w.Header().Set("Vary", "Origin") |
| + w.Header().Set("Access-Control-Allow-Origin", origin) |
| + } |
| + w.WriteHeader(200) |
| + w.Write([]byte("page")) |
| + })) |
| + t.Cleanup(backend.Close) |
| + |
| + reg := openReg(t) |
| + if err := reg.Upsert(registry.Route{Host: "cors.example.com", Kind: "instance", Addr: backend.Listener.Addr().String(), Status: "ready"}); err != nil { |
| + t.Fatalf("upsert: %v", err) |
| + } |
| + e, err := New(reg, nil) |
| + if err != nil { |
| + t.Fatalf("New: %v", err) |
| + } |
| + e.EnableCache(NewCache(DefaultCacheMaxEntries, DefaultCacheMaxBodyBytes)) |
| + const url = "http://cors.example.com/page" |
| + |
| + // One unauthenticated request on a cold URL, eliciting the Vary variant. |
| + getWith(t, e, url, "Origin", "https://crawler.example") |
| + |
| + // Then ordinary traffic: the first is a legitimate miss, every one after |
| + // it must be served from cache. |
| + var last *httptest.ResponseRecorder |
| + for i := 0; i < 10; i++ { |
| + last = getWith(t, e, url, "", "") |
| + if got := last.Body.String(); got != "page" { |
| + t.Fatalf("ordinary request %d body = %q, want %q", i, got, "page") |
| + } |
| + } |
| + if n := hits.Load(); n != 2 { |
| + t.Fatalf("backend invoked %d times for 1 Origin request + 10 ordinary ones, want 2 — after the first ordinary miss every ordinary request must hit the cache", n) |
| + } |
| + if acao := last.Header().Get("Access-Control-Allow-Origin"); acao != "" { |
| + t.Fatalf("ordinary request served the Origin variant (Access-Control-Allow-Origin: %s)", acao) |
| + } |
| +} |
| + |
| // hijackableRecorder embeds httptest.ResponseRecorder (which deliberately |
| // does NOT implement http.Hijacker) and adds a no-op Hijack, standing in for |
| // a real connection's ResponseWriter — the minimum shape needed to prove |
diff --git a/internal/edge/purge_test.go b/internal/edge/purge_test.go| index 485413f26..6d126fa2f 100644 |
| --- a/internal/edge/purge_test.go |
| +++ b/internal/edge/purge_test.go |
| @@ -27,7 +27,7 @@ func TestListenPurgeRemovesCachedEntry(t *testing.T) { |
| dir := t.TempDir() |
| sock := filepath.Join(dir, "instance.sock") |
| cache := NewCache(DefaultCacheMaxEntries, DefaultCacheMaxBodyBytes) |
| - cache.Put(CacheKey{Host: "a.example", Method: "GET", Path: "/event"}, &CachedResponse{ |
| + cache.Put(CacheKey{Host: "a.example", Method: "GET", Path: "/event"}, "", &CachedResponse{ |
| StatusCode: 200, Header: http.Header{}, Body: []byte("hi"), StoredAt: time.Now(), TTL: time.Minute, |
| }) |
| @@ -57,7 +57,7 @@ func TestListenPurgeCannotAffectAnotherHost(t *testing.T) { |
| dir := t.TempDir() |
| sock := filepath.Join(dir, "instance.sock") |
| cache := NewCache(DefaultCacheMaxEntries, DefaultCacheMaxBodyBytes) |
| - cache.Put(CacheKey{Host: "other.example", Method: "GET", Path: "/event"}, &CachedResponse{ |
| + cache.Put(CacheKey{Host: "other.example", Method: "GET", Path: "/event"}, "", &CachedResponse{ |
| StatusCode: 200, Header: http.Header{}, Body: []byte("hi"), StoredAt: time.Now(), TTL: time.Minute, |
| }) |