fix(pgs): httpcache must return cached headers when returning 304

Commit `272d185`

Eric Bower · 2026-04-19 11:20:46 -0400 EDT

parent e20086a

fix(pgs): httpcache must return cached headers when returning 304

2 files changed, +65, -1

M pkg/httpcache/cache_test.go

M pkg/httpcache/serve.go

M pkg/httpcache/cache_test.go

+53, -0

......@@ -914,6 +914,59 @@ func readBody(resp *http.Response) (string, error) {
	return string(buf), nil
}

// Regression: a 304 from cache validation must include the cached response
// headers (ETag, Content-Type, Cache-Control, etc.) so the browser can match
// the 304 to its local cached body. Without them browsers show a blank page.
func TestCache304IncludesCachedHeaders(t *testing.T) {
	logger := slog.Default()
	mux := http.NewServeMux()
	mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
		w.Header().Set("etag", "\"abc\"")
		w.Header().Set("content-type", "text/html; charset=utf-8")
		w.Header().Set("cache-control", "max-age=300")
		w.WriteHeader(200)
		_, _ = w.Write([]byte("<h1>hello</h1>"))
	})

	handler := NewHttpCache(logger, mux)
	tc := NewTestContext(t, handler)
	req, _ := http.NewRequest("GET", tc.cachedServer.URL+"/test", nil)

	// Populate cache with a fresh entry that has ETag, Content-Type, Cache-Control
	cacheKey := handler.GetCacheKey(req)
	cv := testCacheValue(10 * time.Second)
	cv.Header["ETag"] = []string{"\"abc\""}
	cv.Header["Content-Type"] = []string{"text/html; charset=utf-8"}
	cv.Header["Cache-Control"] = []string{"max-age=300"}
	cv.Body = []byte("<h1>hello</h1>")
	cacheData, _ := json.Marshal(cv)
	handler.Cache.Add(cacheKey, cacheData)

	// Send conditional request that triggers a 304 from the cache layer
	resp, _ := tc.DoWithHeaders(req, map[string][]string{
		"If-None-Match": {"\"abc\""},
	})
	if resp.StatusCode != http.StatusNotModified {
		t.Fatalf("expected 304, got %d", resp.StatusCode)
	}

	// The 304 must carry the cached headers so the browser can use them
	// Note: Go's HTTP server strips Content-Type on 304 responses, which is fine
	// per RFC 9110 — the browser already has it from the original 200.
	if got := resp.Header.Get("ETag"); got != "\"abc\"" {
		t.Errorf("expected ETag %q, got %q", "\"abc\"", got)
	}
	if got := resp.Header.Get("Cache-Control"); got != "max-age=300" {
		t.Errorf("expected Cache-Control %q, got %q", "max-age=300", got)
	}

	// Body must be empty per RFC 9110 15.4.5
	body, _ := readBody(resp)
	if body != "" {
		t.Errorf("expected empty body for 304, got %q", body)
	}
}

