amadan

carlos / platform Public

edge: the cache stops missing on Vary, stops leaking private responses, and stops being floodable

Five review findings against the response cache, all reproduced before being
fixed:

C1 — a Vary-carrying response was stored under varyValue(request, vary) but
looked up under an empty VaryValue, so it NEVER hit: 2 backend calls for 2
identical requests, measured. Every miss also inserted a new unreachable entry,
so a crawler cycling Accept-Language could evict the whole cache. Lookup is now
two-phase (Cache.GetFor): the Vary header names the last response for a URL
asked for are remembered, replayed against the request, and resolved through the
same varyValue both sides now share, so store key and lookup key cannot drift.

C1 (admission) — two caps bound how much of the cache one URL or path can own,
since the global LRU alone lets one URL evict everyone else's entries: 8 Vary
variants per URL, 32 entries per path. RawQuery stays in the key (correctness:
different queries can mean different content) and the per-path cap is what
bounds a ?x=1..N flood. Measured before: 65 variants and 129 entries from a
64-wide flood; after: 8 and 32.

C2 — the cache replayed other visitors' responses. Cache-Control: private was
cached, Set-Cookie was cloned and served to everyone (verified: sid=abc123 came
back out of the cache), and a response to an Authorization-bearing request was
served to anonymous visitors. All three now refuse, with an explicit
Cache-Control: public overriding the Authorization case per RFC 9111 §3.5.
parseMaxAge also scans the whole header before honoring max-age, so
"max-age=600, private" is refused rather than cached on directive order.

I1 — TTLs are clamped to DefaultCacheMaxTTL (5m), which is what makes spec
decision 4 true: an app asking for max-age=31536000 previously turned one lost
purge into a permanently wrong page.

I3 — the full-cycle test gains a sibling that drives the REAL purge channel: a
JSON POST /purge over the unix control socket ListenPurge binds, rather than the
in-process cache API, so a broken socket, encoding, or host scoping would fail
it.

Minors: gofmt on cache.go; the per-refusal log line for Vary: Cookie is gone
(unrate-limited, and a crawler could flood it from a hot path).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Paul Campbell 3ece43fb4afe904983be5eff4c7e0de1f1da698e parent f810271
4 files changed, +607 −44
  • internal/edge/cache.go +161 −17
  • internal/edge/cache_test.go +59 −0
  • internal/edge/edge.go +85 −15
  • internal/edge/edge_test.go +302 −12
