Commit 65a2f09
Eric Bower
·
2026-04-28 12:57:39 -0400 EDT
parent 66ac6b0
fix(httpcache): cdn accept encoding
Two bugs combined to produce the broken behavior:
Bug 1: CDN forwarded Accept-Encoding to upstream (cmd/pgs/cdn/main.go)
proxyServe.ServeHTTP cloned the incoming request (including its Accept-Encoding: zstd header sent by Caddy) and forwarded it to ash.pgs.sh. The upstream responded with a zstd-compressed body + content-encoding: zstd. The CDN then cached that compressed blob. Caddy (sitting in front of the CDN) can't transcode
zstd→nothing, so clients received raw zstd bytes named .xml.
Fix: proxyReq.Header.Del("Accept-Encoding") before the upstream round-trip. The CDN should store one uncompressed representation per URL; Caddy handles per-client content encoding on egress.
Bug 2: matchVary was looking in the wrong place (pkg/httpcache/serve.go, rw.go)
When the response included Vary: Accept-Encoding, matchVary looked for Accept-Encoding in the response headers map — but Accept-Encoding is a request header and was never there. The cachedValue == "" branch silently continued, causing every request to match regardless of what encoding it accepted.
Fix: ToCacheValue now accepts the *http.Request and snapshots the Vary-relevant request header values into a new CacheValue.VaryRequestHeaders map[string]string field. matchVary now compares the incoming request's headers against that snapshot instead of looking in response headers. Legacy entries with no
VaryRequestHeaders are treated as misses so they repopulate correctly.
4 files changed,
+57,
-38
+6,
-0
| ... | ... | @@ -86,6 +86,12 @@ func (p *proxyServe) ServeHTTP(w http.ResponseWriter, req *http.Request) { | |
| 86 | 86 | proxyReq.URL.Path = target.Path | |
| 87 | 87 | proxyReq.URL.RawQuery = target.RawQuery | |
| 88 | 88 | proxyReq.RequestURI = "" | |
| 89 | + | // Prevent the upstream from returning a compressed body. The CDN cache | |
| 90 | + | // stores a single representation per URL; Caddy handles per-client | |
| 91 | + | // encoding on the way out. If we forward Accept-Encoding, origin may | |
| 92 | + | // return zstd-compressed bytes that get cached and then served to | |
| 93 | + | // clients that never requested zstd. | |
| 94 | + | proxyReq.Header.Del("Accept-Encoding") | |
| 89 | 95 | // Preserve the original Host header so ash.pgs.sh routes correctly. | |
| 90 | 96 | proxyReq.Host = req.Host | |
| 91 | 97 |
+3,
-2
| ... | ... | @@ -242,8 +242,9 @@ func TestCacheVary(t *testing.T) { | |
| 242 | 242 | cacheKey := handler.GetCacheKey(req) | |
| 243 | 243 | cv := testCacheValue(250 * time.Second) | |
| 244 | 244 | cv.Header["Vary"] = []string{"Accept-Encoding"} | |
| 245 | - | // Store the original request header that selected this representation. | |
| 246 | - | cv.Header["Accept-Encoding"] = []string{"gzip"} | |
| 245 | + | // VaryRequestHeaders snapshots the request header values that were present | |
| 246 | + | // when this entry was cached, keyed by the lowercase header name. | |
| 247 | + | cv.VaryRequestHeaders = map[string]string{"accept-encoding": "gzip"} | |
| 247 | 248 | cacheValue, _ := json.Marshal(cv) | |
| 248 | 249 | handler.Cache.Add(cacheKey, cacheValue) | |
| 249 | 250 |
+25,
-9
| ... | ... | @@ -43,26 +43,42 @@ func (rw *responseWriter) Send() { | |
| 43 | 43 | _, _ = rw.ResponseWriter.Write(rw.body) | |
| 44 | 44 | } | |
| 45 | 45 | ||
| 46 | - | func (rw *responseWriter) ToCacheValue() *CacheValue { | |
| 46 | + | func (rw *responseWriter) ToCacheValue(r *http.Request) *CacheValue { | |
| 47 | 47 | // Normalize header keys to lowercase to avoid case-sensitivity issues | |
| 48 | 48 | // in the cached map (e.g., "ETag" vs "Etag" as separate keys). | |
| 49 | 49 | headers := make(map[string][]string) | |
| 50 | 50 | for k, v := range rw.Header() { | |
| 51 | 51 | headers[strings.ToLower(k)] = v | |
| 52 | 52 | } | |
| 53 | + | ||
| 54 | + | // Snapshot the request header values named by the response Vary header so | |
| 55 | + | // matchVary can compare them on future cache lookups (Vary lists request | |
| 56 | + | // header names, not response header names). | |
| 57 | + | varyReqHdrs := make(map[string]string) | |
| 58 | + | if vary := headers["vary"]; len(vary) > 0 { | |
| 59 | + | for _, field := range strings.FieldsFunc(vary[0], func(c rune) bool { return c == ',' }) { | |
| 60 | + | field = strings.TrimSpace(strings.ToLower(field)) | |
| 61 | + | if field != "" && field != "*" { | |
| 62 | + | varyReqHdrs[field] = r.Header.Get(field) | |
| 63 | + | } | |
| 64 | + | } | |
| 65 | + | } | |
| 66 | + | ||
| 53 | 67 | cv := &CacheValue{ | |
| 54 | - | Header: headers, | |
| 55 | - | Body: rw.body, | |
| 56 | - | CreatedAt: time.Now(), | |
| 57 | - | StatusCode: rw.StatusCode(), | |
| 68 | + | Header: headers, | |
| 69 | + | Body: rw.body, | |
| 70 | + | CreatedAt: time.Now(), | |
| 71 | + | StatusCode: rw.StatusCode(), | |
| 72 | + | VaryRequestHeaders: varyReqHdrs, | |
| 58 | 73 | } | |
| 59 | 74 | ||
| 60 | 75 | return cv | |
| 61 | 76 | } | |
| 62 | 77 | ||
| 63 | 78 | type CacheValue struct { | |
| 64 | - | Header map[string][]string `json:"headers"` | |
| 65 | - | Body []byte `json:"body"` | |
| 66 | - | CreatedAt time.Time `json:"created_at"` | |
| 67 | - | StatusCode int `json:"status_code"` | |
| 79 | + | Header map[string][]string `json:"headers"` | |
| 80 | + | Body []byte `json:"body"` | |
| 81 | + | CreatedAt time.Time `json:"created_at"` | |
| 82 | + | StatusCode int `json:"status_code"` | |
| 83 | + | VaryRequestHeaders map[string]string `json:"vary_request_headers,omitempty"` | |
| 68 | 84 | } |
+23,
-27
| ... | ... | @@ -186,7 +186,7 @@ func (c *HttpCache) ServeHTTP(w http.ResponseWriter, r *http.Request) { | |
| 186 | 186 | err = isResponseCachable(r, wrapped) | |
| 187 | 187 | if err == nil { | |
| 188 | 188 | log.Info("storing cache") | |
| 189 | - | nextValue := wrapped.ToCacheValue() | |
| 189 | + | nextValue := wrapped.ToCacheValue(r) | |
| 190 | 190 | enc, _ := json.Marshal(nextValue) | |
| 191 | 191 | c.Cache.Remove(cacheKey) | |
| 192 | 192 | c.Cache.Add(cacheKey, enc) |
| ... | ... | @@ -227,47 +227,43 @@ func serveCache(w http.ResponseWriter, freshness time.Duration, cacheKey string, | |
| 227 | 227 | _, _ = w.Write(cacheValue.Body) | |
| 228 | 228 | } | |
| 229 | 229 | ||
| 230 | - | // matchVary checks if the request matches the Vary header from the cached response | |
| 230 | + | // matchVary checks if the request matches the Vary header from the cached response. | |
| 231 | 231 | // RFC 9111 4.1 Vary. | |
| 232 | - | func matchVary(r *http.Request, cachedHeaders map[string][]string) bool { | |
| 233 | - | vary := getHeader(cachedHeaders, "Vary") | |
| 232 | + | // | |
| 233 | + | // Vary lists *request* header names, so we compare the incoming request headers | |
| 234 | + | // against the request header values that were snapshotted when the entry was | |
| 235 | + | // stored (CacheValue.VaryRequestHeaders). Comparing against response headers | |
| 236 | + | // (the old approach) silently skipped every field because request header names | |
| 237 | + | // never appear in response header maps. | |
| 238 | + | func matchVary(r *http.Request, cacheValue *CacheValue) bool { | |
| 239 | + | vary := getHeader(cacheValue.Header, "Vary") | |
| 234 | 240 | if vary == "" { | |
| 235 | 241 | return true | |
| 236 | 242 | } | |
| 237 | 243 | ||
| 238 | - | // Vary: * means the response is not cacheable | |
| 244 | + | // Vary: * means the response is not cacheable by a shared cache. | |
| 239 | 245 | if vary == "*" { | |
| 240 | 246 | return false | |
| 241 | 247 | } | |
| 242 | 248 | ||
| 243 | - | // Parse Vary header and check each field | |
| 244 | - | fields := strings.FieldsFunc(vary, func(r rune) bool { | |
| 245 | - | return r == ',' | |
| 246 | - | }) | |
| 249 | + | // If the entry predates VaryRequestHeaders (legacy/zero value), fall back to | |
| 250 | + | // treating it as a miss so the cache re-populates with a fresh entry. | |
| 251 | + | if len(cacheValue.VaryRequestHeaders) == 0 { | |
| 252 | + | return false | |
| 253 | + | } | |
| 247 | 254 | ||
| 255 | + | fields := strings.FieldsFunc(vary, func(c rune) bool { return c == ',' }) | |
| 248 | 256 | for _, field := range fields { | |
| 249 | 257 | field = strings.TrimSpace(strings.ToLower(field)) | |
| 250 | 258 | if field == "" { | |
| 251 | 259 | continue | |
| 252 | 260 | } | |
| 253 | - | ||
| 254 | - | // Get the request header value | |
| 255 | - | reqValue := r.Header.Get(field) | |
| 256 | - | ||
| 257 | - | // Get the cached header value for this field (case-insensitive lookup) | |
| 258 | - | var cachedValue string | |
| 259 | - | for key, values := range cachedHeaders { | |
| 260 | - | if strings.ToLower(key) == field && len(values) > 0 { | |
| 261 | - | cachedValue = values[0] | |
| 262 | - | break | |
| 263 | - | } | |
| 264 | - | } | |
| 265 | - | if cachedValue == "" { | |
| 266 | - | continue | |
| 261 | + | cachedReqVal, known := cacheValue.VaryRequestHeaders[field] | |
| 262 | + | if !known { | |
| 263 | + | // Field listed in Vary but not recorded — treat as miss. | |
| 264 | + | return false | |
| 267 | 265 | } | |
| 268 | - | ||
| 269 | - | // Compare values - must match exactly | |
| 270 | - | if reqValue != cachedValue { | |
| 266 | + | if r.Header.Get(field) != cachedReqVal { | |
| 271 | 267 | return false | |
| 272 | 268 | } | |
| 273 | 269 | } |
| ... | ... | @@ -562,7 +558,7 @@ func (c *HttpCache) maybeUseCache(cacheKey string, w http.ResponseWriter, r *htt | |
| 562 | 558 | } | |
| 563 | 559 | ||
| 564 | 560 | // RFC 9111 4.1 Vary - check if request matches cached Vary values | |
| 565 | - | if !matchVary(r, cacheValue.Header) { | |
| 561 | + | if !matchVary(r, &cacheValue) { | |
| 566 | 562 | return fmt.Errorf("vary mismatch") | |
| 567 | 563 | } | |
| 568 | 564 |