func TestCacheAgeTtl(t *testing.T) {
	mux := http.NewServeMux()
	mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {

M pkg/httpcache/serve.go

+12, -1

......@@ -594,7 +594,18 @@ func (c *HttpCache) maybeUseCache(cacheKey string, w http.ResponseWriter, r *htt
	if valid {
		// RFC 9111 4.3.4 304 Not Modified
		// https://www.rfc-editor.org/rfc/rfc9111.html#section-4.3.4
		w.Header().Set("cache-status", cacheStatusHit(cacheKey, c.Ttl.Seconds()))
		// A 304 response must include headers the client needs to update
		// its cached representation (ETag, Last-Modified, Cache-Control, etc.)
		hdr := w.Header()
		for key, values := range cacheValue.Header {
			if isForbiddenHeader(key) {
				continue
			}
			for _, value := range values {
				hdr.Add(key, value)
			}
		}
		hdr.Set("cache-status", cacheStatusHit(cacheKey, c.Ttl.Seconds()))
		w.WriteHeader(status)
		return nil
	}

...	...		@@ -914,6 +914,59 @@ func readBody(resp *http.Response) (string, error) {
914	914		return string(buf), nil
915	915		}
916	916
	917	+	// Regression: a 304 from cache validation must include the cached response
	918	+	// headers (ETag, Content-Type, Cache-Control, etc.) so the browser can match
	919	+	// the 304 to its local cached body. Without them browsers show a blank page.
	920	+	func TestCache304IncludesCachedHeaders(t *testing.T) {
	921	+	logger := slog.Default()
	922	+	mux := http.NewServeMux()
	923	+	mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
	924	+	w.Header().Set("etag", "\"abc\"")
	925	+	w.Header().Set("content-type", "text/html; charset=utf-8")
	926	+	w.Header().Set("cache-control", "max-age=300")
	927	+	w.WriteHeader(200)
	928	+	_, _ = w.Write([]byte("<h1>hello</h1>"))
	929	+	})
	930	+
	931	+	handler := NewHttpCache(logger, mux)
	932	+	tc := NewTestContext(t, handler)
	933	+	req, _ := http.NewRequest("GET", tc.cachedServer.URL+"/test", nil)
	934	+
	935	+	// Populate cache with a fresh entry that has ETag, Content-Type, Cache-Control
	936	+	cacheKey := handler.GetCacheKey(req)
	937	+	cv := testCacheValue(10 * time.Second)
	938	+	cv.Header["ETag"] = []string{"\"abc\""}
	939	+	cv.Header["Content-Type"] = []string{"text/html; charset=utf-8"}
	940	+	cv.Header["Cache-Control"] = []string{"max-age=300"}
	941	+	cv.Body = []byte("<h1>hello</h1>")
	942	+	cacheData, _ := json.Marshal(cv)
	943	+	handler.Cache.Add(cacheKey, cacheData)
	944	+
	945	+	// Send conditional request that triggers a 304 from the cache layer
	946	+	resp, _ := tc.DoWithHeaders(req, map[string][]string{
	947	+	"If-None-Match": {"\"abc\""},
	948	+	})
	949	+	if resp.StatusCode != http.StatusNotModified {
	950	+	t.Fatalf("expected 304, got %d", resp.StatusCode)
	951	+	}
	952	+
	953	+	// The 304 must carry the cached headers so the browser can use them
	954	+	// Note: Go's HTTP server strips Content-Type on 304 responses, which is fine
	955	+	// per RFC 9110 — the browser already has it from the original 200.
	956	+	if got := resp.Header.Get("ETag"); got != "\"abc\"" {
	957	+	t.Errorf("expected ETag %q, got %q", "\"abc\"", got)
	958	+	}
	959	+	if got := resp.Header.Get("Cache-Control"); got != "max-age=300" {
	960	+	t.Errorf("expected Cache-Control %q, got %q", "max-age=300", got)
	961	+	}
	962	+
	963	+	// Body must be empty per RFC 9110 15.4.5
	964	+	body, _ := readBody(resp)
	965	+	if body != "" {
	966	+	t.Errorf("expected empty body for 304, got %q", body)
	967	+	}
	968	+	}
	969	+
917	970		func TestCacheAgeTtl(t *testing.T) {
918	971		mux := http.NewServeMux()
919	972		mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {

...	...		@@ -594,7 +594,18 @@ func (c HttpCache) maybeUseCache(cacheKey string, w http.ResponseWriter, r htt
594	594		if valid {
595	595		// RFC 9111 4.3.4 304 Not Modified
596	596		// https://www.rfc-editor.org/rfc/rfc9111.html#section-4.3.4
597		-	w.Header().Set("cache-status", cacheStatusHit(cacheKey, c.Ttl.Seconds()))
	597	+	// A 304 response must include headers the client needs to update
	598	+	// its cached representation (ETag, Last-Modified, Cache-Control, etc.)
	599	+	hdr := w.Header()
	600	+	for key, values := range cacheValue.Header {
	601	+	if isForbiddenHeader(key) {
	602	+	continue
	603	+	}
	604	+	for _, value := range values {
	605	+	hdr.Add(key, value)
	606	+	}
	607	+	}
	608	+	hdr.Set("cache-status", cacheStatusHit(cacheKey, c.Ttl.Seconds()))
598	609		w.WriteHeader(status)
599	610		return nil
600	611		}

Commit 272d185

Commit `272d185`