diff --git a/internal/edge/cache.go b/internal/edge/cache.go
index fe873a387..189ec4952 100644
--- a/internal/edge/cache.go
+++ b/internal/edge/cache.go
@@ -7,6 +7,26 @@ import (
"time"
)
+// Admission caps that bound how much of the cache any single URL or path can
+// occupy, independent of the global LRU. The global LRU alone does not stop
+// one URL from being pumped full of noise entries that then evict OTHER
+// hosts' useful entries — a crawler sending Accept-Language: xx-1 … xx-4096
+// (distinct Vary variants) or ?x=1 … ?x=4096 (distinct query strings, part of
+// the key because correctness requires it: different query strings can mean
+// different content) would otherwise flush the whole cache on demand.
+//
+// Two caps, because the two floods land on different key components:
+// - maxVaryVariantsPerURL bounds the Vary variants of one exact URL
+// (host+method+path+query).
+// - maxEntriesPerPath bounds everything under one path regardless of query
+// string or variant, which is what actually bounds a query-string flood.
+//
+// Both evict the oldest entry in their group (by StoredAt) when exceeded.
+const (
+ maxVaryVariantsPerURL = 8
+ maxEntriesPerPath = 32
+)
+
// CacheKey is the full key for a cached response, including Host, Method, Path, RawQuery,
// and VaryValue (concatenation of request-header values named by the response's Vary header).
type CacheKey struct {
@@ -36,15 +56,27 @@ type Cache struct {
mu sync.Mutex
maxEntries int
maxBodyBytes int
- order *list.List // MRU at front, LRU at back
+ order *list.List // MRU at front, LRU at back
entries map[cacheFullKey]*list.Element
- byURL map[cacheURLKey]map[string]struct{} // urlKey -> set of VaryValue
+ 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 map[cacheURLKey]string
}
type cacheURLKey struct {
Host, Method, Path, RawQuery string
}
+type cachePathKey struct {
+ Host, Method, Path string
+}
+
type cacheFullKey struct {
cacheURLKey
VaryValue string
@@ -62,7 +94,9 @@ func NewCache(maxEntries, maxBodyBytes int) *Cache {
maxBodyBytes: maxBodyBytes,
order: list.New(),
entries: make(map[cacheFullKey]*list.Element),
- byURL: make(map[cacheURLKey]map[string]struct{}),
+ byURL: make(map[cacheURLKey]map[cacheFullKey]*list.Element),
+ byPath: make(map[cachePathKey]map[cacheFullKey]*list.Element),
+ varyNames: make(map[cacheURLKey]string),
}
}
@@ -74,9 +108,17 @@ func (k CacheKey) fullKey() cacheFullKey {
return cacheFullKey{cacheURLKey: k.urlKey(), VaryValue: k.VaryValue}
}
+func (k cacheURLKey) pathKey() cachePathKey {
+ return cachePathKey{Host: k.Host, Method: k.Method, Path: k.Path}
+}
+
// Get retrieves a cached response if it exists and has not expired.
// Returns (nil, false) for a miss or an expired entry.
// Expired entries are evicted lazily on lookup, not swept on a timer.
+//
+// Get takes the VaryValue already resolved by the caller. Request handling
+// goes through GetFor instead, which resolves it first — a request can't know
+// the response's Vary header until the response exists.
func (c *Cache) Get(key CacheKey) (*CachedResponse, bool) {
c.mu.Lock()
defer c.mu.Unlock()
@@ -94,8 +136,36 @@ func (c *Cache) Get(key CacheKey) (*CachedResponse, bool) {
return e.resp, true
}
+// GetFor is the request-time lookup: phase one resolves which Vary variant r
+// belongs to, phase two does the ordinary Get. baseKey carries everything
+// known before the response exists (host, method, path, query) with VaryValue
+// 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,
+// 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
+// was precisely that drift: storing under varyValue(request, vary) while
+// looking up under "", so a Vary-carrying response never hit.
+func (c *Cache) GetFor(baseKey CacheKey, r *http.Request) (*CachedResponse, bool) {
+ c.mu.Lock()
+ names := c.varyNames[baseKey.urlKey()]
+ c.mu.Unlock()
+ if names != "" {
+ baseKey.VaryValue = varyValue(r, names)
+ }
+ return c.Get(baseKey)
+}
+
// Put stores a response in the cache. Silently drops a response whose body
-// exceeds maxBodyBytes. Evicts the least-recently-used entry when at maxEntries.
+// exceeds maxBodyBytes. Evicts the least-recently-used entry when at
+// 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) {
if len(resp.Body) > c.maxBodyBytes {
return
@@ -103,6 +173,12 @@ func (c *Cache) Put(key CacheKey, resp *CachedResponse) {
c.mu.Lock()
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 el, ok := c.entries[fk]; ok {
el.Value.(*cacheEntry).resp = resp
c.order.MoveToFront(el)
@@ -110,11 +186,18 @@ func (c *Cache) Put(key CacheKey, resp *CachedResponse) {
}
el := c.order.PushFront(&cacheEntry{key: fk, resp: resp})
c.entries[fk] = el
- uk := fk.cacheURLKey
if c.byURL[uk] == nil {
- c.byURL[uk] = make(map[string]struct{})
+ c.byURL[uk] = make(map[cacheFullKey]*list.Element)
+ }
+ c.byURL[uk][fk] = el
+ pk := uk.pathKey()
+ if c.byPath[pk] == nil {
+ c.byPath[pk] = make(map[cacheFullKey]*list.Element)
}
- c.byURL[uk][fk.VaryValue] = struct{}{}
+ c.byPath[pk][fk] = el
+
+ c.capGroupLocked(c.byURL[uk], maxVaryVariantsPerURL, el)
+ c.capGroupLocked(c.byPath[pk], maxEntriesPerPath, el)
for len(c.entries) > c.maxEntries {
victim := c.order.Back()
if victim == nil {
@@ -124,15 +207,50 @@ func (c *Cache) Put(key CacheKey, resp *CachedResponse) {
}
}
+// capGroupLocked drops the oldest entries (by StoredAt) from one admission
+// group until it holds at most max, never dropping keep (the entry just
+// inserted — evicting it would make the flood self-defeating in the wrong
+// direction, turning every request into a permanent miss AND a permanent
+// insert). Caller must hold c.mu.
+func (c *Cache) capGroupLocked(group map[cacheFullKey]*list.Element, max int, keep *list.Element) {
+ for len(group) > max {
+ var oldest *list.Element
+ for _, el := range group {
+ if el == keep {
+ continue
+ }
+ if oldest == nil || el.Value.(*cacheEntry).resp.StoredAt.Before(oldest.Value.(*cacheEntry).resp.StoredAt) {
+ oldest = el
+ }
+ }
+ if oldest == nil {
+ return
+ }
+ c.removeLocked(oldest)
+ }
+}
+
// removeLocked drops el from every index. Caller must hold c.mu.
func (c *Cache) removeLocked(el *list.Element) {
e := el.Value.(*cacheEntry)
c.order.Remove(el)
delete(c.entries, e.key)
- if variants := c.byURL[e.key.cacheURLKey]; variants != nil {
- delete(variants, e.key.VaryValue)
+ uk := e.key.cacheURLKey
+ if variants := c.byURL[uk]; variants != nil {
+ delete(variants, e.key)
if len(variants) == 0 {
- delete(c.byURL, e.key.cacheURLKey)
+ delete(c.byURL, uk)
+ // No entries left for this URL, so the remembered Vary names have
+ // nothing to resolve against — dropping them keeps varyNames
+ // bounded by the number of live URLs.
+ delete(c.varyNames, uk)
+ }
+ }
+ pk := uk.pathKey()
+ if group := c.byPath[pk]; group != nil {
+ delete(group, e.key)
+ if len(group) == 0 {
+ delete(c.byPath, pk)
}
}
}
@@ -147,14 +265,40 @@ func (c *Cache) PurgeURL(host, path, rawQuery string) int {
// GET and HEAD are purged together — a write invalidates both.
for _, method := range [...]string{"GET", "HEAD"} {
uk := cacheURLKey{Host: host, Method: method, Path: path, RawQuery: rawQuery}
- variants := c.byURL[uk]
- for vary := range variants {
- fk := cacheFullKey{cacheURLKey: uk, VaryValue: vary}
- if el, ok := c.entries[fk]; ok {
- c.removeLocked(el)
- n++
- }
+ for _, el := range elementsOf(c.byURL[uk]) {
+ c.removeLocked(el)
+ n++
}
}
return n
}
+
+// PurgeHost drops every cached entry for host — all paths, all query strings,
+// all Vary variants — and returns how many it removed. This is what a deploy
+// calls: a fresh process for a host means new code and potentially new
+// content, so everything cached from the old one is suspect. A whole-cache
+// scan is fine here; it happens once per spawn, not per request.
+func (c *Cache) PurgeHost(host string) int {
+ c.mu.Lock()
+ defer c.mu.Unlock()
+ var doomed []*list.Element
+ for key, el := range c.entries {
+ if key.Host == host {
+ doomed = append(doomed, el)
+ }
+ }
+ for _, el := range doomed {
+ c.removeLocked(el)
+ }
+ return len(doomed)
+}
+
+// elementsOf snapshots a group's elements so the caller can remove them while
+// iterating (removeLocked mutates the very map being ranged over).
+func elementsOf(group map[cacheFullKey]*list.Element) []*list.Element {
+ els := make([]*list.Element, 0, len(group))
+ for _, el := range group {
+ els = append(els, el)
+ }
+ return els
+}
diff --git a/internal/edge/cache_test.go b/internal/edge/cache_test.go
index aa06d467d..71bf388e8 100644
--- a/internal/edge/cache_test.go
+++ b/internal/edge/cache_test.go
@@ -1,6 +1,7 @@
package edge
import (
+ "fmt"
"net/http"
"testing"
"time"
@@ -68,6 +69,64 @@ func TestCacheEvictsLeastRecentlyUsedAtCapacity(t *testing.T) {
}
}
+// 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.
+func TestCachePurgeHostRemovesEveryEntryForThatHost(t *testing.T) {
+ c := NewCache(100, 1<<20)
+ mkResp := func(b string) *CachedResponse {
+ return &CachedResponse{StatusCode: 200, Header: http.Header{}, Body: []byte(b), StoredAt: time.Now(), TTL: time.Minute}
+ }
+ keys := []CacheKey{
+ {Host: "a", Method: "GET", Path: "/one"},
+ {Host: "a", Method: "GET", Path: "/two", VaryValue: "en"},
+ {Host: "a", Method: "GET", Path: "/two", VaryValue: "fr"},
+ {Host: "a", Method: "HEAD", Path: "/one"},
+ {Host: "a", Method: "GET", Path: "/one", RawQuery: "page=2"},
+ }
+ for _, k := range keys {
+ c.Put(k, mkResp(k.Path))
+ }
+ other := CacheKey{Host: "b", Method: "GET", Path: "/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))
+ }
+ for _, k := range keys {
+ if _, ok := c.Get(k); ok {
+ t.Fatalf("entry %+v survived PurgeHost", k)
+ }
+ }
+ if _, ok := c.Get(other); !ok {
+ t.Fatal("PurgeHost(\"a\") must leave host b's entries untouched")
+ }
+ if n := c.PurgeHost("nobody"); n != 0 {
+ t.Fatalf("PurgeHost on an uncached host removed %d, want 0", n)
+ }
+}
+
+// An attacker pumping distinct query strings at one path must not be able to
+// evict the rest of the fleet's cache: the per-path cap bounds how many
+// entries a single path can occupy, independent of the global LRU.
+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.mu.Lock()
+ n := len(c.byPath[cachePathKey{Host: "a", Method: "GET", Path: "/event"}])
+ total := len(c.entries)
+ c.mu.Unlock()
+ if n > maxEntriesPerPath {
+ t.Fatalf("one path holds %d cached entries, want at most %d", n, maxEntriesPerPath)
+ }
+ if total > maxEntriesPerPath {
+ t.Fatalf("cache holds %d entries after a query-string flood, want at most %d", total, maxEntriesPerPath)
+ }
+}
+
func TestCachePurgeURLRemovesAllVaryVariants(t *testing.T) {
c := NewCache(10, 1<<20)
en := CacheKey{Host: "a", Method: "GET", Path: "/event", VaryValue: "en"}
diff --git a/internal/edge/edge.go b/internal/edge/edge.go
index e382c0fff..6e63ec092 100644
--- a/internal/edge/edge.go
+++ b/internal/edge/edge.go
@@ -51,6 +51,14 @@ const (
DefaultCacheMaxBodyBytes = 1 << 20 // 1 MiB
)
+// DefaultCacheMaxTTL is the ceiling every cached entry's TTL is clamped to,
+// however long the app's own max-age asks for. It is what makes spec decision
+// 4 true — "a lost purge self-heals within a bounded window": without a
+// ceiling, an app setting max-age=31536000 turns one dropped purge call into a
+// permanently wrong page. Five minutes is short enough that a missed purge is
+// a blip and long enough that a crawler sweep still rides the cache.
+const DefaultCacheMaxTTL = 5 * time.Minute
+
// Activator is the hibernation seam (implemented by the activator package):
// given a hibernate-flagged instance route, it wakes (or joins an in-progress
// wake of) the backing process before letting the request through, and
@@ -283,7 +291,11 @@ func (e *Edge) ServeHTTP(w http.ResponseWriter, r *http.Request) {
var key CacheKey
if cacheable {
key = CacheKey{Host: host, Method: r.Method, Path: r.URL.Path, RawQuery: r.URL.RawQuery}
- if resp, hit := e.cache.Get(key); hit {
+ // GetFor, not Get: the variant this request belongs to depends on the
+ // response's Vary header, which doesn't exist yet — GetFor resolves it
+ // from the Vary names the last response for this URL recorded, so the
+ // lookup key matches the key maybeStore stored under.
+ if resp, hit := e.cache.GetFor(key, r); hit {
writeCached(w, resp)
return
}
@@ -339,8 +351,22 @@ func (e *Edge) cacheMaxBody() int {
}
// maybeStore inspects a recorded response and caches it if it qualifies:
-// 2xx status, a Cache-Control that names a positive max-age, and no
-// Vary: Cookie (varying by cookie would defeat correctness per-visitor).
+// 2xx status, a Cache-Control that names a positive max-age, and none of the
+// per-visitor markers below. This cache is shared across every visitor of a
+// host and caching is unconditional per route (not opt-in per app), so each
+// refusal here is the only thing standing between one visitor's response and
+// everyone else's:
+//
+// - Cache-Control: private — the app itself saying "one visitor only".
+// - Set-Cookie — the response carries per-visitor state (a session id, a
+// CSRF token); replaying it would hand that cookie to later visitors.
+// - an Authorization header on the REQUEST — the response is presumed
+// private per RFC 9111 §3.5, unless Cache-Control explicitly says public,
+// which is the app asserting the response is safe to share.
+// - Vary: Cookie — varying by cookie would defeat correctness per-visitor.
+//
+// The stored TTL is clamped to DefaultCacheMaxTTL so a lost purge always
+// self-heals within a bounded window.
func (e *Edge) maybeStore(key CacheKey, rec *recordingResponseWriter) {
if e.cache == nil || !rec.cacheable || rec.truncated {
return
@@ -348,18 +374,29 @@ func (e *Edge) maybeStore(key CacheKey, rec *recordingResponseWriter) {
if rec.status < 200 || rec.status >= 300 {
return
}
- ttl, ok := parseMaxAge(rec.Header().Get("Cache-Control"))
+ cc := rec.Header().Get("Cache-Control")
+ ttl, ok := parseMaxAge(cc)
if !ok || ttl <= 0 {
return
}
+ if len(rec.Header().Values("Set-Cookie")) > 0 {
+ return
+ }
+ if rec.request.Header.Get("Authorization") != "" && !hasCCDirective(cc, "public") {
+ return
+ }
+ if ttl > DefaultCacheMaxTTL {
+ ttl = DefaultCacheMaxTTL
+ }
vary := rec.Header().Get("Vary")
if vary != "" {
for _, h := range strings.Split(vary, ",") {
if strings.EqualFold(strings.TrimSpace(h), "Cookie") {
- log.Printf("edge: refusing to cache %s%s — Vary: Cookie would break per-visitor correctness", key.Host, key.Path)
return
}
}
+ // The same computation GetFor replays at lookup time — both call
+ // varyValue so the store key and the lookup key can never drift.
key.VaryValue = varyValue(rec.request, vary)
}
e.cache.Put(key, &CachedResponse{
@@ -388,23 +425,56 @@ func varyValue(r *http.Request, vary string) string {
}
// parseMaxAge extracts max-age from a Cache-Control header. Returns
-// ok=false for "no-store", "no-cache", or a missing/malformed max-age — any
-// of which mean "don't cache this."
+// ok=false for "no-store", "no-cache", "private", or a missing/malformed
+// max-age — any of which mean "don't cache this in a shared cache."
+//
+// The whole header is scanned before the max-age is honored, so a
+// "max-age=600, private" (directives in either order, and "private" in its
+// field-name form private="Set-Cookie") is refused rather than cached because
+// max-age happened to come first.
func parseMaxAge(cc string) (time.Duration, bool) {
- for _, part := range strings.Split(cc, ",") {
- part = strings.TrimSpace(part)
- if strings.EqualFold(part, "no-store") || strings.EqualFold(part, "no-cache") {
+ var ttl time.Duration
+ found := false
+ for name, arg := range ccDirectives(cc) {
+ switch name {
+ case "no-store", "no-cache", "private":
return 0, false
- }
- if v, ok := strings.CutPrefix(strings.ToLower(part), "max-age="); ok {
- secs, err := strconv.Atoi(v)
+ case "max-age":
+ secs, err := strconv.Atoi(strings.TrimSpace(arg))
if err != nil {
return 0, false
}
- return time.Duration(secs) * time.Second, true
+ ttl, found = time.Duration(secs)*time.Second, true
+ }
+ }
+ if !found {
+ return 0, false
+ }
+ return ttl, true
+}
+
+// hasCCDirective reports whether cc carries the named directive.
+func hasCCDirective(cc, want string) bool {
+ for name := range ccDirectives(cc) {
+ if name == want {
+ return true
+ }
+ }
+ return false
+}
+
+// ccDirectives iterates a Cache-Control header as (lowercased name, argument)
+// pairs; the argument is "" for a bare directive. One parse shape for every
+// directive question this file asks.
+func ccDirectives(cc string) func(func(string, string) bool) {
+ return func(yield func(string, string) bool) {
+ for _, part := range strings.Split(cc, ",") {
+ name, arg, _ := strings.Cut(strings.TrimSpace(part), "=")
+ if !yield(strings.ToLower(strings.TrimSpace(name)), arg) {
+ return
+ }
}
}
- return 0, false
}
// writeCached replays a cached response verbatim onto w.
diff --git a/internal/edge/edge_test.go b/internal/edge/edge_test.go
index a14968d3f..1ad2320c6 100644
--- a/internal/edge/edge_test.go
+++ b/internal/edge/edge_test.go
@@ -2,8 +2,11 @@ package edge
import (
"bufio"
+ "bytes"
"context"
+ "encoding/json"
"errors"
+ "fmt"
"io"
"net"
"net/http"
@@ -11,6 +14,7 @@ import (
"os"
"path/filepath"
"strings"
+ "sync/atomic"
"syscall"
"testing"
"time"
@@ -577,8 +581,14 @@ func TestServeHTTPNeverCachesNon2xx(t *testing.T) {
}
}
-func TestServeHTTPVaryAcceptLanguageProducesSeparateEntries(t *testing.T) {
+// varyBackendEdge wires an Edge in front of a backend that echoes the request's
+// Accept-Language and counts its own invocations — the counter is what proves a
+// second request was answered from cache rather than re-proxied, which asserting
+// on response bodies alone cannot do (a miss echoes the same body a hit would).
+func varyBackendEdge(t *testing.T, host string, hits *atomic.Int64) *Edge {
+ t.Helper()
backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ hits.Add(1)
w.Header().Set("Cache-Control", "max-age=60")
w.Header().Set("Vary", "Accept-Language")
w.WriteHeader(200)
@@ -587,7 +597,7 @@ func TestServeHTTPVaryAcceptLanguageProducesSeparateEntries(t *testing.T) {
t.Cleanup(backend.Close)
reg := openReg(t)
- if err := reg.Upsert(registry.Route{Host: "i18n.example.com", Kind: "instance", Addr: backend.Listener.Addr().String(), Status: "ready"}); err != nil {
+ if err := reg.Upsert(registry.Route{Host: host, Kind: "instance", Addr: backend.Listener.Addr().String(), Status: "ready"}); err != nil {
t.Fatalf("upsert: %v", err)
}
e, err := New(reg, nil)
@@ -595,19 +605,214 @@ func TestServeHTTPVaryAcceptLanguageProducesSeparateEntries(t *testing.T) {
t.Fatalf("New: %v", err)
}
e.EnableCache(NewCache(DefaultCacheMaxEntries, DefaultCacheMaxBodyBytes))
+ return e
+}
+
+// getLang issues a GET carrying Accept-Language: lang (omitted entirely when
+// lang is "") and returns the body.
+func getLang(t *testing.T, e *Edge, url, lang string) string {
+ t.Helper()
+ req := httptest.NewRequest("GET", url, nil)
+ if lang != "" {
+ req.Header.Set("Accept-Language", lang)
+ }
+ rec := httptest.NewRecorder()
+ e.ServeHTTP(rec, req)
+ return rec.Body.String()
+}
+
+// A Vary-carrying response must be both separated per variant AND actually
+// re-served from cache on a repeat request. The backend-invocation count is
+// the load-bearing assertion: pre-fix, ServeHTTP looked entries up with an
+// empty VaryValue while maybeStore stored them under varyValue(request, vary),
+// so every request was a miss — 4 backend hits, not 2 — while the bodies still
+// differed per language, which is why the old body-comparison assertion passed
+// with Vary support entirely broken.
+func TestServeHTTPVaryAcceptLanguageProducesSeparateEntries(t *testing.T) {
+ var hits atomic.Int64
+ e := varyBackendEdge(t, "i18n.example.com", &hits)
+ const url = "http://i18n.example.com/event"
+
+ for _, want := range []struct{ lang, body string }{
+ {"en", "lang=en"}, {"en", "lang=en"}, {"fr", "lang=fr"}, {"fr", "lang=fr"},
+ } {
+ if got := getLang(t, e, url, want.lang); got != want.body {
+ t.Fatalf("Accept-Language %q: body = %q, want %q", want.lang, got, want.body)
+ }
+ }
+
+ if n := hits.Load(); n != 2 {
+ t.Fatalf("backend invoked %d times across 4 requests (en, en, fr, fr), want 2 — the repeat of each language must be served from cache", n)
+ }
+}
- reqEn := httptest.NewRequest("GET", "http://i18n.example.com/event", nil)
- reqEn.Header.Set("Accept-Language", "en")
- recEn := httptest.NewRecorder()
- e.ServeHTTP(recEn, reqEn)
+// The core regression: a repeat of an identical request against a
+// Vary-carrying response must hit. Here no Accept-Language is sent at all, so
+// both requests resolve to the same (empty-valued) variant.
+func TestServeHTTPVaryRepeatIdenticalRequestHitsCache(t *testing.T) {
+ var hits atomic.Int64
+ e := varyBackendEdge(t, "repeat.example.com", &hits)
+ const url = "http://repeat.example.com/event"
- reqFr := httptest.NewRequest("GET", "http://i18n.example.com/event", nil)
- reqFr.Header.Set("Accept-Language", "fr")
- recFr := httptest.NewRecorder()
- e.ServeHTTP(recFr, reqFr)
+ if got := getLang(t, e, url, ""); got != "lang=" {
+ t.Fatalf("first body = %q, want %q", got, "lang=")
+ }
+ if got := getLang(t, e, url, ""); got != "lang=" {
+ t.Fatalf("second body = %q, want %q", got, "lang=")
+ }
+ if n := hits.Load(); n != 1 {
+ t.Fatalf("backend invoked %d times across 2 identical requests, want 1 — the second must be a cache hit", n)
+ }
+}
- if recEn.Body.String() == recFr.Body.String() {
- t.Fatalf("expected distinct cached responses per Accept-Language, got %q for both", recEn.Body.String())
+// A crawler pumping distinct Vary values at one URL must not be able to fill
+// the cache with unreachable variants: the per-URL cap bounds how many a
+// single URL can occupy, evicting the oldest as new ones arrive.
+func TestServeHTTPVaryVariantCapBoundsEntriesPerURL(t *testing.T) {
+ var hits atomic.Int64
+ e := varyBackendEdge(t, "flood.example.com", &hits)
+ const url = "http://flood.example.com/event"
+
+ langs := make([]string, 4*maxVaryVariantsPerURL)
+ for i := range langs {
+ langs[i] = fmt.Sprintf("xx-%d", i)
+ getLang(t, e, url, langs[i])
+ }
+
+ uk := CacheKey{Host: "flood.example.com", Method: "GET", Path: "/event"}.urlKey()
+ e.cache.mu.Lock()
+ n := len(e.cache.byURL[uk])
+ e.cache.mu.Unlock()
+ if n > maxVaryVariantsPerURL {
+ t.Fatalf("one URL holds %d cached variants, want at most %d", n, maxVaryVariantsPerURL)
+ }
+
+ // The newest variant survives; the oldest was evicted to make room.
+ before := hits.Load()
+ getLang(t, e, url, langs[len(langs)-1])
+ if hits.Load() != before {
+ t.Fatal("the most recent Vary variant should still be cached")
+ }
+ getLang(t, e, url, langs[0])
+ if hits.Load() != before+1 {
+ t.Fatal("the oldest Vary variant should have been evicted by the per-URL cap")
+ }
+}
+
+// cacheProbeEdge wires an Edge whose backend answers with headers chosen per
+// path by hdr — the shape every "must not be cached" rule test needs.
+func cacheProbeEdge(t *testing.T, host string, hdr func(path string, h http.Header)) *Edge {
+ t.Helper()
+ backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ hdr(r.URL.Path, w.Header())
+ w.WriteHeader(200)
+ w.Write([]byte("body for " + r.URL.Path))
+ }))
+ t.Cleanup(backend.Close)
+
+ reg := openReg(t)
+ if err := reg.Upsert(registry.Route{Host: host, 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))
+ return e
+}
+
+func cachedAt(e *Edge, host, path string) bool {
+ _, ok := e.cache.Get(CacheKey{Host: host, Method: "GET", Path: path})
+ return ok
+}
+
+// Cache-Control: private means "for one visitor only" — caching it at the
+// shared edge and replaying it to everyone else is a cross-visitor leak.
+func TestServeHTTPRefusesCacheControlPrivate(t *testing.T) {
+ e := cacheProbeEdge(t, "priv.example.com", func(path string, h http.Header) {
+ if path == "/private" {
+ h.Set("Cache-Control", "private, max-age=600")
+ return
+ }
+ h.Set("Cache-Control", "max-age=600")
+ })
+ e.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "http://priv.example.com/private", nil))
+ e.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "http://priv.example.com/shared", nil))
+
+ if cachedAt(e, "priv.example.com", "/private") {
+ t.Fatal("a Cache-Control: private response must never be cached at the shared edge")
+ }
+ if !cachedAt(e, "priv.example.com", "/shared") {
+ t.Fatal("an ordinary max-age response must still be cached")
+ }
+}
+
+// A Set-Cookie response carries per-visitor state (a session, a CSRF token);
+// caching it would hand that cookie to every later visitor.
+func TestServeHTTPRefusesResponseWithSetCookie(t *testing.T) {
+ e := cacheProbeEdge(t, "cookie2.example.com", func(path string, h http.Header) {
+ h.Set("Cache-Control", "max-age=600")
+ if path == "/session" {
+ h.Set("Set-Cookie", "sid=abc123; Path=/; HttpOnly")
+ }
+ })
+ e.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "http://cookie2.example.com/session", nil))
+ e.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "http://cookie2.example.com/plain", nil))
+
+ if cachedAt(e, "cookie2.example.com", "/session") {
+ t.Fatal("a response carrying Set-Cookie must never be cached — the cookie would be replayed to other visitors")
+ }
+ if !cachedAt(e, "cookie2.example.com", "/plain") {
+ t.Fatal("a response without Set-Cookie must still be cached")
+ }
+}
+
+// A response to an Authorization-bearing request is presumed private (RFC 9111
+// §3.5) unless the app explicitly marks it public.
+func TestServeHTTPRefusesAuthorizedRequestUnlessPublic(t *testing.T) {
+ e := cacheProbeEdge(t, "auth.example.com", func(path string, h http.Header) {
+ if path == "/public" {
+ h.Set("Cache-Control", "public, max-age=600")
+ return
+ }
+ h.Set("Cache-Control", "max-age=600")
+ })
+ authGet := func(path string) {
+ req := httptest.NewRequest("GET", "http://auth.example.com"+path, nil)
+ req.Header.Set("Authorization", "Bearer secret-token")
+ e.ServeHTTP(httptest.NewRecorder(), req)
+ }
+ authGet("/account")
+ authGet("/public")
+ e.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "http://auth.example.com/anon", nil))
+
+ if cachedAt(e, "auth.example.com", "/account") {
+ t.Fatal("a response to an Authorization-bearing request must not be cached without an explicit Cache-Control: public")
+ }
+ if !cachedAt(e, "auth.example.com", "/public") {
+ t.Fatal("Cache-Control: public overrides the Authorization concern (RFC 9111 §3.5) — this response should be cached")
+ }
+ if !cachedAt(e, "auth.example.com", "/anon") {
+ t.Fatal("an unauthenticated request's ordinary max-age response must still be cached")
+ }
+}
+
+// An app asking for a year of freshness would make a single lost purge
+// effectively permanent; the edge clamps every TTL to its own ceiling so a
+// missed purge always self-heals within a bounded window (spec decision 4).
+func TestServeHTTPClampsMaxAgeToCeiling(t *testing.T) {
+ e := cacheProbeEdge(t, "ttl.example.com", func(path string, h http.Header) {
+ h.Set("Cache-Control", "max-age=31536000")
+ })
+ e.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "http://ttl.example.com/event", nil))
+
+ resp, ok := e.cache.Get(CacheKey{Host: "ttl.example.com", Method: "GET", Path: "/event"})
+ if !ok {
+ t.Fatal("expected the response to be cached")
+ }
+ if resp.TTL != DefaultCacheMaxTTL {
+ t.Fatalf("cached TTL = %s, want it clamped to %s", resp.TTL, DefaultCacheMaxTTL)
}
}
@@ -704,6 +909,91 @@ func (f activatorFunc) RetireExcept(keep map[string]bool) {
// No-op for tests that don't need retirement tracking.
}
+// TestCacheBeforeWakeFullCycleOverPurgeSocket is the full cycle with the REAL
+// purge channel in it: the same cold-miss → cached-repeat → purge → fresh
+// sequence as TestCacheBeforeWakeFullCycle, except the purge arrives the way a
+// spawned instance actually sends it — a JSON POST /purge over the unix
+// control socket ListenPurge binds (the socket whose path the activator hands
+// the child as CARLOS_CONTROL_SOCKET). The sibling test below drives the
+// in-process cache API, which cannot catch a broken socket, encoding, or
+// host-scoping; this one proves the whole channel from "instance calls purge"
+// to "edge cache entry is gone."
+func TestCacheBeforeWakeFullCycleOverPurgeSocket(t *testing.T) {
+ body := "version 1"
+ backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Cache-Control", "max-age=60")
+ w.WriteHeader(200)
+ w.Write([]byte(body))
+ }))
+ t.Cleanup(backend.Close)
+
+ wakes := 0
+ act := activatorFunc(func(w http.ResponseWriter, r *http.Request, rt registry.Route, proxy http.Handler) bool {
+ wakes++
+ proxy.ServeHTTP(w, r)
+ return true
+ })
+
+ reg := openReg(t)
+ if err := reg.Upsert(registry.Route{Host: "sock-cycle.example.com", Kind: "instance", Addr: backend.Listener.Addr().String(), Status: "ready", Hibernate: true}); err != nil {
+ t.Fatalf("upsert: %v", err)
+ }
+ e, err := New(reg, act)
+ if err != nil {
+ t.Fatalf("New: %v", err)
+ }
+ cache := NewCache(DefaultCacheMaxEntries, DefaultCacheMaxBodyBytes)
+ e.EnableCache(cache)
+
+ // The control socket the activator would create for this instance and
+ // hand it as CARLOS_CONTROL_SOCKET.
+ controlSocket := filepath.Join(t.TempDir(), "instance.sock.control")
+ pl, err := ListenPurge(controlSocket, "sock-cycle.example.com", cache)
+ if err != nil {
+ if errors.Is(err, syscall.EPERM) || strings.Contains(err.Error(), "operation not permitted") {
+ t.Skipf("unix-socket listeners not permitted in this environment: %v", err)
+ }
+ t.Fatalf("ListenPurge: %v", err)
+ }
+ defer pl.Close()
+
+ get := func() string {
+ rec := httptest.NewRecorder()
+ e.ServeHTTP(rec, httptest.NewRequest("GET", "http://sock-cycle.example.com/event", nil))
+ return rec.Body.String()
+ }
+
+ if got := get(); got != "version 1" {
+ t.Fatalf("first request body = %q, want %q", got, "version 1")
+ }
+ if wakes != 1 {
+ t.Fatalf("expected exactly 1 wake for the cold request, got %d", wakes)
+ }
+ if got := get(); got != "version 1" || wakes != 1 {
+ t.Fatalf("second request body = %q wakes = %d, want %q and no second wake", got, wakes, "version 1")
+ }
+
+ // The write happens, and the instance purges its own page over the control
+ // socket — exactly the request a real instance makes.
+ body = "version 2"
+ payload, _ := json.Marshal(purgeRequest{Path: "/event", Query: ""})
+ resp, err := dialPurgeSocket(t, controlSocket).Post("http://unix/purge", "application/json", bytes.NewReader(payload))
+ if err != nil {
+ t.Fatalf("purge POST over control socket: %v", err)
+ }
+ resp.Body.Close()
+ if resp.StatusCode != http.StatusOK {
+ t.Fatalf("purge status = %d, want 200", resp.StatusCode)
+ }
+
+ if got := get(); got != "version 2" {
+ t.Fatalf("post-purge request body = %q, want %q — the purge never reached the cache through the socket", got, "version 2")
+ }
+ if wakes != 2 {
+ t.Fatalf("expected the post-purge request to miss and wake once more, wakes = %d", wakes)
+ }
+}
+
// TestCacheBeforeWakeFullCycle exercises the sequence the whole feature
// exists for: a cold miss wakes the instance and populates the cache; a
// repeat request hits the cache and does NOT wake the instance again; a