From 871248eda7bcd2d50f18110c38fce83d0a2b729a Mon Sep 17 00:00:00 2001 From: Inhyuk Andy Cho Date: Mon, 5 Jan 2026 09:12:39 +0900 Subject: [PATCH 1/9] fix: update cache on validation --- internal/validationresponsehandler.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/validationresponsehandler.go b/internal/validationresponsehandler.go index 292e705..3c06403 100644 --- a/internal/validationresponsehandler.go +++ b/internal/validationresponsehandler.go @@ -81,6 +81,7 @@ func (r *validationResponseHandler) HandleValidationResponse( // RFC 9111 §4.3.3 Handling Validation Responses (304 Not Modified) // RFC 9111 §4.3.4 Freshening Stored Responses upon Validation updateStoredHeaders(ctx.Stored.Data, resp) + _ = r.rs.StoreResponse(req, ctx.Stored.Data, ctx.URLKey, ctx.Refs, ctx.Start, ctx.End, ctx.RefIndex) CacheStatusRevalidated.ApplyTo(ctx.Stored.Data.Header) r.l.LogCacheRevalidated(req, ctx.URLKey, ctx.ToMisc(nil)) return ctx.Stored.Data, nil From e8f96db7d5465b3d2ffd9da1ff6606c929efe7ec Mon Sep 17 00:00:00 2001 From: Inhyuk Andy Cho Date: Tue, 20 Jan 2026 19:39:01 +0900 Subject: [PATCH 2/9] test: add ResponseStorer for 304 --- internal/validationresponsehandler_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/validationresponsehandler_test.go b/internal/validationresponsehandler_test.go index e2e8740..9c6aafd 100644 --- a/internal/validationresponsehandler_test.go +++ b/internal/validationresponsehandler_test.go @@ -65,6 +65,14 @@ func Test_validationResponseHandler_HandleValidationResponse(t *testing.T) { l: noopLogger, }, setup: func(tt *testing.T, handler *validationResponseHandler) args { + handler.rs = &MockResponseStorer{ + StoreResponseFunc: func(req *http.Request, resp *http.Response, key string, headers ResponseRefs, reqTime, respTime time.Time, refIndex int) error { + testutil.AssertEqual(tt, "key", key) + testutil.AssertTrue(tt, respTime.Equal(base)) + testutil.AssertTrue(tt, reqTime.Equal(base)) + return nil + }, + } return args{ req: &http.Request{Method: http.MethodGet}, resp: &http.Response{StatusCode: http.StatusNotModified, Header: http.Header{}}, From 307a09bbfd07268e9ef51ce1f789914558120fb7 Mon Sep 17 00:00:00 2001 From: Bart Venter Date: Thu, 12 Mar 2026 13:44:57 +0000 Subject: [PATCH 3/9] refactor: rename updateStoredHeaders to mergeResponseHeaders for clarity --- internal/helpers.go | 18 +++++++++--------- internal/validationresponsehandler.go | 12 ++++++++++-- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/internal/helpers.go b/internal/helpers.go index 4913d6c..9b0aa64 100644 --- a/internal/helpers.go +++ b/internal/helpers.go @@ -88,17 +88,17 @@ func removeHopByHopHeaders(resp *http.Response) { } } -// updateStoredHeaders updates the stored response headers with the -// headers from the revalidated response, excluding hop-by-hop headers -// and the Content-Length header, as per RFC 9111 §3.2. -func updateStoredHeaders(storedResp, resp *http.Response) { - omitted := hopByHopHeaders(resp.Header) - omitted["Content-Length"] = struct{}{} - for hdr, val := range resp.Header { - if _, ok := omitted[hdr]; ok { +// mergeResponseHeaders merges the headers from the revalidated response into +// the stored response, excluding hop-by-hop headers and the Content-Length +// header, as per RFC 9111 §3.2. +func mergeResponseHeaders(targetResp *http.Response, srcHdrs http.Header) { + nonCacheHdrs := hopByHopHeaders(srcHdrs) + nonCacheHdrs["Content-Length"] = struct{}{} + for hdr, val := range srcHdrs { + if _, ok := nonCacheHdrs[hdr]; ok { continue } - storedResp.Header[hdr] = val + targetResp.Header[hdr] = val } } diff --git a/internal/validationresponsehandler.go b/internal/validationresponsehandler.go index 3c06403..428c370 100644 --- a/internal/validationresponsehandler.go +++ b/internal/validationresponsehandler.go @@ -80,8 +80,16 @@ func (r *validationResponseHandler) HandleValidationResponse( if err == nil && req.Method == http.MethodGet && resp.StatusCode == http.StatusNotModified { // RFC 9111 §4.3.3 Handling Validation Responses (304 Not Modified) // RFC 9111 §4.3.4 Freshening Stored Responses upon Validation - updateStoredHeaders(ctx.Stored.Data, resp) - _ = r.rs.StoreResponse(req, ctx.Stored.Data, ctx.URLKey, ctx.Refs, ctx.Start, ctx.End, ctx.RefIndex) + mergeResponseHeaders(ctx.Stored.Data, resp.Header) + _ = r.rs.StoreResponse( + req, + ctx.Stored.Data, + ctx.URLKey, + ctx.Refs, + ctx.Start, + ctx.End, + ctx.RefIndex, + ) CacheStatusRevalidated.ApplyTo(ctx.Stored.Data.Header) r.l.LogCacheRevalidated(req, ctx.URLKey, ctx.ToMisc(nil)) return ctx.Stored.Data, nil From ce2dcc1c99aa24a9ba95dbd17fb71f4dd9864e36 Mon Sep 17 00:00:00 2001 From: Bart Venter Date: Thu, 12 Mar 2026 14:33:30 +0000 Subject: [PATCH 4/9] test: add revalidation test to ensure cache updates on 304 responses Closes: #31 --- roundtripper_test.go | 99 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 98 insertions(+), 1 deletion(-) diff --git a/roundtripper_test.go b/roundtripper_test.go index 69771b1..06d3820 100644 --- a/roundtripper_test.go +++ b/roundtripper_test.go @@ -22,13 +22,14 @@ import ( "net/http/httptest" "net/url" "os" + "sync/atomic" "testing" "time" "github.com/bartventer/httpcache/internal" "github.com/bartventer/httpcache/internal/testutil" "github.com/bartventer/httpcache/store" - _ "github.com/bartventer/httpcache/store/memcache" + "github.com/bartventer/httpcache/store/memcache" ) func mockTransport(fields func(rt *transport)) *transport { @@ -860,3 +861,99 @@ func Test_transport_Vary(t *testing.T) { testutil.AssertEqual(t, tc.wantBody, string(body), i) } } + +// This test verifies that when a cached response is revalidated via a 304 Not +// Modified, the cache entry is updated with any new headers from the 304 +// response, and subsequent requests can HIT the cache again until it becomes +// stale once more. +func Test_transport_RevalidationUpdatesCache(t *testing.T) { + var originCalls atomic.Int32 + + const etag = `"v1"` + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + originCalls.Add(1) + + // Revalidation path: client sends validator, server says cached body is still valid + if r.Header.Get("If-None-Match") == etag { + w.Header().Set("ETag", etag) + w.Header().Set("Cache-Control", "max-age=1") + w.Header().Set("Expires", time.Now().Add(1*time.Second).UTC().Format(http.TimeFormat)) + w.WriteHeader(http.StatusNotModified) + return + } + + // Initial fetch + w.Header().Set("ETag", etag) + w.Header().Set("Cache-Control", "max-age=1") + w.Header().Set("Expires", time.Now().Add(1*time.Second).UTC().Format(http.TimeFormat)) + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("hello")) + })) + defer server.Close() + + c := memcache.Open() + tr := newTransport(c) + + req, _ := http.NewRequest(http.MethodGet, server.URL, nil) + + tests := []struct { + name string + expectedStatusCode int + expectedCacheStatus string + expectedBody string + expectedOriginCalls int32 + preReqFunc func() + }{ + { + name: "Initial request should be a MISS", + expectedStatusCode: http.StatusOK, + expectedCacheStatus: internal.CacheStatusMiss.Value, + expectedBody: "hello", + expectedOriginCalls: 1, + }, + { + name: "Second request should be a HIT", + expectedStatusCode: http.StatusOK, + expectedCacheStatus: internal.CacheStatusHit.Value, + expectedBody: "hello", + expectedOriginCalls: 1, + }, + { + name: "After becoming stale, request should be REVALIDATED via 304", + expectedStatusCode: http.StatusOK, + expectedCacheStatus: internal.CacheStatusRevalidated.Value, + expectedBody: "hello", + expectedOriginCalls: 2, + preReqFunc: func() { + time.Sleep(1100 * time.Millisecond) + }, + }, + { + name: "After revalidation, request should be HIT again", + expectedStatusCode: http.StatusOK, + expectedCacheStatus: internal.CacheStatusHit.Value, + expectedBody: "hello", + expectedOriginCalls: 2, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if tc.preReqFunc != nil { + tc.preReqFunc() + } + resp, err := tr.RoundTrip(req) + testutil.RequireNoError(t, err) + testutil.AssertEqual(t, tc.expectedStatusCode, resp.StatusCode) + testutil.AssertEqual( + t, + tc.expectedCacheStatus, + resp.Header.Get(internal.CacheStatusHeader), + ) + body, _ := io.ReadAll(resp.Body) + _ = resp.Body.Close() + testutil.AssertEqual(t, tc.expectedBody, string(body)) + testutil.AssertEqual(t, tc.expectedOriginCalls, originCalls.Load()) + }) + } +} From bb06c97e1a0efd09ece1a97ce6552ca3d6938bde Mon Sep 17 00:00:00 2001 From: Bart Venter Date: Thu, 12 Mar 2026 15:15:16 +0000 Subject: [PATCH 5/9] refactor(store/expapi): suppress false positive from gosec linter --- store/expapi/expapi.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/store/expapi/expapi.go b/store/expapi/expapi.go index 2a3d542..bb81d6d 100644 --- a/store/expapi/expapi.go +++ b/store/expapi/expapi.go @@ -98,12 +98,18 @@ func retrieve(conn driver.Conn) http.Handler { if errors.Is(err, driver.ErrNotExist) { http.Error(w, fmt.Sprintf("key %q not found", key), http.StatusNotFound) } else { - http.Error(w, fmt.Sprintf("failed to get value for key %q: %v", key, err), http.StatusInternalServerError) + http.Error( + w, + fmt.Sprintf("failed to get value for key %q: %v", key, err), + http.StatusInternalServerError, + ) } return } w.Header().Set("Content-Type", "application/octet-stream") + w.Header().Set("X-Content-Type-Options", "nosniff") w.WriteHeader(http.StatusOK) + //nolint:gosec // G705: debug/admin endpoint intentionally returns raw cache bytes (not HTML rendering). if _, err := w.Write(value); err != nil { http.Error( w, @@ -121,7 +127,11 @@ func destroy(conn driver.Conn) http.Handler { if errors.Is(err, driver.ErrNotExist) { http.Error(w, fmt.Sprintf("key %q not found", key), http.StatusNotFound) } else { - http.Error(w, fmt.Sprintf("failed to delete value for key %q: %v", key, err), http.StatusInternalServerError) + http.Error( + w, + fmt.Sprintf("failed to delete value for key %q: %v", key, err), + http.StatusInternalServerError, + ) } return } From d010c2b7b6531bd6bce870c3a83b31ad5862b17a Mon Sep 17 00:00:00 2001 From: Bart Venter Date: Thu, 12 Mar 2026 15:15:52 +0000 Subject: [PATCH 6/9] chore: disable goheader check in golangci-lint --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index bf0962c..f4b9f6b 100644 --- a/Makefile +++ b/Makefile @@ -58,7 +58,7 @@ $(FMTSTAMP): $(GOFILES) $(GOTESTFILES) lint: $(LINTSTAMP) ## Run linters $(LINTSTAMP): $(GOFILES) $(GOTESTFILES) - golangci-lint run --verbose + golangci-lint run --disable goheader --verbose touch $@ ## Testing: From 62e534bd7a19ef3600687d2b4c5106279300f879 Mon Sep 17 00:00:00 2001 From: Bart Venter Date: Thu, 12 Mar 2026 15:30:15 +0000 Subject: [PATCH 7/9] Revert "chore: disable goheader check in golangci-lint" This reverts commit d010c2b7b6531bd6bce870c3a83b31ad5862b17a. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index f4b9f6b..bf0962c 100644 --- a/Makefile +++ b/Makefile @@ -58,7 +58,7 @@ $(FMTSTAMP): $(GOFILES) $(GOTESTFILES) lint: $(LINTSTAMP) ## Run linters $(LINTSTAMP): $(GOFILES) $(GOTESTFILES) - golangci-lint run --disable goheader --verbose + golangci-lint run --verbose touch $@ ## Testing: From 890f7338596584fccc8b71bc0a01c712c5d63d7c Mon Sep 17 00:00:00 2001 From: Bart Venter Date: Thu, 12 Mar 2026 15:32:39 +0000 Subject: [PATCH 8/9] Reapply "chore: disable goheader check in golangci-lint" This reverts commit 62e534bd7a19ef3600687d2b4c5106279300f879. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index bf0962c..f4b9f6b 100644 --- a/Makefile +++ b/Makefile @@ -58,7 +58,7 @@ $(FMTSTAMP): $(GOFILES) $(GOTESTFILES) lint: $(LINTSTAMP) ## Run linters $(LINTSTAMP): $(GOFILES) $(GOTESTFILES) - golangci-lint run --verbose + golangci-lint run --disable goheader --verbose touch $@ ## Testing: From 78454a0741dffd847ad22e6b04e6fef85b711a99 Mon Sep 17 00:00:00 2001 From: Bart Venter Date: Thu, 12 Mar 2026 15:33:13 +0000 Subject: [PATCH 9/9] Revert "refactor(store/expapi): suppress false positive from gosec linter" This reverts commit bb06c97e1a0efd09ece1a97ce6552ca3d6938bde. --- store/expapi/expapi.go | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/store/expapi/expapi.go b/store/expapi/expapi.go index bb81d6d..2a3d542 100644 --- a/store/expapi/expapi.go +++ b/store/expapi/expapi.go @@ -98,18 +98,12 @@ func retrieve(conn driver.Conn) http.Handler { if errors.Is(err, driver.ErrNotExist) { http.Error(w, fmt.Sprintf("key %q not found", key), http.StatusNotFound) } else { - http.Error( - w, - fmt.Sprintf("failed to get value for key %q: %v", key, err), - http.StatusInternalServerError, - ) + http.Error(w, fmt.Sprintf("failed to get value for key %q: %v", key, err), http.StatusInternalServerError) } return } w.Header().Set("Content-Type", "application/octet-stream") - w.Header().Set("X-Content-Type-Options", "nosniff") w.WriteHeader(http.StatusOK) - //nolint:gosec // G705: debug/admin endpoint intentionally returns raw cache bytes (not HTML rendering). if _, err := w.Write(value); err != nil { http.Error( w, @@ -127,11 +121,7 @@ func destroy(conn driver.Conn) http.Handler { if errors.Is(err, driver.ErrNotExist) { http.Error(w, fmt.Sprintf("key %q not found", key), http.StatusNotFound) } else { - http.Error( - w, - fmt.Sprintf("failed to delete value for key %q: %v", key, err), - http.StatusInternalServerError, - ) + http.Error(w, fmt.Sprintf("failed to delete value for key %q: %v", key, err), http.StatusInternalServerError) } return }