Commit f46c23c
Eric Bower
·
2026-04-21 22:57:35 -0400 EDT
parent 8045405
chore(httpcache): cache-status cleanup and random bug fixes
2 files changed,
+53,
-80
+3,
-3
| ... | ... | @@ -772,11 +772,11 @@ func TestCache304NotModifiedMerge(t *testing.T) { | |
| 772 | 772 | resp1, _ := tc.DoWithHeaders(req, map[string][]string{ | |
| 773 | 773 | "If-None-Match": {"\"abc\""}, | |
| 774 | 774 | }) | |
| 775 | - | if resp1.StatusCode != http.StatusOK { | |
| 776 | - | t.Errorf("expected 200 (ETag changed after revalidation), got %d", resp1.StatusCode) | |
| 775 | + | if resp1.StatusCode != http.StatusNotModified { | |
| 776 | + | t.Errorf("expected 304 (ETag changed after revalidation), got %d", resp1.StatusCode) | |
| 777 | 777 | } | |
| 778 | 778 | status := resp1.Header.Get("cache-status") | |
| 779 | - | if !strings.Contains(status, "hit") { | |
| 779 | + | if !strings.Contains(status, "fwd=stale") { | |
| 780 | 780 | t.Errorf("expected cache-status hit, got %s", status) | |
| 781 | 781 | } | |
| 782 | 782 |
+50,
-77
| ... | ... | @@ -2,6 +2,7 @@ package httpcache | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "encoding/json" | |
| 5 | + | "errors" | |
| 5 | 6 | "fmt" | |
| 6 | 7 | "log/slog" | |
| 7 | 8 | "net/http" |
| ... | ... | @@ -12,6 +13,8 @@ import ( | |
| 12 | 13 | "github.com/hashicorp/golang-lru/v2/expirable" | |
| 13 | 14 | ) | |
| 14 | 15 | ||
| 16 | + | var ErrMustRevalidate = errors.New("cache is stale and must-revalidate requires revalidation") | |
| 17 | + | ||
| 15 | 18 | type CacheKey interface { | |
| 16 | 19 | GetCacheKey(r *http.Request) string | |
| 17 | 20 | } |
| ... | ... | @@ -99,19 +102,19 @@ func (c *HttpCache) ServeHTTP(w http.ResponseWriter, r *http.Request) { | |
| 99 | 102 | // revalidated with conditional headers derived from the stored response. | |
| 100 | 103 | // Preserve original client conditional headers so we can evaluate them | |
| 101 | 104 | // after revalidation to decide whether the client gets 304 or 200. | |
| 102 | - | clientIfNoneMatch := r.Header.Get("If-None-Match") | |
| 103 | - | clientIfModifiedSince := r.Header.Get("If-Modified-Since") | |
| 105 | + | clientIfNoneMatch := r.Header.Get("if-none-match") | |
| 106 | + | clientIfModifiedSince := r.Header.Get("if-modified-since") | |
| 104 | 107 | clientConditional := clientIfNoneMatch != "" || clientIfModifiedSince != "" | |
| 105 | 108 | ||
| 106 | - | if err.Error() == "cache is stale and must-revalidate requires revalidation" { | |
| 109 | + | if errors.Is(err, ErrMustRevalidate) { | |
| 107 | 110 | if cachedData, exists := c.Cache.Get(cacheKey); exists { | |
| 108 | 111 | var cachedValue CacheValue | |
| 109 | 112 | if json.Unmarshal(cachedData, &cachedValue) == nil { | |
| 110 | - | if etag := getHeader(cachedValue.Header, "ETag"); etag != "" { | |
| 111 | - | r.Header.Set("If-None-Match", etag) | |
| 113 | + | if etag := getHeader(cachedValue.Header, "etag"); etag != "" { | |
| 114 | + | r.Header.Set("if-none-match", etag) | |
| 112 | 115 | } | |
| 113 | - | if lastMod := getHeader(cachedValue.Header, "Last-Modified"); lastMod != "" { | |
| 114 | - | r.Header.Set("If-Modified-Since", lastMod) | |
| 116 | + | if lastMod := getHeader(cachedValue.Header, "last-modified"); lastMod != "" { | |
| 117 | + | r.Header.Set("if-modified-since", lastMod) | |
| 115 | 118 | } | |
| 116 | 119 | } | |
| 117 | 120 | } |
| ... | ... | @@ -157,21 +160,15 @@ func (c *HttpCache) ServeHTTP(w http.ResponseWriter, r *http.Request) { | |
| 157 | 160 | if clientConditional { | |
| 158 | 161 | // Client sent conditional headers -- re-evaluate against the | |
| 159 | 162 | // updated cached entry and return 304 if it still matches. | |
| 160 | - | r.Header.Set("If-None-Match", clientIfNoneMatch) | |
| 161 | - | r.Header.Set("If-Modified-Since", clientIfModifiedSince) | |
| 162 | - | valid, status := c.handleValidation(r, &cacheValue) | |
| 163 | + | r.Header.Set("if-none-match", clientIfNoneMatch) | |
| 164 | + | r.Header.Set("if-modified-since", clientIfModifiedSince) | |
| 165 | + | valid := c.handleValidation(r, &cacheValue) | |
| 163 | 166 | if valid { | |
| 164 | - | hdr := w.Header() | |
| 165 | - | for key, values := range cacheValue.Header { | |
| 166 | - | if isForbiddenHeader(key) { | |
| 167 | - | continue | |
| 168 | - | } | |
| 169 | - | hdr[key] = values | |
| 170 | - | } | |
| 167 | + | hdr := stripForbiddenHeaders(w, &cacheValue) | |
| 171 | 168 | ageDur := calcAge(cacheValue.CreatedAt) | |
| 172 | 169 | hdr.Set("age", strconv.Itoa(int(ageDur.Seconds())+1)) | |
| 173 | - | hdr.Set("cache-status", cacheStatusHit(cacheKey, c.Ttl.Seconds())) | |
| 174 | - | w.WriteHeader(status) | |
| 170 | + | hdr.Set("cache-status", cacheStatusStale(cacheKey, wrapped.StatusCode())) | |
| 171 | + | w.WriteHeader(http.StatusNotModified) | |
| 175 | 172 | return | |
| 176 | 173 | } | |
| 177 | 174 | } |
| ... | ... | @@ -213,15 +210,7 @@ func isForbiddenHeader(key string) bool { | |
| 213 | 210 | } | |
| 214 | 211 | ||
| 215 | 212 | func serveCache(w http.ResponseWriter, freshness time.Duration, cacheKey string, cacheValue *CacheValue) { | |
| 216 | - | hdr := w.Header() | |
| 217 | - | for key, values := range cacheValue.Header { | |
| 218 | - | // RFC 9111 3.1 - Skip forbidden headers | |
| 219 | - | if isForbiddenHeader(key) { | |
| 220 | - | continue | |
| 221 | - | } | |
| 222 | - | hdr[key] = values | |
| 223 | - | } | |
| 224 | - | ||
| 213 | + | hdr := stripForbiddenHeaders(w, cacheValue) | |
| 225 | 214 | ageDur := calcAge(cacheValue.CreatedAt) | |
| 226 | 215 | age := ageDur.Seconds() | |
| 227 | 216 | hdr.Set("age", strconv.Itoa(int(age)+1)) |
| ... | ... | @@ -293,30 +282,12 @@ func getHeader(headers map[string][]string, key string) string { | |
| 293 | 282 | // handleValidation handles conditional request validation. | |
| 294 | 283 | // RFC 9110 13 Conditional Requests. | |
| 295 | 284 | // RFC 9111 4.3.2 Response Validation. | |
| 296 | - | func (c *HttpCache) handleValidation(r *http.Request, cacheValue *CacheValue) (bool, int) { | |
| 297 | - | // Get ETag and Last-Modified with case-insensitive lookup | |
| 298 | - | var etag string | |
| 299 | - | var lastModified string | |
| 300 | - | for key, values := range cacheValue.Header { | |
| 301 | - | lowerKey := strings.ToLower(key) | |
| 302 | - | c.Logger.Debug( | |
| 303 | - | "validate", | |
| 304 | - | "key", key, | |
| 305 | - | "lowerKey", lowerKey, | |
| 306 | - | "values", values, | |
| 307 | - | "etag", etag, | |
| 308 | - | "lastModified", lastModified, | |
| 309 | - | ) | |
| 310 | - | if lowerKey == "etag" && len(values) > 0 { | |
| 311 | - | etag = values[0] | |
| 312 | - | } | |
| 313 | - | if lowerKey == "last-modified" && len(values) > 0 { | |
| 314 | - | lastModified = values[0] | |
| 315 | - | } | |
| 316 | - | } | |
| 285 | + | func (c *HttpCache) handleValidation(r *http.Request, cacheValue *CacheValue) bool { | |
| 286 | + | etag := getHeader(cacheValue.Header, "etag") | |
| 287 | + | lastModified := getHeader(cacheValue.Header, "last-modified") | |
| 317 | 288 | ||
| 318 | 289 | c.Logger.Debug( | |
| 319 | - | "validate result", | |
| 290 | + | "validate", | |
| 320 | 291 | "etag", etag, | |
| 321 | 292 | "lastModified", lastModified, | |
| 322 | 293 | ) |
| ... | ... | @@ -327,20 +298,17 @@ func (c *HttpCache) handleValidation(r *http.Request, cacheValue *CacheValue) (b | |
| 327 | 298 | if ifNoneMatch != "" { | |
| 328 | 299 | // Wildcard If-None-Match: * | |
| 329 | 300 | if ifNoneMatch == "*" { | |
| 330 | - | if etag != "" { | |
| 331 | - | return true, http.StatusNotModified | |
| 332 | - | } | |
| 333 | - | return false, 0 | |
| 301 | + | return etag != "" | |
| 334 | 302 | } | |
| 335 | 303 | ||
| 336 | 304 | // Check if any of the provided ETags match | |
| 337 | 305 | etags := parseETags(ifNoneMatch) | |
| 338 | 306 | for _, etagVal := range etags { | |
| 339 | 307 | if etagVal == etag { | |
| 340 | - | return true, http.StatusNotModified | |
| 308 | + | return true | |
| 341 | 309 | } | |
| 342 | 310 | } | |
| 343 | - | return false, 0 | |
| 311 | + | return false | |
| 344 | 312 | } | |
| 345 | 313 | ||
| 346 | 314 | // RFC 9110 13.1.3 If-Modified-Since |
| ... | ... | @@ -352,7 +320,7 @@ func (c *HttpCache) handleValidation(r *http.Request, cacheValue *CacheValue) (b | |
| 352 | 320 | cachedTime := parseTimeFallback(lastModified) | |
| 353 | 321 | if !cachedTime.IsZero() { | |
| 354 | 322 | if !cachedTime.After(reqTime) { | |
| 355 | - | return true, http.StatusNotModified | |
| 323 | + | return true | |
| 356 | 324 | } | |
| 357 | 325 | } | |
| 358 | 326 | } |
| ... | ... | @@ -371,13 +339,13 @@ func (c *HttpCache) handleValidation(r *http.Request, cacheValue *CacheValue) (b | |
| 371 | 339 | // Cached response is not modified since the request time | |
| 372 | 340 | // We can serve from cache, but don't return 304 | |
| 373 | 341 | // The caller will handle the cache hit | |
| 374 | - | return false, 0 | |
| 342 | + | return false | |
| 375 | 343 | } | |
| 376 | 344 | } | |
| 377 | 345 | } | |
| 378 | 346 | } | |
| 379 | 347 | ||
| 380 | - | return false, 0 | |
| 348 | + | return false | |
| 381 | 349 | } | |
| 382 | 350 | ||
| 383 | 351 | func parseETags(etags string) []string { |
| ... | ... | @@ -628,7 +596,7 @@ func (c *HttpCache) maybeUseCache(cacheKey string, w http.ResponseWriter, r *htt | |
| 628 | 596 | age := calcAge(cacheValue.CreatedAt) | |
| 629 | 597 | freshness := calcFreshness(cacheContState, expires, age, c.Ttl) | |
| 630 | 598 | if freshness <= 0 { | |
| 631 | - | return fmt.Errorf("cache is stale and must-revalidate requires revalidation") | |
| 599 | + | return ErrMustRevalidate | |
| 632 | 600 | } | |
| 633 | 601 | } | |
| 634 | 602 |
| ... | ... | @@ -640,32 +608,22 @@ func (c *HttpCache) maybeUseCache(cacheKey string, w http.ResponseWriter, r *htt | |
| 640 | 608 | return fmt.Errorf("cache has no-store") | |
| 641 | 609 | } | |
| 642 | 610 | ||
| 611 | + | age := calcAge(cacheValue.CreatedAt) | |
| 612 | + | freshness := calcFreshness(cacheContState, expires, age, c.Ttl) | |
| 613 | + | ||
| 643 | 614 | // RFC 9111 4.3 Validation - check validation headers first | |
| 644 | 615 | // RFC 9110 13 Conditional Requests | |
| 645 | 616 | // https://www.rfc-editor.org/rfc/rfc9110.html#section-13 | |
| 646 | - | valid, status := c.handleValidation(r, &cacheValue) | |
| 617 | + | valid := c.handleValidation(r, &cacheValue) | |
| 647 | 618 | if valid { | |
| 648 | - | // RFC 9111 4.3.4 304 Not Modified | |
| 649 | - | // https://www.rfc-editor.org/rfc/rfc9111.html#section-4.3.4 | |
| 650 | - | // A 304 response must include headers the client needs to update | |
| 651 | - | // its cached representation (ETag, Last-Modified, Cache-Control, etc.) | |
| 652 | - | hdr := w.Header() | |
| 653 | - | for key, values := range cacheValue.Header { | |
| 654 | - | if isForbiddenHeader(key) { | |
| 655 | - | continue | |
| 656 | - | } | |
| 657 | - | hdr[key] = values | |
| 658 | - | } | |
| 619 | + | hdr := stripForbiddenHeaders(w, &cacheValue) | |
| 659 | 620 | ageDur := calcAge(cacheValue.CreatedAt) | |
| 660 | 621 | hdr.Set("age", strconv.Itoa(int(ageDur.Seconds())+1)) | |
| 661 | - | hdr.Set("cache-status", cacheStatusHit(cacheKey, c.Ttl.Seconds())) | |
| 662 | - | w.WriteHeader(status) | |
| 622 | + | hdr.Set("cache-status", cacheStatusHit(cacheKey, freshness.Seconds())) | |
| 623 | + | w.WriteHeader(http.StatusNotModified) | |
| 663 | 624 | return nil | |
| 664 | 625 | } | |
| 665 | 626 | ||
| 666 | - | age := calcAge(cacheValue.CreatedAt) | |
| 667 | - | freshness := calcFreshness(cacheContState, expires, age, c.Ttl) | |
| 668 | - | ||
| 669 | 627 | // Check if request allows stale responses (max-stale) | |
| 670 | 628 | // RFC 9111 5.2.1.2 - max-stale allows serving stale responses | |
| 671 | 629 | // We need to check this before the freshness <= 0 check |
| ... | ... | @@ -758,6 +716,10 @@ func cacheStatusHit(cacheKey string, ttl float64) string { | |
| 758 | 716 | return fmt.Sprintf("pico; hit; ttl=%d; key=%s", int(ttl), cacheKey) | |
| 759 | 717 | } | |
| 760 | 718 | ||
| 719 | + | func cacheStatusStale(cacheKey string, originStatus int) string { | |
| 720 | + | return fmt.Sprintf("pico; fwd=stale; fwd-status=%d", originStatus) | |
| 721 | + | } | |
| 722 | + | ||
| 761 | 723 | func cacheStatusMiss(cacheKey string, stored bool) string { | |
| 762 | 724 | // RFC 9211 2.2 Cache-Status fwd | |
| 763 | 725 | // https://www.rfc-editor.org/rfc/rfc9211#section-2.2 |
| ... | ... | @@ -772,3 +734,14 @@ func cacheStatusMiss(cacheKey string, stored bool) string { | |
| 772 | 734 | status = fmt.Sprintf("%s; key=%s", status, cacheKey) | |
| 773 | 735 | return status | |
| 774 | 736 | } | |
| 737 | + | ||
| 738 | + | func stripForbiddenHeaders(w http.ResponseWriter, cacheValue *CacheValue) http.Header { | |
| 739 | + | hdr := w.Header() | |
| 740 | + | for key, values := range cacheValue.Header { | |
| 741 | + | if isForbiddenHeader(key) { | |
| 742 | + | continue | |
| 743 | + | } | |
| 744 | + | hdr[key] = values | |
| 745 | + | } | |
| 746 | + | return hdr | |
| 747 | + | } |