From ac7f7243845d7a966d72fd4b1219f3460698ec92 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Thu, 3 Sep 2026 14:46:18 +0200 Subject: [PATCH 01/10] fix(handler): fetch conda repodata gzip-compressed on both hops #304 made the ProxyCached path request Accept-Encoding: identity so the metadata cache stores upstream bytes verbatim. That is required for the signed / hash-pinned index ecosystems, but conda's repodata.json is large plain JSON: linux-64 repodata.json is ~441 MB uncompressed (over the metadata_max_size cap, so it 502s today) versus ~34 MB gzip. Replace the ProxyCached path's verbatim bool with an explicit acceptEncoding string ('' = leave unset / transparent, 'identity', or 'gzip'), reusing #304's existing store-and-replay of Content-Encoding unchanged. ProxyCached keeps its exported signature and continues to send identity, so the nine other ecosystems and helm/maven are untouched; only conda's repodata.json / current_repodata.json now request gzip. Setting Accept-Encoding explicitly disables Go's transparent decompression, so the compressed bytes and the Content-Encoding: gzip header are cached and replayed exactly as identity bytes are. conda, mamba and pixi solicit and decode gzip on .json URLs; repodata.json.bz2 stays identity. Fixes #305 --- internal/handler/conda.go | 21 +++-- internal/handler/conda_test.go | 150 +++++++++++++++++++++++++++++++++ internal/handler/handler.go | 63 ++++++++------ 3 files changed, 204 insertions(+), 30 deletions(-) diff --git a/internal/handler/conda.go b/internal/handler/conda.go index cef814b5..bc7cf5dd 100644 --- a/internal/handler/conda.go +++ b/internal/handler/conda.go @@ -42,7 +42,11 @@ func (h *CondaHandler) Routes() http.Handler { // Channel index (repodata) mux.HandleFunc("GET /{channel}/{arch}/repodata.json", h.handleRepodata) - mux.HandleFunc("GET /{channel}/{arch}/repodata.json.bz2", h.proxyCached) + // .bz2 is already compressed and libmamba only decodes Content-Encoding on + // .json URLs, so keep it identity. + mux.HandleFunc("GET /{channel}/{arch}/repodata.json.bz2", func(w http.ResponseWriter, r *http.Request) { + h.proxyCached(w, r, "identity") + }) mux.HandleFunc("GET /{channel}/{arch}/current_repodata.json", h.handleRepodata) // Package downloads (cache these) @@ -131,7 +135,12 @@ func (h *CondaHandler) parseFilename(filename string) (name, version string) { // handleRepodata proxies repodata.json, applying cooldown filtering when enabled. func (h *CondaHandler) handleRepodata(w http.ResponseWriter, r *http.Request) { if h.proxy.Cooldown == nil || !h.proxy.Cooldown.Enabled() { - h.proxyCached(w, r) + // repodata.json / current_repodata.json are large plain JSON (linux-64 + // repodata.json is ~441 MB uncompressed, over the metadata_max_size cap, + // vs ~34 MB gzip). Request gzip so both hops stay compressed and the + // cache stores the small blob; conda/mamba/pixi decode Content-Encoding + // on .json URLs. See issue #305. + h.proxyCached(w, r, "gzip") return } @@ -236,11 +245,13 @@ func (h *CondaHandler) applyCooldownFiltering(body []byte) ([]byte, error) { return json.Marshal(repodata) } -// proxyCached forwards a metadata request with caching. -func (h *CondaHandler) proxyCached(w http.ResponseWriter, r *http.Request) { +// proxyCached forwards a metadata request with caching, sending the given +// upstream Accept-Encoding ("identity" for verbatim bytes, "gzip" to keep both +// hops compressed for the large .json repodata). +func (h *CondaHandler) proxyCached(w http.ResponseWriter, r *http.Request, acceptEncoding string) { cacheKey := strings.TrimPrefix(r.URL.Path, "/") cacheKey = strings.ReplaceAll(cacheKey, "/", "_") - h.proxy.ProxyCached(w, r, h.upstreamURL+r.URL.Path, "conda", cacheKey, "*/*") + h.proxy.proxyCachedWithEncoding(w, r, h.upstreamURL+r.URL.Path, "conda", cacheKey, acceptEncoding, "*/*") } // proxyUpstream forwards a request to Anaconda without caching. diff --git a/internal/handler/conda_test.go b/internal/handler/conda_test.go index 1b570395..1ba01ebd 100644 --- a/internal/handler/conda_test.go +++ b/internal/handler/conda_test.go @@ -1,10 +1,15 @@ package handler import ( + "bytes" "encoding/json" "log/slog" "net/http" "net/http/httptest" + "strconv" + "strings" + "sync" + "sync/atomic" "testing" "time" @@ -303,3 +308,148 @@ func TestCondaHandleRepodataWithoutCooldown(t *testing.T) { t.Fatalf("status = %d, want %d", w.Code, http.StatusOK) } } + +// TestCondaRepodataRequestsGzip covers issue #305: the large *.json repodata +// routes must fetch, cache and serve gzip-compressed (Content-Encoding: gzip) +// so neither hop pays the uncompressed size, while repodata.json.bz2 (already +// compressed) stays identity. conda/mamba/pixi solicit and decode gzip on .json. +func TestCondaRepodataRequestsGzip(t *testing.T) { + plain := []byte(`{"packages":{},"repodata_version":1}`) + compressed := gzipPayload(t, plain) + + var available atomic.Bool + available.Store(true) + var jsonUpstreamReqs atomic.Int32 + var sawAcceptEncoding sync.Map // path -> last Accept-Encoding seen + + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + sawAcceptEncoding.Store(r.URL.Path, r.Header.Get(headerAcceptEncoding)) + if !available.Load() { + http.Error(w, "unavailable", http.StatusServiceUnavailable) + return + } + if strings.HasSuffix(r.URL.Path, ".json") { + jsonUpstreamReqs.Add(1) + if strings.Contains(r.Header.Get(headerAcceptEncoding), "gzip") { + w.Header().Set(headerContentType, contentTypeJSON) + w.Header().Set(headerContentEncoding, "gzip") + _, _ = w.Write(compressed) + return + } + w.Header().Set(headerContentType, contentTypeJSON) + _, _ = w.Write(plain) + return + } + // .bz2: already compressed, upstream sends no Content-Encoding. + w.Header().Set(headerContentType, "application/octet-stream") + _, _ = w.Write([]byte("bz2-bytes")) + })) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + proxy.CacheMetadata = true + proxy.MetadataTTL = time.Hour + proxy.HTTPClient = upstream.Client() + routes := NewCondaHandlerWithUpstream(proxy, "http://proxy.local", upstream.URL).Routes() + + get := func(path string) *httptest.ResponseRecorder { + w := httptest.NewRecorder() + routes.ServeHTTP(w, httptest.NewRequest(http.MethodGet, path, nil)) + return w + } + lastAE := func(path string) string { + v, _ := sawAcceptEncoding.Load(path) + s, _ := v.(string) + return s + } + + for _, name := range []string{"repodata.json", "current_repodata.json"} { + path := "/conda-forge/linux-64/" + name + w := get(path) + if w.Code != http.StatusOK { + t.Fatalf("%s: status = %d, want 200: %s", name, w.Code, w.Body.String()) + } + if got := lastAE(path); got != "gzip" { + t.Errorf("%s: upstream Accept-Encoding = %q, want %q", name, got, "gzip") + } + if !bytes.Equal(w.Body.Bytes(), compressed) { + t.Errorf("%s: body not the compressed bytes (got %d, want %d)", name, w.Body.Len(), len(compressed)) + } + if got := w.Header().Get(headerContentEncoding); got != "gzip" { + t.Errorf("%s: Content-Encoding = %q, want %q", name, got, "gzip") + } + if got := w.Header().Get(headerContentLength); got != strconv.Itoa(len(compressed)) { + t.Errorf("%s: Content-Length = %q, want %d", name, got, len(compressed)) + } + } + + // .bz2 stays identity. + wbz := get("/conda-forge/linux-64/repodata.json.bz2") + if wbz.Code != http.StatusOK { + t.Fatalf("bz2: status = %d, want 200", wbz.Code) + } + if got := lastAE("/conda-forge/linux-64/repodata.json.bz2"); got != "identity" { + t.Errorf("bz2: upstream Accept-Encoding = %q, want %q", got, "identity") + } + if got := wbz.Header().Get(headerContentEncoding); got != "" { + t.Errorf("bz2: Content-Encoding = %q, want empty", got) + } + + // Cached replay with the upstream down: same compressed bytes + header, no new .json fetch. + reqsBefore := jsonUpstreamReqs.Load() + available.Store(false) + wc := get("/conda-forge/linux-64/repodata.json") + if wc.Code != http.StatusOK { + t.Fatalf("cached repodata.json: status = %d, want 200: %s", wc.Code, wc.Body.String()) + } + if !bytes.Equal(wc.Body.Bytes(), compressed) { + t.Errorf("cached repodata.json: body not the compressed bytes") + } + if got := wc.Header().Get(headerContentEncoding); got != "gzip" { + t.Errorf("cached repodata.json: Content-Encoding = %q, want %q", got, "gzip") + } + if jsonUpstreamReqs.Load() != reqsBefore { + t.Errorf("cached repodata.json hit upstream: reqs %d -> %d", reqsBefore, jsonUpstreamReqs.Load()) + } +} + +// TestCondaRepodataStreamPathRequestsGzip covers the default cache_metadata=off +// branch: the streaming path must also request gzip for repodata.json and +// forward the Content-Encoding header. +func TestCondaRepodataStreamPathRequestsGzip(t *testing.T) { + plain := []byte(`{"packages":{}}`) + compressed := gzipPayload(t, plain) + var sawAcceptEncoding string + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + sawAcceptEncoding = r.Header.Get(headerAcceptEncoding) + if strings.Contains(sawAcceptEncoding, "gzip") { + w.Header().Set(headerContentType, contentTypeJSON) + w.Header().Set(headerContentEncoding, "gzip") + _, _ = w.Write(compressed) + return + } + w.Header().Set(headerContentType, contentTypeJSON) + _, _ = w.Write(plain) + })) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + proxy.CacheMetadata = false + proxy.HTTPClient = upstream.Client() + routes := NewCondaHandlerWithUpstream(proxy, "http://proxy.local", upstream.URL).Routes() + + w := httptest.NewRecorder() + routes.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/conda-forge/linux-64/repodata.json", nil)) + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200: %s", w.Code, w.Body.String()) + } + if sawAcceptEncoding != "gzip" { + t.Errorf("stream path upstream Accept-Encoding = %q, want %q", sawAcceptEncoding, "gzip") + } + if !bytes.Equal(w.Body.Bytes(), compressed) { + t.Errorf("stream path body not the compressed bytes") + } + if got := w.Header().Get(headerContentEncoding); got != "gzip" { + t.Errorf("stream path Content-Encoding = %q, want %q", got, "gzip") + } +} diff --git a/internal/handler/handler.go b/internal/handler/handler.go index 7c022896..d6161635 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -667,15 +667,17 @@ func metadataStoragePath(ecosystem, cacheKey string) string { // cacheKey is typically the package name but can include subpath components. // Optional acceptHeaders specify the Accept header(s) to send; defaults to application/json. func (p *Proxy) FetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, upstreamURL string, acceptHeaders ...string) ([]byte, string, error) { - return p.fetchOrCacheMetadata(ctx, ecosystem, cacheKey, upstreamURL, false, acceptHeaders...) + return p.fetchOrCacheMetadata(ctx, ecosystem, cacheKey, upstreamURL, "", acceptHeaders...) } -// fetchOrCacheMetadata implements FetchOrCacheMetadata. When verbatim is true -// (the ProxyCached path, which serves upstream bytes through unchanged) the -// upstream is fetched with Accept-Encoding: identity so signed and hash-pinned -// index files are cached exactly as sent. Direct callers that parse or rewrite -// the body pass verbatim=false and keep transparent transfer compression. -func (p *Proxy) fetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, upstreamURL string, verbatim bool, acceptHeaders ...string) ([]byte, string, error) { +// fetchOrCacheMetadata implements FetchOrCacheMetadata. acceptEncoding controls +// the upstream Accept-Encoding: an empty string leaves it unset so Go +// transparently decompresses (for direct callers that parse or rewrite the +// body); any non-empty value is sent verbatim, which disables Go's +// decompression so the wire bytes and their Content-Encoding are stored and +// replayed as sent. The ProxyCached path uses "identity" for signed indexes and +// "gzip" where both hops should stay compressed. +func (p *Proxy) fetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, upstreamURL, acceptEncoding string, acceptHeaders ...string) ([]byte, string, error) { if containsPathTraversal(cacheKey) { return nil, "", fmt.Errorf("invalid cache key: %q", cacheKey) } @@ -715,10 +717,10 @@ func (p *Proxy) fetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, u } // Try upstream - meta, err := p.fetchUpstreamMetadata(ctx, upstreamURL, entry, accept, verbatim) + meta, err := p.fetchUpstreamMetadata(ctx, upstreamURL, entry, accept, acceptEncoding) if errors.Is(err, errStale304) { // 304 but cached file is gone; retry without ETag - meta, err = p.fetchUpstreamMetadata(ctx, upstreamURL, nil, accept, verbatim) + meta, err = p.fetchUpstreamMetadata(ctx, upstreamURL, nil, accept, acceptEncoding) } if err == nil { if p.CacheMetadata { @@ -771,20 +773,19 @@ type upstreamMetadata struct { } // fetchUpstreamMetadata fetches metadata from upstream, using ETag for conditional revalidation. -// It requests the identity encoding and never transparently decompresses, so the returned -// bytes are exactly what the upstream sent; any Content-Encoding the upstream applied -// anyway is reported alongside so callers can store and replay it. -func (p *Proxy) fetchUpstreamMetadata(ctx context.Context, upstreamURL string, entry *database.MetadataCacheEntry, accept string, verbatim bool) (*upstreamMetadata, error) { +// When acceptEncoding is non-empty it is sent as the Accept-Encoding header, which disables Go's +// transparent decompression (it only applies when the transport adds the header itself), so the +// returned bytes are exactly what the upstream sent and any Content-Encoding it applied is reported +// alongside for the caller to store and replay. An empty acceptEncoding leaves Go to negotiate and +// decompress transparently. +func (p *Proxy) fetchUpstreamMetadata(ctx context.Context, upstreamURL string, entry *database.MetadataCacheEntry, accept, acceptEncoding string) (*upstreamMetadata, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, upstreamURL, nil) if err != nil { return nil, fmt.Errorf("creating request: %w", err) } req.Header.Set("Accept", accept) - if verbatim { - // Setting Accept-Encoding explicitly disables Go's transparent gzip - // decompression (it only applies when the transport adds the header - // itself), so signed index files are cached byte-for-byte as sent. - req.Header.Set(headerAcceptEncoding, "identity") + if acceptEncoding != "" { + req.Header.Set(headerAcceptEncoding, acceptEncoding) } p.applyUpstreamAuth(req) @@ -914,13 +915,22 @@ func (p *Proxy) lookupCachedMeta(ecosystem, cacheKey string) cachedMeta { // When metadata caching is disabled, the response is streamed directly to avoid buffering // large metadata responses (e.g. npm packages with many versions) in memory. func (p *Proxy) ProxyCached(w http.ResponseWriter, r *http.Request, upstreamURL, ecosystem, cacheKey string, acceptHeaders ...string) { + p.proxyCachedWithEncoding(w, r, upstreamURL, ecosystem, cacheKey, "identity", acceptHeaders...) +} + +// proxyCachedWithEncoding is ProxyCached with an explicit upstream Accept-Encoding. +// "identity" preserves signed index bytes (the default); "gzip" keeps both hops +// compressed for large, non-hash-pinned metadata whose clients decode gzip +// (conda repodata). The stored bytes and Content-Encoding are replayed verbatim +// either way. +func (p *Proxy) proxyCachedWithEncoding(w http.ResponseWriter, r *http.Request, upstreamURL, ecosystem, cacheKey, acceptEncoding string, acceptHeaders ...string) { if !p.CacheMetadata { // Stream directly without buffering when caching is off. - p.proxyMetadataStream(w, r, upstreamURL, acceptHeaders...) + p.proxyMetadataStream(w, r, upstreamURL, acceptEncoding, acceptHeaders...) return } - body, contentType, err := p.fetchOrCacheMetadata(r.Context(), ecosystem, cacheKey, upstreamURL, true, acceptHeaders...) + body, contentType, err := p.fetchOrCacheMetadata(r.Context(), ecosystem, cacheKey, upstreamURL, acceptEncoding, acceptHeaders...) if err != nil { if errors.Is(err, ErrUpstreamNotFound) { http.Error(w, "not found", http.StatusNotFound) @@ -974,7 +984,7 @@ func (p *Proxy) writeMetadataCachedResponse(w http.ResponseWriter, r *http.Reque // proxyMetadataStream forwards an upstream metadata response by streaming it to the client // without buffering the full body in memory. -func (p *Proxy) proxyMetadataStream(w http.ResponseWriter, r *http.Request, upstreamURL string, acceptHeaders ...string) { +func (p *Proxy) proxyMetadataStream(w http.ResponseWriter, r *http.Request, upstreamURL, acceptEncoding string, acceptHeaders ...string) { req, err := http.NewRequestWithContext(r.Context(), http.MethodGet, upstreamURL, nil) if err != nil { http.Error(w, "failed to create request", http.StatusInternalServerError) @@ -986,10 +996,13 @@ func (p *Proxy) proxyMetadataStream(w http.ResponseWriter, r *http.Request, upst accept = acceptHeaders[0] } req.Header.Set("Accept", accept) - // ProxyCached serves bytes through verbatim, so request identity to keep - // Go from transparently decompressing (and stripping the Content-Encoding - // of) signed index files, regardless of what the client negotiated. - req.Header.Set(headerAcceptEncoding, "identity") + // Set Accept-Encoding explicitly (identity, or gzip for compressible + // verbatim metadata) so Go does not transparently decompress and strip the + // Content-Encoding of the bytes we forward, regardless of what the client + // negotiated. + if acceptEncoding != "" { + req.Header.Set(headerAcceptEncoding, acceptEncoding) + } p.applyUpstreamAuth(req) for _, header := range []string{"If-Modified-Since", "If-None-Match"} { From 14175bf7fa2f8b81191104df151b5b49d5f4478b Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Thu, 3 Sep 2026 17:22:59 +0200 Subject: [PATCH 02/10] fix(handler): pass metadata content-encoding with the body it describes The adversarial review of the conda gzip route found a reachable regression: writeMetadataCachedResponse took Content-Encoding from a fresh cache-row read while cacheMetadataBlob skips the row write when Storage.Store fails. Under identity that was benign (the body was plain anyway), but on the new gzip route a disk-full or object-store outage served raw gzip bytes as Content-Type: application/json with no Content-Encoding and HTTP 200 -- conda, mamba and pixi fail to parse them, with no HTTP signal and only a Warn log, on every request until a cache write succeeds. fetchOrCacheMetadata now returns the encoding of the body it hands back (the upstream value on a fetch, the stored row's value on a TTL hit or stale fallback) and proxyCachedWithEncoding passes it to writeMetadataCachedResponse, so the header always describes the bytes actually written. cachedMeta drops its now-unused content_encoding field. helm and maven pass "" -- both fetch transparently, so their stored encoding was always empty and behaviour is unchanged. Also fixes a vacuous assertion in the new conda test: the upstream request counter incremented behind the availability gate, so the cached-replay block could never observe a refetch. --- internal/handler/conda_test.go | 52 +++++++++++++++++++++++++++++++++- internal/handler/handler.go | 44 ++++++++++++++-------------- internal/handler/helm.go | 2 +- internal/handler/maven.go | 2 +- 4 files changed, 75 insertions(+), 25 deletions(-) diff --git a/internal/handler/conda_test.go b/internal/handler/conda_test.go index 1ba01ebd..12e346ae 100644 --- a/internal/handler/conda_test.go +++ b/internal/handler/conda_test.go @@ -3,6 +3,7 @@ package handler import ( "bytes" "encoding/json" + "errors" "log/slog" "net/http" "net/http/httptest" @@ -324,12 +325,14 @@ func TestCondaRepodataRequestsGzip(t *testing.T) { upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { sawAcceptEncoding.Store(r.URL.Path, r.Header.Get(headerAcceptEncoding)) + if strings.HasSuffix(r.URL.Path, ".json") { + jsonUpstreamReqs.Add(1) + } if !available.Load() { http.Error(w, "unavailable", http.StatusServiceUnavailable) return } if strings.HasSuffix(r.URL.Path, ".json") { - jsonUpstreamReqs.Add(1) if strings.Contains(r.Header.Get(headerAcceptEncoding), "gzip") { w.Header().Set(headerContentType, contentTypeJSON) w.Header().Set(headerContentEncoding, "gzip") @@ -453,3 +456,50 @@ func TestCondaRepodataStreamPathRequestsGzip(t *testing.T) { t.Errorf("stream path Content-Encoding = %q, want %q", got, "gzip") } } + +// TestCondaRepodataGzipSurvivesCacheWriteFailure covers the failure the gzip +// route makes reachable: when the metadata cache write fails the freshly +// fetched body is still served, so its Content-Encoding must come from the +// fetch and not from the (unwritten) cache row -- otherwise gzip bytes go out +// labelled application/json with no Content-Encoding and every conda client +// fails to parse them. +func TestCondaRepodataGzipSurvivesCacheWriteFailure(t *testing.T) { + plain := []byte(`{"packages":{},"repodata_version":1}`) + compressed := gzipPayload(t, plain) + + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.Header.Get(headerAcceptEncoding), "gzip") { + w.Header().Set(headerContentType, contentTypeJSON) + w.Header().Set(headerContentEncoding, "gzip") + _, _ = w.Write(compressed) + return + } + w.Header().Set(headerContentType, contentTypeJSON) + _, _ = w.Write(plain) + })) + defer upstream.Close() + + proxy, _, store, _ := setupTestProxy(t) + proxy.CacheMetadata = true + proxy.MetadataTTL = time.Hour + proxy.HTTPClient = upstream.Client() + store.storeErr = errors.New("disk full") + + routes := NewCondaHandlerWithUpstream(proxy, "http://proxy.local", upstream.URL).Routes() + w := httptest.NewRecorder() + routes.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/conda-forge/linux-64/repodata.json", nil)) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200: %s", w.Code, w.Body.String()) + } + if !bytes.Equal(w.Body.Bytes(), compressed) { + t.Fatalf("body is not the fetched compressed bytes (got %d, want %d)", w.Body.Len(), len(compressed)) + } + // The cache row was never written, so the header must come from the fetch. + if got := w.Header().Get(headerContentEncoding); got != "gzip" { + t.Errorf("Content-Encoding = %q, want %q (gzip body would be unparseable without it)", got, "gzip") + } + if got := w.Header().Get(headerContentLength); got != strconv.Itoa(len(compressed)) { + t.Errorf("Content-Length = %q, want %d", got, len(compressed)) + } +} diff --git a/internal/handler/handler.go b/internal/handler/handler.go index d6161635..f4948d38 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -667,7 +667,8 @@ func metadataStoragePath(ecosystem, cacheKey string) string { // cacheKey is typically the package name but can include subpath components. // Optional acceptHeaders specify the Accept header(s) to send; defaults to application/json. func (p *Proxy) FetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, upstreamURL string, acceptHeaders ...string) ([]byte, string, error) { - return p.fetchOrCacheMetadata(ctx, ecosystem, cacheKey, upstreamURL, "", acceptHeaders...) + body, contentType, _, err := p.fetchOrCacheMetadata(ctx, ecosystem, cacheKey, upstreamURL, "", acceptHeaders...) + return body, contentType, err } // fetchOrCacheMetadata implements FetchOrCacheMetadata. acceptEncoding controls @@ -677,9 +678,9 @@ func (p *Proxy) FetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, u // decompression so the wire bytes and their Content-Encoding are stored and // replayed as sent. The ProxyCached path uses "identity" for signed indexes and // "gzip" where both hops should stay compressed. -func (p *Proxy) fetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, upstreamURL, acceptEncoding string, acceptHeaders ...string) ([]byte, string, error) { +func (p *Proxy) fetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, upstreamURL, acceptEncoding string, acceptHeaders ...string) ([]byte, string, string, error) { if containsPathTraversal(cacheKey) { - return nil, "", fmt.Errorf("invalid cache key: %q", cacheKey) + return nil, "", "", fmt.Errorf("invalid cache key: %q", cacheKey) } storagePath := metadataStoragePath(ecosystem, cacheKey) @@ -703,7 +704,7 @@ func (p *Proxy) fetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, u ct = entry.ContentType.String } metrics.RecordCacheHit(ecosystem) - return data, ct, nil + return data, ct, entry.ContentEncoding.String, nil } } // Cache file missing/unreadable, fall through to upstream @@ -726,12 +727,12 @@ func (p *Proxy) fetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, u if p.CacheMetadata { p.cacheMetadataBlob(ctx, ecosystem, cacheKey, storagePath, meta) } - return meta.body, meta.contentType, nil + return meta.body, meta.contentType, meta.contentEncoding, nil } // Upstream failed -- fall back to cache if available if !p.CacheMetadata || entry == nil { - return nil, "", fmt.Errorf("upstream failed and no cached metadata: %w", err) + return nil, "", "", fmt.Errorf("upstream failed and no cached metadata: %w", err) } p.Logger.Warn("upstream metadata fetch failed, checking cache", @@ -739,13 +740,13 @@ func (p *Proxy) fetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, u cached, readErr := p.Storage.Open(ctx, entry.StoragePath) if readErr != nil { - return nil, "", fmt.Errorf("upstream failed and cached file missing: %w", err) + return nil, "", "", fmt.Errorf("upstream failed and cached file missing: %w", err) } defer func() { _ = cached.Close() }() data, readErr := p.ReadMetadata(cached) if readErr != nil { - return nil, "", fmt.Errorf("upstream failed and cached read error: %w", err) + return nil, "", "", fmt.Errorf("upstream failed and cached read error: %w", err) } ct := contentTypeJSON @@ -754,7 +755,7 @@ func (p *Proxy) fetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, u } p.Logger.Info("serving metadata from cache", "ecosystem", ecosystem, "key", cacheKey) - return data, ct, nil + return data, ct, entry.ContentEncoding.String, nil } func (p *Proxy) recordMetadataCacheMiss(ecosystem string) { @@ -877,10 +878,9 @@ func (p *Proxy) cacheMetadataBlob(ctx context.Context, ecosystem, cacheKey, stor // cachedMeta holds cache validators and freshness state from a metadata cache entry. type cachedMeta struct { - etag string - lastModified time.Time - contentEncoding string - stale bool + etag string + lastModified time.Time + stale bool } // lookupCachedMeta retrieves cache validators for a metadata entry. @@ -899,9 +899,6 @@ func (p *Proxy) lookupCachedMeta(ecosystem, cacheKey string) cachedMeta { if entry.LastModified.Valid { cm.lastModified = entry.LastModified.Time } - if entry.ContentEncoding.Valid { - cm.contentEncoding = entry.ContentEncoding.String - } // If FetchedAt is older than TTL, upstream must have failed and // we served from stale cache (successful fetches update FetchedAt). if p.MetadataTTL> 0 && entry.FetchedAt.Valid && time.Since(entry.FetchedAt.Time)> p.MetadataTTL { @@ -930,7 +927,7 @@ func (p *Proxy) proxyCachedWithEncoding(w http.ResponseWriter, r *http.Request, return } - body, contentType, err := p.fetchOrCacheMetadata(r.Context(), ecosystem, cacheKey, upstreamURL, acceptEncoding, acceptHeaders...) + body, contentType, contentEncoding, err := p.fetchOrCacheMetadata(r.Context(), ecosystem, cacheKey, upstreamURL, acceptEncoding, acceptHeaders...) if err != nil { if errors.Is(err, ErrUpstreamNotFound) { http.Error(w, "not found", http.StatusNotFound) @@ -941,12 +938,15 @@ func (p *Proxy) proxyCachedWithEncoding(w http.ResponseWriter, r *http.Request, return } - p.writeMetadataCachedResponse(w, r, ecosystem, cacheKey, body, contentType) + p.writeMetadataCachedResponse(w, r, ecosystem, cacheKey, body, contentType, contentEncoding) } // writeMetadataCachedResponse writes a cached metadata response and handles -// conditional request headers using metadata cache validators. -func (p *Proxy) writeMetadataCachedResponse(w http.ResponseWriter, r *http.Request, ecosystem, cacheKey string, body []byte, contentType string) { +// conditional request headers using metadata cache validators. contentEncoding +// must describe the body being written; it is passed in rather than re-read +// from the cache row, which is missing or stale when the metadata cache write +// failed and would otherwise mislabel the bytes. +func (p *Proxy) writeMetadataCachedResponse(w http.ResponseWriter, r *http.Request, ecosystem, cacheKey string, body []byte, contentType, contentEncoding string) { cm := p.lookupCachedMeta(ecosystem, cacheKey) if cm.etag != "" { @@ -966,8 +966,8 @@ func (p *Proxy) writeMetadataCachedResponse(w http.ResponseWriter, r *http.Reque w.Header().Set(headerContentType, contentType) w.Header().Set(headerContentLength, strconv.Itoa(len(body))) - if cm.contentEncoding != "" { - w.Header().Set(headerContentEncoding, cm.contentEncoding) + if contentEncoding != "" { + w.Header().Set(headerContentEncoding, contentEncoding) } if cm.etag != "" { w.Header().Set("ETag", cm.etag) diff --git a/internal/handler/helm.go b/internal/handler/helm.go index 994c2594..ac4728bd 100644 --- a/internal/handler/helm.go +++ b/internal/handler/helm.go @@ -69,7 +69,7 @@ func (h *HelmHandler) handleIndex(w http.ResponseWriter, r *http.Request) { return } - h.proxy.writeMetadataCachedResponse(w, r, helmMetadataEcosystem, h.indexCacheKey(repository, upstreamURL), rewritten, contentType) + h.proxy.writeMetadataCachedResponse(w, r, helmMetadataEcosystem, h.indexCacheKey(repository, upstreamURL), rewritten, contentType, "") } func (h *HelmHandler) handleChart(w http.ResponseWriter, r *http.Request) { diff --git a/internal/handler/maven.go b/internal/handler/maven.go index 10e551e0..fdf571e3 100644 --- a/internal/handler/maven.go +++ b/internal/handler/maven.go @@ -99,7 +99,7 @@ func (h *MavenHandler) handleMetadata(w http.ResponseWriter, r *http.Request, ur return } - h.proxy.writeMetadataCachedResponse(w, r, "maven", cacheKey, body, contentType) + h.proxy.writeMetadataCachedResponse(w, r, "maven", cacheKey, body, contentType, "") } // handleDownload serves an artifact file, fetching and caching from upstream if needed. From c999ce9eff0beeda392b0be76a74815f074036be Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Thu, 3 Sep 2026 21:26:37 +0200 Subject: [PATCH 03/10] fix(handler): pin the stale-fallback content-encoding and drop a dead guard Follow-ups from the adversarial review of the #305 branch, limited to code this branch introduced: - proxyMetadataStream is only ever reached with an explicit Accept-Encoding (ProxyCached passes identity, conda passes gzip or identity), so the guard around the header set was unreachable; replace it with the plain one-token substitution of the former literal, which is the smallest change from main. - The stale-fallback return of fetchOrCacheMetadata (encoding taken from the cache row) was the one #305 return site no test pinned: replacing it with an empty encoding survived the whole suite. Add a conda test that expires the entry, fails the upstream, and asserts the stored gzip blob is served with Content-Encoding: gzip. Not changed, by scope: cacheMetadataBlob still discards the UpsertMetadataCache error (pre-existing on main). If Storage.Store succeeds and the row write fails, a later stale fallback or TTL hit can serve the gzip blob with the row's stale encoding; that needs a DB write failure plus a second event and is tracked separately. --- internal/handler/conda_test.go | 54 ++++++++++++++++++++++++++++++++++ internal/handler/handler.go | 4 +-- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/internal/handler/conda_test.go b/internal/handler/conda_test.go index 12e346ae..8685dca8 100644 --- a/internal/handler/conda_test.go +++ b/internal/handler/conda_test.go @@ -503,3 +503,57 @@ func TestCondaRepodataGzipSurvivesCacheWriteFailure(t *testing.T) { t.Errorf("Content-Length = %q, want %d", got, len(compressed)) } } + +// TestCondaRepodataGzipStaleFallbackKeepsEncoding pins the stale-fallback +// return of fetchOrCacheMetadata: when the upstream fails after the entry has +// expired, the stored gzip blob must be served with its Content-Encoding taken +// from the cache row, not dropped. +func TestCondaRepodataGzipStaleFallbackKeepsEncoding(t *testing.T) { + plain := []byte(`{"packages":{},"repodata_version":1}`) + compressed := gzipPayload(t, plain) + + var available atomic.Bool + available.Store(true) + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !available.Load() { + http.Error(w, "unavailable", http.StatusServiceUnavailable) + return + } + if strings.Contains(r.Header.Get(headerAcceptEncoding), "gzip") { + w.Header().Set(headerContentType, contentTypeJSON) + w.Header().Set(headerContentEncoding, "gzip") + _, _ = w.Write(compressed) + return + } + w.Header().Set(headerContentType, contentTypeJSON) + _, _ = w.Write(plain) + })) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + proxy.CacheMetadata = true + proxy.MetadataTTL = 0 // every request revalidates; an upstream failure falls back to the stale row + proxy.HTTPClient = upstream.Client() + routes := NewCondaHandlerWithUpstream(proxy, "http://proxy.local", upstream.URL).Routes() + + get := func() *httptest.ResponseRecorder { + w := httptest.NewRecorder() + routes.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/conda-forge/linux-64/repodata.json", nil)) + return w + } + + if first := get(); first.Code != http.StatusOK { + t.Fatalf("first status = %d, want 200: %s", first.Code, first.Body.String()) + } + available.Store(false) + stale := get() + if stale.Code != http.StatusOK { + t.Fatalf("stale status = %d, want 200: %s", stale.Code, stale.Body.String()) + } + if !bytes.Equal(stale.Body.Bytes(), compressed) { + t.Errorf("stale body is not the stored compressed bytes") + } + if got := stale.Header().Get(headerContentEncoding); got != "gzip" { + t.Errorf("stale Content-Encoding = %q, want %q", got, "gzip") + } +} diff --git a/internal/handler/handler.go b/internal/handler/handler.go index f4948d38..0dec5aad 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -1000,9 +1000,7 @@ func (p *Proxy) proxyMetadataStream(w http.ResponseWriter, r *http.Request, upst // verbatim metadata) so Go does not transparently decompress and strip the // Content-Encoding of the bytes we forward, regardless of what the client // negotiated. - if acceptEncoding != "" { - req.Header.Set(headerAcceptEncoding, acceptEncoding) - } + req.Header.Set(headerAcceptEncoding, acceptEncoding) p.applyUpstreamAuth(req) for _, header := range []string{"If-Modified-Since", "If-None-Match"} { From 935423b15fd5d34efb93438dc2934e5628feac1c Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Thu, 3 Sep 2026 21:57:13 +0200 Subject: [PATCH 04/10] fix(handler): restore the pre-existing cachedMeta content-encoding field The third adversarial review classified deleting cachedMeta.contentEncoding and its lookupCachedMeta populate as elective: neither line was created by this branch nor forced by the fix (writeMetadataCachedResponse now reads the encoding from its parameter and ignores the row value). Under the rule that pre-existing code this branch did not have to touch stays untouched, restore both as they are on main. No behaviour change. Residuals the review documented, unchanged by scope (both share one root cause: the encoding lives in the cache row and the bytes in the blob, and neither is written or read atomically): - cacheMetadataBlob discards the UpsertMetadataCache error, so after a successful gzip Store and a failed row write a later stale fallback or TTL hit can serve the gzip blob with the row's stale encoding. - During the one-time identity->gzip rollout, a request that read a pre-branch identity row, lost the upstream race to a request that stored the gzip blob, and then failed upstream serves the gzip bytes with no Content-Encoding for that one response; later requests self-heal. - helm and maven now pass an empty encoding; on main a spec-violating upstream that answered a transparent gzip request with an encoding Go does not decode (e.g. br) would have had that header replayed from the row. Degenerate; documented rather than changed. --- internal/handler/handler.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/internal/handler/handler.go b/internal/handler/handler.go index 0dec5aad..4f00715c 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -878,9 +878,10 @@ func (p *Proxy) cacheMetadataBlob(ctx context.Context, ecosystem, cacheKey, stor // cachedMeta holds cache validators and freshness state from a metadata cache entry. type cachedMeta struct { - etag string - lastModified time.Time - stale bool + etag string + lastModified time.Time + contentEncoding string + stale bool } // lookupCachedMeta retrieves cache validators for a metadata entry. @@ -899,6 +900,9 @@ func (p *Proxy) lookupCachedMeta(ecosystem, cacheKey string) cachedMeta { if entry.LastModified.Valid { cm.lastModified = entry.LastModified.Time } + if entry.ContentEncoding.Valid { + cm.contentEncoding = entry.ContentEncoding.String + } // If FetchedAt is older than TTL, upstream must have failed and // we served from stale cache (successful fetches update FetchedAt). if p.MetadataTTL> 0 && entry.FetchedAt.Valid && time.Since(entry.FetchedAt.Time)> p.MetadataTTL { From 9797a4c46e2ad8b91c6c039c4f667dfb7b4dd713 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Fri, 4 Sep 2026 11:33:16 +0200 Subject: [PATCH 05/10] fix(handler): keep conda's proxyCached and .bz2 route as on main Threading acceptEncoding through CondaHandler.proxyCached changed the form of two pieces of original code the fix did not need to touch: the repodata.json.bz2 route (method value rewritten as a closure) and proxyCached itself (new parameter, new call). Restore both exactly as on main; ProxyCached still sends identity, so the .bz2 route is unchanged in behaviour. handleRepodata's non-cooldown branch now derives the cache key inline and calls proxyCachedWithEncoding with gzip directly, so the only original conda.go line that changes is that one call. --- internal/handler/conda.go | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/internal/handler/conda.go b/internal/handler/conda.go index bc7cf5dd..1cc6b89c 100644 --- a/internal/handler/conda.go +++ b/internal/handler/conda.go @@ -42,11 +42,7 @@ func (h *CondaHandler) Routes() http.Handler { // Channel index (repodata) mux.HandleFunc("GET /{channel}/{arch}/repodata.json", h.handleRepodata) - // .bz2 is already compressed and libmamba only decodes Content-Encoding on - // .json URLs, so keep it identity. - mux.HandleFunc("GET /{channel}/{arch}/repodata.json.bz2", func(w http.ResponseWriter, r *http.Request) { - h.proxyCached(w, r, "identity") - }) + mux.HandleFunc("GET /{channel}/{arch}/repodata.json.bz2", h.proxyCached) mux.HandleFunc("GET /{channel}/{arch}/current_repodata.json", h.handleRepodata) // Package downloads (cache these) @@ -139,8 +135,11 @@ func (h *CondaHandler) handleRepodata(w http.ResponseWriter, r *http.Request) { // repodata.json is ~441 MB uncompressed, over the metadata_max_size cap, // vs ~34 MB gzip). Request gzip so both hops stay compressed and the // cache stores the small blob; conda/mamba/pixi decode Content-Encoding - // on .json URLs. See issue #305. - h.proxyCached(w, r, "gzip") + // on .json URLs. repodata.json.bz2 keeps going through proxyCached + // (identity): it is already compressed and libmamba only decodes + // Content-Encoding on .json URLs. See issue #305. + cacheKey := strings.ReplaceAll(strings.TrimPrefix(r.URL.Path, "/"), "/", "_") + h.proxy.proxyCachedWithEncoding(w, r, h.upstreamURL+r.URL.Path, "conda", cacheKey, "gzip", "*/*") return } @@ -245,13 +244,11 @@ func (h *CondaHandler) applyCooldownFiltering(body []byte) ([]byte, error) { return json.Marshal(repodata) } -// proxyCached forwards a metadata request with caching, sending the given -// upstream Accept-Encoding ("identity" for verbatim bytes, "gzip" to keep both -// hops compressed for the large .json repodata). -func (h *CondaHandler) proxyCached(w http.ResponseWriter, r *http.Request, acceptEncoding string) { +// proxyCached forwards a metadata request with caching. +func (h *CondaHandler) proxyCached(w http.ResponseWriter, r *http.Request) { cacheKey := strings.TrimPrefix(r.URL.Path, "/") cacheKey = strings.ReplaceAll(cacheKey, "/", "_") - h.proxy.proxyCachedWithEncoding(w, r, h.upstreamURL+r.URL.Path, "conda", cacheKey, acceptEncoding, "*/*") + h.proxy.ProxyCached(w, r, h.upstreamURL+r.URL.Path, "conda", cacheKey, "*/*") } // proxyUpstream forwards a request to Anaconda without caching. From 5cf1cb1e8541446ff50bd1aaab7fdde633b4f4a1 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sat, 5 Sep 2026 13:41:44 +0200 Subject: [PATCH 06/10] fix(handler): keep writeMetadataCachedResponse and its callers as on main Adding a contentEncoding parameter to writeMetadataCachedResponse changed a signature that predates #304 and dragged its two pre-#304 callers (helm.go, maven.go) into the diff, even though #304 only ever added the cm.contentEncoding block inside the function body. Restore writeMetadataCachedResponse's doc and signature exactly as on main and make it a delegate that passes an empty encoding to a new unexported writeMetadataCachedResponseWithEncoding, which carries the original body with #304's block reading the parameter instead of the cache row. proxyCachedWithEncoding calls the sibling with the encoding returned alongside the body. helm.go and maven.go drop out of the diff; their behaviour is unchanged (both fetch transparently, so their stored encoding was always empty). Same split pattern as ProxyCached -> proxyCachedWithEncoding. --- internal/handler/handler.go | 18 ++++++++++++------ internal/handler/helm.go | 2 +- internal/handler/maven.go | 2 +- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/internal/handler/handler.go b/internal/handler/handler.go index 4f00715c..86fd3d04 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -942,15 +942,21 @@ func (p *Proxy) proxyCachedWithEncoding(w http.ResponseWriter, r *http.Request, return } - p.writeMetadataCachedResponse(w, r, ecosystem, cacheKey, body, contentType, contentEncoding) + p.writeMetadataCachedResponseWithEncoding(w, r, ecosystem, cacheKey, body, contentType, contentEncoding) } // writeMetadataCachedResponse writes a cached metadata response and handles -// conditional request headers using metadata cache validators. contentEncoding -// must describe the body being written; it is passed in rather than re-read -// from the cache row, which is missing or stale when the metadata cache write -// failed and would otherwise mislabel the bytes. -func (p *Proxy) writeMetadataCachedResponse(w http.ResponseWriter, r *http.Request, ecosystem, cacheKey string, body []byte, contentType, contentEncoding string) { +// conditional request headers using metadata cache validators. +func (p *Proxy) writeMetadataCachedResponse(w http.ResponseWriter, r *http.Request, ecosystem, cacheKey string, body []byte, contentType string) { + p.writeMetadataCachedResponseWithEncoding(w, r, ecosystem, cacheKey, body, contentType, "") +} + +// writeMetadataCachedResponseWithEncoding is writeMetadataCachedResponse with +// an explicit Content-Encoding. contentEncoding must describe the body being +// written; it is passed in rather than re-read from the cache row, which is +// missing or stale when the metadata cache write failed and would otherwise +// mislabel the bytes. +func (p *Proxy) writeMetadataCachedResponseWithEncoding(w http.ResponseWriter, r *http.Request, ecosystem, cacheKey string, body []byte, contentType, contentEncoding string) { cm := p.lookupCachedMeta(ecosystem, cacheKey) if cm.etag != "" { diff --git a/internal/handler/helm.go b/internal/handler/helm.go index ac4728bd..994c2594 100644 --- a/internal/handler/helm.go +++ b/internal/handler/helm.go @@ -69,7 +69,7 @@ func (h *HelmHandler) handleIndex(w http.ResponseWriter, r *http.Request) { return } - h.proxy.writeMetadataCachedResponse(w, r, helmMetadataEcosystem, h.indexCacheKey(repository, upstreamURL), rewritten, contentType, "") + h.proxy.writeMetadataCachedResponse(w, r, helmMetadataEcosystem, h.indexCacheKey(repository, upstreamURL), rewritten, contentType) } func (h *HelmHandler) handleChart(w http.ResponseWriter, r *http.Request) { diff --git a/internal/handler/maven.go b/internal/handler/maven.go index fdf571e3..10e551e0 100644 --- a/internal/handler/maven.go +++ b/internal/handler/maven.go @@ -99,7 +99,7 @@ func (h *MavenHandler) handleMetadata(w http.ResponseWriter, r *http.Request, ur return } - h.proxy.writeMetadataCachedResponse(w, r, "maven", cacheKey, body, contentType, "") + h.proxy.writeMetadataCachedResponse(w, r, "maven", cacheKey, body, contentType) } // handleDownload serves an artifact file, fetching and caching from upstream if needed. From 5991d958597a60934a647f851bf3356291eaf36e Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sat, 5 Sep 2026 13:44:03 +0200 Subject: [PATCH 07/10] fix(handler): move the conda gzip change to its own branch The conda call site in handleRepodata predates #304 and #304 never touched it, so under the rule that this PR only corrects code and behaviour #304 introduced it does not belong here. Restore conda.go and conda_test.go as on main; the conda change continues on a stacked branch against its own issue. Replace the conda-route tests with tests that exercise proxyCachedWithEncoding directly, so this PR still pins its own plumbing: gzip is requested and the compressed bytes plus Content-Encoding are cached and replayed (cached and streaming paths), the header survives a metadata cache write failure, and the stale fallback keeps the stored encoding. --- internal/handler/conda.go | 10 +- internal/handler/conda_test.go | 254 ------------------ .../handler/proxy_cached_encoding_test.go | 168 ++++++++++++ 3 files changed, 169 insertions(+), 263 deletions(-) create mode 100644 internal/handler/proxy_cached_encoding_test.go diff --git a/internal/handler/conda.go b/internal/handler/conda.go index 1cc6b89c..cef814b5 100644 --- a/internal/handler/conda.go +++ b/internal/handler/conda.go @@ -131,15 +131,7 @@ func (h *CondaHandler) parseFilename(filename string) (name, version string) { // handleRepodata proxies repodata.json, applying cooldown filtering when enabled. func (h *CondaHandler) handleRepodata(w http.ResponseWriter, r *http.Request) { if h.proxy.Cooldown == nil || !h.proxy.Cooldown.Enabled() { - // repodata.json / current_repodata.json are large plain JSON (linux-64 - // repodata.json is ~441 MB uncompressed, over the metadata_max_size cap, - // vs ~34 MB gzip). Request gzip so both hops stay compressed and the - // cache stores the small blob; conda/mamba/pixi decode Content-Encoding - // on .json URLs. repodata.json.bz2 keeps going through proxyCached - // (identity): it is already compressed and libmamba only decodes - // Content-Encoding on .json URLs. See issue #305. - cacheKey := strings.ReplaceAll(strings.TrimPrefix(r.URL.Path, "/"), "/", "_") - h.proxy.proxyCachedWithEncoding(w, r, h.upstreamURL+r.URL.Path, "conda", cacheKey, "gzip", "*/*") + h.proxyCached(w, r) return } diff --git a/internal/handler/conda_test.go b/internal/handler/conda_test.go index 8685dca8..1b570395 100644 --- a/internal/handler/conda_test.go +++ b/internal/handler/conda_test.go @@ -1,16 +1,10 @@ package handler import ( - "bytes" "encoding/json" - "errors" "log/slog" "net/http" "net/http/httptest" - "strconv" - "strings" - "sync" - "sync/atomic" "testing" "time" @@ -309,251 +303,3 @@ func TestCondaHandleRepodataWithoutCooldown(t *testing.T) { t.Fatalf("status = %d, want %d", w.Code, http.StatusOK) } } - -// TestCondaRepodataRequestsGzip covers issue #305: the large *.json repodata -// routes must fetch, cache and serve gzip-compressed (Content-Encoding: gzip) -// so neither hop pays the uncompressed size, while repodata.json.bz2 (already -// compressed) stays identity. conda/mamba/pixi solicit and decode gzip on .json. -func TestCondaRepodataRequestsGzip(t *testing.T) { - plain := []byte(`{"packages":{},"repodata_version":1}`) - compressed := gzipPayload(t, plain) - - var available atomic.Bool - available.Store(true) - var jsonUpstreamReqs atomic.Int32 - var sawAcceptEncoding sync.Map // path -> last Accept-Encoding seen - - upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - sawAcceptEncoding.Store(r.URL.Path, r.Header.Get(headerAcceptEncoding)) - if strings.HasSuffix(r.URL.Path, ".json") { - jsonUpstreamReqs.Add(1) - } - if !available.Load() { - http.Error(w, "unavailable", http.StatusServiceUnavailable) - return - } - if strings.HasSuffix(r.URL.Path, ".json") { - if strings.Contains(r.Header.Get(headerAcceptEncoding), "gzip") { - w.Header().Set(headerContentType, contentTypeJSON) - w.Header().Set(headerContentEncoding, "gzip") - _, _ = w.Write(compressed) - return - } - w.Header().Set(headerContentType, contentTypeJSON) - _, _ = w.Write(plain) - return - } - // .bz2: already compressed, upstream sends no Content-Encoding. - w.Header().Set(headerContentType, "application/octet-stream") - _, _ = w.Write([]byte("bz2-bytes")) - })) - defer upstream.Close() - - proxy, _, _, _ := setupTestProxy(t) - proxy.CacheMetadata = true - proxy.MetadataTTL = time.Hour - proxy.HTTPClient = upstream.Client() - routes := NewCondaHandlerWithUpstream(proxy, "http://proxy.local", upstream.URL).Routes() - - get := func(path string) *httptest.ResponseRecorder { - w := httptest.NewRecorder() - routes.ServeHTTP(w, httptest.NewRequest(http.MethodGet, path, nil)) - return w - } - lastAE := func(path string) string { - v, _ := sawAcceptEncoding.Load(path) - s, _ := v.(string) - return s - } - - for _, name := range []string{"repodata.json", "current_repodata.json"} { - path := "/conda-forge/linux-64/" + name - w := get(path) - if w.Code != http.StatusOK { - t.Fatalf("%s: status = %d, want 200: %s", name, w.Code, w.Body.String()) - } - if got := lastAE(path); got != "gzip" { - t.Errorf("%s: upstream Accept-Encoding = %q, want %q", name, got, "gzip") - } - if !bytes.Equal(w.Body.Bytes(), compressed) { - t.Errorf("%s: body not the compressed bytes (got %d, want %d)", name, w.Body.Len(), len(compressed)) - } - if got := w.Header().Get(headerContentEncoding); got != "gzip" { - t.Errorf("%s: Content-Encoding = %q, want %q", name, got, "gzip") - } - if got := w.Header().Get(headerContentLength); got != strconv.Itoa(len(compressed)) { - t.Errorf("%s: Content-Length = %q, want %d", name, got, len(compressed)) - } - } - - // .bz2 stays identity. - wbz := get("/conda-forge/linux-64/repodata.json.bz2") - if wbz.Code != http.StatusOK { - t.Fatalf("bz2: status = %d, want 200", wbz.Code) - } - if got := lastAE("/conda-forge/linux-64/repodata.json.bz2"); got != "identity" { - t.Errorf("bz2: upstream Accept-Encoding = %q, want %q", got, "identity") - } - if got := wbz.Header().Get(headerContentEncoding); got != "" { - t.Errorf("bz2: Content-Encoding = %q, want empty", got) - } - - // Cached replay with the upstream down: same compressed bytes + header, no new .json fetch. - reqsBefore := jsonUpstreamReqs.Load() - available.Store(false) - wc := get("/conda-forge/linux-64/repodata.json") - if wc.Code != http.StatusOK { - t.Fatalf("cached repodata.json: status = %d, want 200: %s", wc.Code, wc.Body.String()) - } - if !bytes.Equal(wc.Body.Bytes(), compressed) { - t.Errorf("cached repodata.json: body not the compressed bytes") - } - if got := wc.Header().Get(headerContentEncoding); got != "gzip" { - t.Errorf("cached repodata.json: Content-Encoding = %q, want %q", got, "gzip") - } - if jsonUpstreamReqs.Load() != reqsBefore { - t.Errorf("cached repodata.json hit upstream: reqs %d -> %d", reqsBefore, jsonUpstreamReqs.Load()) - } -} - -// TestCondaRepodataStreamPathRequestsGzip covers the default cache_metadata=off -// branch: the streaming path must also request gzip for repodata.json and -// forward the Content-Encoding header. -func TestCondaRepodataStreamPathRequestsGzip(t *testing.T) { - plain := []byte(`{"packages":{}}`) - compressed := gzipPayload(t, plain) - var sawAcceptEncoding string - upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - sawAcceptEncoding = r.Header.Get(headerAcceptEncoding) - if strings.Contains(sawAcceptEncoding, "gzip") { - w.Header().Set(headerContentType, contentTypeJSON) - w.Header().Set(headerContentEncoding, "gzip") - _, _ = w.Write(compressed) - return - } - w.Header().Set(headerContentType, contentTypeJSON) - _, _ = w.Write(plain) - })) - defer upstream.Close() - - proxy, _, _, _ := setupTestProxy(t) - proxy.CacheMetadata = false - proxy.HTTPClient = upstream.Client() - routes := NewCondaHandlerWithUpstream(proxy, "http://proxy.local", upstream.URL).Routes() - - w := httptest.NewRecorder() - routes.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/conda-forge/linux-64/repodata.json", nil)) - if w.Code != http.StatusOK { - t.Fatalf("status = %d, want 200: %s", w.Code, w.Body.String()) - } - if sawAcceptEncoding != "gzip" { - t.Errorf("stream path upstream Accept-Encoding = %q, want %q", sawAcceptEncoding, "gzip") - } - if !bytes.Equal(w.Body.Bytes(), compressed) { - t.Errorf("stream path body not the compressed bytes") - } - if got := w.Header().Get(headerContentEncoding); got != "gzip" { - t.Errorf("stream path Content-Encoding = %q, want %q", got, "gzip") - } -} - -// TestCondaRepodataGzipSurvivesCacheWriteFailure covers the failure the gzip -// route makes reachable: when the metadata cache write fails the freshly -// fetched body is still served, so its Content-Encoding must come from the -// fetch and not from the (unwritten) cache row -- otherwise gzip bytes go out -// labelled application/json with no Content-Encoding and every conda client -// fails to parse them. -func TestCondaRepodataGzipSurvivesCacheWriteFailure(t *testing.T) { - plain := []byte(`{"packages":{},"repodata_version":1}`) - compressed := gzipPayload(t, plain) - - upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if strings.Contains(r.Header.Get(headerAcceptEncoding), "gzip") { - w.Header().Set(headerContentType, contentTypeJSON) - w.Header().Set(headerContentEncoding, "gzip") - _, _ = w.Write(compressed) - return - } - w.Header().Set(headerContentType, contentTypeJSON) - _, _ = w.Write(plain) - })) - defer upstream.Close() - - proxy, _, store, _ := setupTestProxy(t) - proxy.CacheMetadata = true - proxy.MetadataTTL = time.Hour - proxy.HTTPClient = upstream.Client() - store.storeErr = errors.New("disk full") - - routes := NewCondaHandlerWithUpstream(proxy, "http://proxy.local", upstream.URL).Routes() - w := httptest.NewRecorder() - routes.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/conda-forge/linux-64/repodata.json", nil)) - - if w.Code != http.StatusOK { - t.Fatalf("status = %d, want 200: %s", w.Code, w.Body.String()) - } - if !bytes.Equal(w.Body.Bytes(), compressed) { - t.Fatalf("body is not the fetched compressed bytes (got %d, want %d)", w.Body.Len(), len(compressed)) - } - // The cache row was never written, so the header must come from the fetch. - if got := w.Header().Get(headerContentEncoding); got != "gzip" { - t.Errorf("Content-Encoding = %q, want %q (gzip body would be unparseable without it)", got, "gzip") - } - if got := w.Header().Get(headerContentLength); got != strconv.Itoa(len(compressed)) { - t.Errorf("Content-Length = %q, want %d", got, len(compressed)) - } -} - -// TestCondaRepodataGzipStaleFallbackKeepsEncoding pins the stale-fallback -// return of fetchOrCacheMetadata: when the upstream fails after the entry has -// expired, the stored gzip blob must be served with its Content-Encoding taken -// from the cache row, not dropped. -func TestCondaRepodataGzipStaleFallbackKeepsEncoding(t *testing.T) { - plain := []byte(`{"packages":{},"repodata_version":1}`) - compressed := gzipPayload(t, plain) - - var available atomic.Bool - available.Store(true) - upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if !available.Load() { - http.Error(w, "unavailable", http.StatusServiceUnavailable) - return - } - if strings.Contains(r.Header.Get(headerAcceptEncoding), "gzip") { - w.Header().Set(headerContentType, contentTypeJSON) - w.Header().Set(headerContentEncoding, "gzip") - _, _ = w.Write(compressed) - return - } - w.Header().Set(headerContentType, contentTypeJSON) - _, _ = w.Write(plain) - })) - defer upstream.Close() - - proxy, _, _, _ := setupTestProxy(t) - proxy.CacheMetadata = true - proxy.MetadataTTL = 0 // every request revalidates; an upstream failure falls back to the stale row - proxy.HTTPClient = upstream.Client() - routes := NewCondaHandlerWithUpstream(proxy, "http://proxy.local", upstream.URL).Routes() - - get := func() *httptest.ResponseRecorder { - w := httptest.NewRecorder() - routes.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/conda-forge/linux-64/repodata.json", nil)) - return w - } - - if first := get(); first.Code != http.StatusOK { - t.Fatalf("first status = %d, want 200: %s", first.Code, first.Body.String()) - } - available.Store(false) - stale := get() - if stale.Code != http.StatusOK { - t.Fatalf("stale status = %d, want 200: %s", stale.Code, stale.Body.String()) - } - if !bytes.Equal(stale.Body.Bytes(), compressed) { - t.Errorf("stale body is not the stored compressed bytes") - } - if got := stale.Header().Get(headerContentEncoding); got != "gzip" { - t.Errorf("stale Content-Encoding = %q, want %q", got, "gzip") - } -} diff --git a/internal/handler/proxy_cached_encoding_test.go b/internal/handler/proxy_cached_encoding_test.go new file mode 100644 index 00000000..e1ee33f0 --- /dev/null +++ b/internal/handler/proxy_cached_encoding_test.go @@ -0,0 +1,168 @@ +package handler + +import ( + "bytes" + "errors" + "net/http" + "net/http/httptest" + "strconv" + "strings" + "sync/atomic" + "testing" + "time" +) + +// gzipWhenAskedUpstream serves compressed bytes with Content-Encoding: gzip +// when the request advertises gzip, plain bytes otherwise, like a CDN that +// compresses on the fly. It records the last Accept-Encoding it saw and counts +// every request before the availability gate so a cache-miss refetch during a +// simulated outage is observable. +type gzipWhenAskedUpstream struct { + *httptest.Server + available atomic.Bool + requests atomic.Int32 + acceptEncoding atomic.Value // string +} + +func newGzipWhenAskedUpstream(plain, compressed []byte) *gzipWhenAskedUpstream { + u := &gzipWhenAskedUpstream{} + u.available.Store(true) + u.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + u.requests.Add(1) + u.acceptEncoding.Store(r.Header.Get(headerAcceptEncoding)) + if !u.available.Load() { + http.Error(w, "unavailable", http.StatusServiceUnavailable) + return + } + w.Header().Set(headerContentType, contentTypeJSON) + if strings.Contains(r.Header.Get(headerAcceptEncoding), "gzip") { + w.Header().Set(headerContentEncoding, "gzip") + _, _ = w.Write(compressed) + return + } + _, _ = w.Write(plain) + })) + return u +} + +func (u *gzipWhenAskedUpstream) sawAcceptEncoding() string { + s, _ := u.acceptEncoding.Load().(string) + return s +} + +// serveGzip issues one request through proxyCachedWithEncoding asking the +// upstream for gzip. +func serveGzip(proxy *Proxy, upstreamURL string) *httptest.ResponseRecorder { + w := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodGet, "/index.json", nil) + proxy.proxyCachedWithEncoding(w, r, upstreamURL, "gzip-test", "index", "gzip", "*/*") + return w +} + +func assertGzipResponse(t *testing.T, label string, w *httptest.ResponseRecorder, compressed []byte) { + t.Helper() + if w.Code != http.StatusOK { + t.Fatalf("%s: status = %d, want 200: %s", label, w.Code, w.Body.String()) + } + if !bytes.Equal(w.Body.Bytes(), compressed) { + t.Errorf("%s: body is not the compressed bytes (got %d, want %d)", label, w.Body.Len(), len(compressed)) + } + if got := w.Header().Get(headerContentEncoding); got != "gzip" { + t.Errorf("%s: Content-Encoding = %q, want %q", label, got, "gzip") + } + if got := w.Header().Get(headerContentLength); got != strconv.Itoa(len(compressed)) { + t.Errorf("%s: Content-Length = %q, want %d", label, got, len(compressed)) + } +} + +// TestProxyCachedWithEncoding_GzipCachesAndReplays covers the cached path: +// requesting gzip upstream stores the compressed bytes plus Content-Encoding +// and replays both from cache without contacting the upstream again. +func TestProxyCachedWithEncoding_GzipCachesAndReplays(t *testing.T) { + plain := []byte(`{"packages":{}}`) + compressed := gzipPayload(t, plain) + upstream := newGzipWhenAskedUpstream(plain, compressed) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + proxy.CacheMetadata = true + proxy.MetadataTTL = time.Hour + proxy.HTTPClient = upstream.Client() + + first := serveGzip(proxy, upstream.URL+"/index.json") + assertGzipResponse(t, "first", first, compressed) + if got := upstream.sawAcceptEncoding(); got != "gzip" { + t.Errorf("upstream Accept-Encoding = %q, want %q", got, "gzip") + } + + before := upstream.requests.Load() + upstream.available.Store(false) + cached := serveGzip(proxy, upstream.URL+"/index.json") + assertGzipResponse(t, "cached", cached, compressed) + if upstream.requests.Load() != before { + t.Errorf("cached replay hit upstream: requests %d -> %d", before, upstream.requests.Load()) + } +} + +// TestProxyCachedWithEncoding_GzipStreamPath covers the cache_metadata=false +// branch: the streaming path must request gzip and forward Content-Encoding. +func TestProxyCachedWithEncoding_GzipStreamPath(t *testing.T) { + plain := []byte(`{"packages":{}}`) + compressed := gzipPayload(t, plain) + upstream := newGzipWhenAskedUpstream(plain, compressed) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + proxy.CacheMetadata = false + proxy.HTTPClient = upstream.Client() + + w := serveGzip(proxy, upstream.URL+"/index.json") + assertGzipResponse(t, "stream", w, compressed) + if got := upstream.sawAcceptEncoding(); got != "gzip" { + t.Errorf("stream path upstream Accept-Encoding = %q, want %q", got, "gzip") + } +} + +// TestProxyCachedWithEncoding_GzipSurvivesCacheWriteFailure covers the failure +// the gzip mode makes reachable: when the metadata cache write fails the +// freshly fetched body is still served, so its Content-Encoding must come from +// the fetch and not from the (unwritten) cache row -- otherwise gzip bytes go +// out labelled application/json with no Content-Encoding. +func TestProxyCachedWithEncoding_GzipSurvivesCacheWriteFailure(t *testing.T) { + plain := []byte(`{"packages":{}}`) + compressed := gzipPayload(t, plain) + upstream := newGzipWhenAskedUpstream(plain, compressed) + defer upstream.Close() + + proxy, _, store, _ := setupTestProxy(t) + proxy.CacheMetadata = true + proxy.MetadataTTL = time.Hour + proxy.HTTPClient = upstream.Client() + store.storeErr = errors.New("disk full") + + w := serveGzip(proxy, upstream.URL+"/index.json") + assertGzipResponse(t, "store-failure", w, compressed) +} + +// TestProxyCachedWithEncoding_GzipStaleFallbackKeepsEncoding pins the +// stale-fallback return: when the upstream fails after the entry has expired, +// the stored gzip blob is served with its Content-Encoding taken from the +// cache row. +func TestProxyCachedWithEncoding_GzipStaleFallbackKeepsEncoding(t *testing.T) { + plain := []byte(`{"packages":{}}`) + compressed := gzipPayload(t, plain) + upstream := newGzipWhenAskedUpstream(plain, compressed) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + proxy.CacheMetadata = true + proxy.MetadataTTL = 0 // every request revalidates; an upstream failure falls back to the stale row + proxy.HTTPClient = upstream.Client() + + first := serveGzip(proxy, upstream.URL+"/index.json") + assertGzipResponse(t, "first", first, compressed) + + upstream.available.Store(false) + stale := serveGzip(proxy, upstream.URL+"/index.json") + assertGzipResponse(t, "stale", stale, compressed) +} From b83ba429056073c3ec1e7b575d25b71b38f08d23 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sun, 6 Sep 2026 13:28:05 +0200 Subject: [PATCH 08/10] fix(homebrew): fetch the JSON API gzip-compressed on both hops Homebrew (#254) routes every API path through ProxyCached and so, since #304, fetches formula.jws.json (~33 MB plain, ~5 MB gzip) uncompressed on every refresh -- the case that motivated #305. Request gzip for the JSON API via proxyCachedWithEncoding: brew fetches every API download with curl --compressed and decodes Content-Encoding itself, so the compressed bytes and header are cached and served as-is and both hops stay compressed. The analytics endpoints are the one brew consumer fetched without --compressed; they stay on identity. --- internal/handler/homebrew.go | 11 +++- internal/handler/homebrew_test.go | 95 +++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) diff --git a/internal/handler/homebrew.go b/internal/handler/homebrew.go index 0fd5b62f..0af67afc 100644 --- a/internal/handler/homebrew.go +++ b/internal/handler/homebrew.go @@ -55,7 +55,16 @@ func (h *HomebrewHandler) Routes() http.Handler { upstreamURL += "?" + r.URL.RawQuery } - h.proxy.ProxyCached(w, r, upstreamURL, homebrewMetadataEcosystem, homebrewMetadataCacheKey(requestPath, r.URL.RawQuery), "*/*") + // brew fetches every JSON API download with `curl --compressed` and + // decodes Content-Encoding itself, and formula.jws.json is ~33 MB plain + // versus ~5 MB gzip, so keep both hops compressed. The analytics + // endpoints are the one consumer brew fetches without --compressed; + // they stay identity. + acceptEncoding := "gzip" + if strings.HasPrefix(requestPath, "analytics/") { + acceptEncoding = "identity" + } + h.proxy.proxyCachedWithEncoding(w, r, upstreamURL, homebrewMetadataEcosystem, homebrewMetadataCacheKey(requestPath, r.URL.RawQuery), acceptEncoding, "*/*") }) } diff --git a/internal/handler/homebrew_test.go b/internal/handler/homebrew_test.go index e8931ad5..e5d7e5c5 100644 --- a/internal/handler/homebrew_test.go +++ b/internal/handler/homebrew_test.go @@ -1,11 +1,13 @@ package handler import ( + "bytes" "io" "net/http" "net/http/httptest" "strconv" "strings" + "sync/atomic" "testing" "time" @@ -361,3 +363,96 @@ func TestRegisterHomebrewArtifactsRejectsOtherHomebrewRoutes(t *testing.T) { t.Errorf("blocked Homebrew routes made %d upstream requests, want 0", upstreamRequests) } } + +// TestHomebrewHandler_RequestsGzipForAPIPaths covers #305's motivating case: +// the JSON API files are fetched, cached and served gzip-compressed with +// Content-Encoding: gzip (brew fetches them with --compressed), while the +// analytics endpoints, which brew fetches without --compressed, stay identity. +func TestHomebrewHandler_RequestsGzipForAPIPaths(t *testing.T) { + plain := []byte(`{"payload":"signed bytes","signatures":[]}`) + compressed := gzipPayload(t, plain) + + var available atomic.Bool + available.Store(true) + var requests atomic.Int32 + var sawAcceptEncoding atomic.Value // string + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + sawAcceptEncoding.Store(r.Header.Get(headerAcceptEncoding)) + if !available.Load() { + http.Error(w, "unavailable", http.StatusServiceUnavailable) + return + } + w.Header().Set(headerContentType, "application/json") + if strings.Contains(r.Header.Get(headerAcceptEncoding), "gzip") { + w.Header().Set(headerContentEncoding, "gzip") + _, _ = w.Write(compressed) + return + } + _, _ = w.Write(plain) + })) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + proxy.CacheMetadata = true + proxy.MetadataTTL = time.Hour + proxy.HTTPClient = upstream.Client() + h := NewHomebrewHandler(proxy, upstream.URL+"/api").Routes() + + get := func(path string) *httptest.ResponseRecorder { + w := httptest.NewRecorder() + h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, path, nil)) + return w + } + lastAE := func() string { + s, _ := sawAcceptEncoding.Load().(string) + return s + } + + first := get("/formula.jws.json") + if first.Code != http.StatusOK { + t.Fatalf("formula.jws.json: status = %d, want 200: %s", first.Code, first.Body.String()) + } + if got := lastAE(); got != "gzip" { + t.Errorf("formula.jws.json: upstream Accept-Encoding = %q, want %q", got, "gzip") + } + if !bytes.Equal(first.Body.Bytes(), compressed) { + t.Errorf("formula.jws.json: body is not the compressed bytes (got %d, want %d)", first.Body.Len(), len(compressed)) + } + if got := first.Header().Get(headerContentEncoding); got != "gzip" { + t.Errorf("formula.jws.json: Content-Encoding = %q, want %q", got, "gzip") + } + if got := first.Header().Get(headerContentLength); got != strconv.Itoa(len(compressed)) { + t.Errorf("formula.jws.json: Content-Length = %q, want %d", got, len(compressed)) + } + + // Replay from cache with the upstream down: same bytes and header, no refetch. + before := requests.Load() + available.Store(false) + cached := get("/formula.jws.json") + if cached.Code != http.StatusOK { + t.Fatalf("cached formula.jws.json: status = %d, want 200: %s", cached.Code, cached.Body.String()) + } + if !bytes.Equal(cached.Body.Bytes(), compressed) || cached.Header().Get(headerContentEncoding) != "gzip" { + t.Errorf("cached formula.jws.json: body/header not replayed verbatim") + } + if requests.Load() != before { + t.Errorf("cached formula.jws.json hit upstream: requests %d -> %d", before, requests.Load()) + } + available.Store(true) + + // Analytics is fetched by brew without --compressed: stays identity, no header. + analytics := get("/analytics/install/30d.json") + if analytics.Code != http.StatusOK { + t.Fatalf("analytics: status = %d, want 200: %s", analytics.Code, analytics.Body.String()) + } + if got := lastAE(); got != "identity" { + t.Errorf("analytics: upstream Accept-Encoding = %q, want %q", got, "identity") + } + if !bytes.Equal(analytics.Body.Bytes(), plain) { + t.Errorf("analytics: body = %q, want plain %q", analytics.Body.Bytes(), plain) + } + if got := analytics.Header().Get(headerContentEncoding); got != "" { + t.Errorf("analytics: Content-Encoding = %q, want empty", got) + } +} From 0fd10bcf1f87ecfbd6db6be1e8e3fe79b7c5f682 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sun, 6 Sep 2026 19:42:55 +0200 Subject: [PATCH 09/10] fix(handler): leave Accept-Encoding unset in proxyMetadataStream for an empty value fetchUpstreamMetadata treats an empty acceptEncoding as 'do not set the header'; proxyMetadataStream set it unconditionally, which would send an empty Accept-Encoding line if a caller ever passed . Guard it the same way so both paths agree. No caller passes today. --- internal/handler/handler.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/handler/handler.go b/internal/handler/handler.go index 53ced277..74d61bbe 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -1041,8 +1041,11 @@ func (p *Proxy) proxyMetadataStream(w http.ResponseWriter, r *http.Request, upst // Set Accept-Encoding explicitly (identity, or gzip for compressible // verbatim metadata) so Go does not transparently decompress and strip the // Content-Encoding of the bytes we forward, regardless of what the client - // negotiated. - req.Header.Set(headerAcceptEncoding, acceptEncoding) + // negotiated. An empty value leaves the header unset, as in + // fetchUpstreamMetadata. + if acceptEncoding != "" { + req.Header.Set(headerAcceptEncoding, acceptEncoding) + } p.applyUpstreamAuth(req) for _, header := range []string{"If-Modified-Since", "If-None-Match"} { From 9f7772cc1846feb4cf2627412ceff55e69bae46f Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sun, 6 Sep 2026 20:06:18 +0200 Subject: [PATCH 10/10] fix(handler): keep the metadata row and blob from describing different bytes Two ways the cache row could stop describing the stored blob once a caller requests gzip, both raised by the review of #324: - cacheMetadataBlob stored the blob and then discarded the UpsertMetadataCache error. After a successful gzip store and a failed row write, a later TTL hit or stale fallback served the gzip blob with the previous row's encoding. On a row-write failure, log it and delete the blob just written, so the next request refetches instead. - fetchOrCacheMetadata read the row once up front and reused it for the stale fallback. A request that read an identity row, lost the upstream race to a request that stored the gzip blob, and then failed upstream labelled the new blob with the old row. Re-read the row before falling back so the encoding matches the blob as it is now. Both only become harmful with an encoding change, which this branch introduces; the pre-existing validator-from-row read is tracked separately. --- internal/handler/handler.go | 28 +++++- .../handler/proxy_cached_encoding_test.go | 92 +++++++++++++++++++ 2 files changed, 119 insertions(+), 1 deletion(-) diff --git a/internal/handler/handler.go b/internal/handler/handler.go index 74d61bbe..df45d43c 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -768,6 +768,11 @@ func (p *Proxy) fetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, u p.Logger.Warn("upstream metadata fetch failed, checking cache", "ecosystem", ecosystem, "key", cacheKey, "error", err) + // Re-read the row so the encoding describes the blob as it is now: a + // concurrent refetch may have replaced both since entry was read above + // (an identity blob swapped for a gzip one during rollout). + entry = p.currentMetadataEntry(ecosystem, cacheKey, entry) + cached, readErr := p.Storage.Open(ctx, entry.StoragePath) if readErr != nil { return nil, "", "", fmt.Errorf("upstream failed and cached file missing: %w", err) @@ -895,7 +900,7 @@ func (p *Proxy) cacheMetadataBlob(ctx context.Context, ecosystem, cacheKey, stor return } - _ = p.DB.UpsertMetadataCache(&database.MetadataCacheEntry{ + err = p.DB.UpsertMetadataCache(&database.MetadataCacheEntry{ Ecosystem: ecosystem, Name: cacheKey, StoragePath: storagePath, @@ -906,6 +911,27 @@ func (p *Proxy) cacheMetadataBlob(ctx context.Context, ecosystem, cacheKey, stor LastModified: sql.NullTime{Time: meta.lastModified, Valid: !meta.lastModified.IsZero()}, FetchedAt: sql.NullTime{Time: time.Now(), Valid: true}, }) + if err != nil { + // The blob is written but the row describing it is not, so a later + // TTL hit or stale fallback would serve these bytes with the previous + // row's encoding. Drop the blob so row and bytes can never disagree; + // the next request refetches instead. + p.Logger.Warn("failed to record cached metadata, discarding blob", "ecosystem", ecosystem, "key", cacheKey, "error", err) + if delErr := p.Storage.Delete(ctx, storagePath); delErr != nil { + p.Logger.Warn("failed to discard metadata blob", "ecosystem", ecosystem, "key", cacheKey, "error", delErr) + } + } +} + +// currentMetadataEntry re-reads the metadata cache row and returns it, or +// fallback when the row cannot be read. Used before serving a stored blob so +// its encoding comes from the row as it is now rather than from a snapshot +// taken before the upstream fetch. +func (p *Proxy) currentMetadataEntry(ecosystem, cacheKey string, fallback *database.MetadataCacheEntry) *database.MetadataCacheEntry { + if fresh, err := p.DB.GetMetadataCache(ecosystem, cacheKey); err == nil && fresh != nil { + return fresh + } + return fallback } // cachedMeta holds cache validators and freshness state from a metadata cache entry. diff --git a/internal/handler/proxy_cached_encoding_test.go b/internal/handler/proxy_cached_encoding_test.go index e1ee33f0..35c8793b 100644 --- a/internal/handler/proxy_cached_encoding_test.go +++ b/internal/handler/proxy_cached_encoding_test.go @@ -2,6 +2,7 @@ package handler import ( "bytes" + "context" "errors" "net/http" "net/http/httptest" @@ -166,3 +167,94 @@ func TestProxyCachedWithEncoding_GzipStaleFallbackKeepsEncoding(t *testing.T) { stale := serveGzip(proxy, upstream.URL+"/index.json") assertGzipResponse(t, "stale", stale, compressed) } + +// TestProxyCachedWithEncoding_UpsertFailureDiscardsBlob covers the row-write +// failure: when the gzip blob is stored but the cache row cannot be updated, +// the blob must be discarded so a later stale fallback cannot serve gzip +// bytes with the previous row's encoding. The fresh response is still +// correct because its encoding comes from the fetch. +func TestProxyCachedWithEncoding_UpsertFailureDiscardsBlob(t *testing.T) { + plain := []byte(`{"packages":{}}`) + compressed := gzipPayload(t, plain) + upstream := newGzipWhenAskedUpstream(plain, compressed) + defer upstream.Close() + + proxy, db, store, _ := setupTestProxy(t) + proxy.CacheMetadata = true + proxy.MetadataTTL = 0 // every request revalidates + proxy.HTTPClient = upstream.Client() + + // Seed an identity row + plain blob, as every key has before the gzip rollout. + w := httptest.NewRecorder() + proxy.proxyCachedWithEncoding(w, httptest.NewRequest(http.MethodGet, "/index.json", nil), + upstream.URL+"/index.json", "gzip-test", "index", "identity", "*/*") + if w.Code != http.StatusOK { + t.Fatalf("seed status = %d, want 200", w.Code) + } + + // Now DB writes fail while reads keep working. + db.SetMaxOpenConns(1) + if _, err := db.Exec("PRAGMA query_only=1"); err != nil { + t.Fatalf("PRAGMA query_only=1: %v", err) + } + fresh := serveGzip(proxy, upstream.URL+"/index.json") + assertGzipResponse(t, "fresh with failed row write", fresh, compressed) + + storagePath := metadataStoragePath("gzip-test", "index") + if exists, _ := store.Exists(context.Background(), storagePath); exists { + t.Fatalf("blob %s still present after the row write failed", storagePath) + } + + // Upstream down: the stale fallback must not serve the orphaned gzip + // blob under the old identity row. + if _, err := db.Exec("PRAGMA query_only=0"); err != nil { + t.Fatalf("PRAGMA query_only=0: %v", err) + } + upstream.available.Store(false) + stale := serveGzip(proxy, upstream.URL+"/index.json") + if stale.Code == http.StatusOK { + t.Fatalf("stale fallback served status 200 (Content-Encoding=%q, %d bytes) from an orphaned blob; want an error", + stale.Header().Get(headerContentEncoding), stale.Body.Len()) + } +} + +// TestProxyCachedWithEncoding_StaleFallbackRereadsRow covers the rollout race: +// a request that read the identity row, lost the upstream race to a request +// that stored the gzip blob, and then failed upstream must label the blob +// with the row as it is now, not with the row it read at the start. +func TestProxyCachedWithEncoding_StaleFallbackRereadsRow(t *testing.T) { + plain := []byte(`{"packages":{}}`) + compressed := gzipPayload(t, plain) + + var proxy *Proxy + var requests atomic.Int32 + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if requests.Add(1) == 1 { + w.Header().Set(headerContentType, contentTypeJSON) + _, _ = w.Write(plain) // seed request: identity + return + } + // Second request has already read the identity row. Simulate a + // concurrent request finishing first: store the gzip blob and row, + // then fail this request so it takes the stale fallback. + proxy.cacheMetadataBlob(r.Context(), "gzip-test", "index", metadataStoragePath("gzip-test", "index"), + &upstreamMetadata{body: compressed, contentType: contentTypeJSON, contentEncoding: "gzip"}) + http.Error(w, "unavailable", http.StatusServiceUnavailable) + })) + defer upstream.Close() + + proxy, _, _, _ = setupTestProxy(t) + proxy.CacheMetadata = true + proxy.MetadataTTL = 0 + proxy.HTTPClient = upstream.Client() + + w := httptest.NewRecorder() + proxy.proxyCachedWithEncoding(w, httptest.NewRequest(http.MethodGet, "/index.json", nil), + upstream.URL+"/index.json", "gzip-test", "index", "identity", "*/*") + if w.Code != http.StatusOK { + t.Fatalf("seed status = %d, want 200", w.Code) + } + + raced := serveGzip(proxy, upstream.URL+"/index.json") + assertGzipResponse(t, "stale fallback after concurrent gzip store", raced, compressed) +}

AltStyle によって変換されたページ (->オリジナル) /