diff --git a/internal/handler/chef.go b/internal/handler/chef.go index 72839920..764d8a36 100644 --- a/internal/handler/chef.go +++ b/internal/handler/chef.go @@ -162,6 +162,7 @@ func (h *ChefHandler) serveDocument(w http.ResponseWriter, r *http.Request, docu http.Error(w, "invalid Chef Supermarket response", http.StatusBadGateway) return } + markStale(w, upstream.stale) serveRewrittenMetadata(w, r, rewritten) } diff --git a/internal/handler/composer.go b/internal/handler/composer.go index 5bc2586f..643b287f 100644 --- a/internal/handler/composer.go +++ b/internal/handler/composer.go @@ -189,11 +189,13 @@ func (h *ComposerHandler) handlePackageMetadata(w http.ResponseWriter, r *http.R return // the client left while waiting on a shared rewrite } h.proxy.Logger.Warn("failed to rewrite metadata, proxying original", "error", err) + markStale(w, upstream.stale) w.Header().Set(headerContentType, "application/json") _, _ = w.Write(upstream.body) return } + markStale(w, upstream.stale) serveRewrittenMetadata(w, r, rewritten) } diff --git a/internal/handler/composer_hosted.go b/internal/handler/composer_hosted.go index 9a27e6fc..689cf8f9 100644 --- a/internal/handler/composer_hosted.go +++ b/internal/handler/composer_hosted.go @@ -99,6 +99,7 @@ func (h *ComposerHandler) serveMetadataWithLocal(w http.ResponseWriter, r *http. http.Error(w, "failed to rewrite package metadata", http.StatusBadGateway) return } + markStale(w, upstream.stale) serveRewrittenMetadata(w, r, h.proxy.uncachedRewrite("composer", h.proxyURL, cacheKey, out)) } diff --git a/internal/handler/handler.go b/internal/handler/handler.go index d29508eb..6419d17a 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -1197,6 +1197,17 @@ func JSONError(w http.ResponseWriter, status int, message string) { _, _ = fmt.Fprintf(w, `{"error":%q}`, message) } +// staleWarning marks a response served from the metadata cache because the +// upstream could not be reached. +const staleWarning = `110 - "Response is Stale"` + +// markStale sets the stale warning on a metadata response when stale is set. +func markStale(w http.ResponseWriter, stale bool) { + if stale { + w.Header().Set("Warning", staleWarning) + } +} + // ErrUpstreamNotFound indicates the upstream returned 404. var ErrUpstreamNotFound = fmt.Errorf("upstream: %w", fetch.ErrNotFound) @@ -1816,9 +1827,7 @@ func (p *Proxy) writeMetadataCachedResponseWithEncoding(w http.ResponseWriter, r if contentEncoding != "" { w.Header().Set(headerContentEncoding, contentEncoding) } - if cm.stale { - w.Header().Set("Warning", `110 - "Response is Stale"`) - } + markStale(w, cm.stale) w.WriteHeader(http.StatusOK) if r.Method != http.MethodHead { _, _ = w.Write(body) diff --git a/internal/handler/npm_hosted.go b/internal/handler/npm_hosted.go index 65b59ba3..7532cbdd 100644 --- a/internal/handler/npm_hosted.go +++ b/internal/handler/npm_hosted.go @@ -123,6 +123,7 @@ func (h *NPMHandler) serveMetadataWithLocal(w http.ResponseWriter, r *http.Reque JSONError(w, http.StatusBadGateway, "failed to rewrite package metadata") return } + markStale(w, upstream.stale) serveRewrittenMetadata(w, r, h.proxy.uncachedRewrite("npm", h.proxyURL, metadataCacheKey, out)) } diff --git a/internal/handler/stale_metadata_test.go b/internal/handler/stale_metadata_test.go new file mode 100644 index 00000000..9cd8e754 --- /dev/null +++ b/internal/handler/stale_metadata_test.go @@ -0,0 +1,94 @@ +package handler + +import ( + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" +) + +// upstreamThen serves body once and answers every later request with status. +func upstreamThen(t *testing.T, body string, status int) *httptest.Server { + t.Helper() + var requests atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if requests.Add(1) == 1 { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(body)) + return + } + w.WriteHeader(status) + })) + t.Cleanup(srv.Close) + return srv +} + +// staleProxy returns a proxy whose cached metadata expires right away. +func staleProxy(t *testing.T, upstream *httptest.Server) *Proxy { + t.Helper() + proxy, _, _, _ := setupTestProxy(t) + proxy.CacheMetadata = true + proxy.MetadataTTL = time.Millisecond + proxy.HTTPClient = upstream.Client() + return proxy +} + +// requestTwice requests path, lets the cached copy expire, and requests it +// again. +func requestTwice(t *testing.T, h http.Handler, path string) *httptest.ResponseRecorder { + t.Helper() + first := httptest.NewRecorder() + h.ServeHTTP(first, httptest.NewRequest(http.MethodGet, path, nil)) + if first.Code != http.StatusOK { + t.Fatalf("first request: status = %d, body %s", first.Code, first.Body) + } + time.Sleep(5 * time.Millisecond) + w := httptest.NewRecorder() + h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, path, nil)) + return w +} + +// staleStatuses are upstream answers the metadata cache falls back on. +var staleStatuses = []int{http.StatusBadGateway, http.StatusNotFound, http.StatusUnauthorized, http.StatusForbidden} + +const staleComposerDocument = `{"packages":{"acme/widget":[{"name":"acme/widget","version":"1.0.0",` + + `"dist":{"type":"zip","url":"https://repo.packagist.org/files/acme/widget/1.0.0/widget.zip"}}]},"minified":"composer/2.0"}` + +func TestComposerStaleMetadataIsMarked(t *testing.T) { + for _, status := range staleStatuses { + t.Run(http.StatusText(status), func(t *testing.T) { + upstream := upstreamThen(t, staleComposerDocument, status) + h := NewComposerHandlerWithUpstreams(staleProxy(t, upstream), "http://proxy.test", upstream.URL, upstream.URL).Routes() + + w := requestTwice(t, h, "/p2/acme/widget.json") + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 from the cache, body %s", w.Code, w.Body) + } + if got := w.Header().Get("Warning"); got != staleWarning { + t.Errorf("Warning = %q, want %q", got, staleWarning) + } + }) + } +} + +const staleChefUniverse = `{"apt":{"7.5.0":{"location_type":"supermarket",` + + `"location_path":"https://supermarket.chef.io/api/v1","download_url":` + + `"https://supermarket.chef.io/api/v1/cookbooks/apt/versions/7.5.0/download","dependencies":{}}}}` + +func TestChefStaleMetadataIsMarked(t *testing.T) { + for _, status := range staleStatuses { + t.Run(http.StatusText(status), func(t *testing.T) { + upstream := upstreamThen(t, staleChefUniverse, status) + h := NewChefHandler(staleProxy(t, upstream), chefTestProxyURL, upstream.URL).Routes() + + w := requestTwice(t, h, "/universe") + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 from the cache, body %s", w.Code, w.Body) + } + if got := w.Header().Get("Warning"); got != staleWarning { + t.Errorf("Warning = %q, want %q", got, staleWarning) + } + }) + } +}