Skip to content

Navigation Menu

Sign in
Sign up

fix(handler): let the ProxyCached path request a per-call Accept-Encoding #324

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pinguinfuss wants to merge 11 commits into git-pkgs:main
base: main
Choose a base branch
Loading
from pinguinfuss:issue-305-transfer-compression
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ac7f724
fix(handler): fetch conda repodata gzip-compressed on both hops
pinguinfuss Sep 3, 2026
14175bf
fix(handler): pass metadata content-encoding with the body it describes
pinguinfuss Sep 3, 2026
c999ce9
fix(handler): pin the stale-fallback content-encoding and drop a dead...
pinguinfuss Sep 3, 2026
935423b
fix(handler): restore the pre-existing cachedMeta content-encoding field
pinguinfuss Sep 3, 2026
9797a4c
fix(handler): keep conda's proxyCached and .bz2 route as on main
pinguinfuss Sep 4, 2026
5cf1cb1
fix(handler): keep writeMetadataCachedResponse and its callers as on ...
pinguinfuss Sep 5, 2026
5991d95
fix(handler): move the conda gzip change to its own branch
pinguinfuss Sep 5, 2026
288bd78
Merge origin/main into issue-305-transfer-compression
pinguinfuss Sep 6, 2026
b83ba42
fix(homebrew): fetch the JSON API gzip-compressed on both hops
pinguinfuss Sep 6, 2026
0fd10bc
fix(handler): leave Accept-Encoding unset in proxyMetadataStream for ...
pinguinfuss Sep 6, 2026
9f7772c
fix(handler): keep the metadata row and blob from describing differen...
pinguinfuss Sep 6, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Some comments aren't visible on the classic Files Changed page.

122 changes: 86 additions & 36 deletions internal/handler/handler.go
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -697,17 +697,20 @@ 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...)
body, contentType, _, err := p.fetchOrCacheMetadata(ctx, ecosystem, cacheKey, upstreamURL, "", acceptHeaders...)
return body, contentType, err
}

// 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, 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)
Expand All @@ -731,7 +734,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
Expand All @@ -745,35 +748,40 @@ 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 {
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",
"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)
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
Expand All @@ -782,7 +790,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) {
Expand All @@ -801,20 +809,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)

Expand Down Expand Up @@ -893,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,
Expand All @@ -904,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.
Expand Down Expand Up @@ -946,13 +974,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, 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)
Expand All @@ -963,12 +1000,21 @@ func (p *Proxy) ProxyCached(w http.ResponseWriter, r *http.Request, upstreamURL,
return
}

p.writeMetadataCachedResponse(w, r, ecosystem, cacheKey, body, contentType)
p.writeMetadataCachedResponseWithEncoding(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) {
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 != "" {
Expand All @@ -992,8 +1038,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.stale {
w.Header().Set("Warning", `110 - "Response is Stale"`)
Expand All @@ -1006,7 +1052,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(), r.Method, upstreamURL, nil)
if err != nil {
http.Error(w, "failed to create request", http.StatusInternalServerError)
Expand All @@ -1018,10 +1064,14 @@ 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. 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"} {
Expand Down
11 changes: 10 additions & 1 deletion internal/handler/homebrew.go
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, "*/*")
})
}

Expand Down
95 changes: 95 additions & 0 deletions internal/handler/homebrew_test.go
View file Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package handler

import (
"bytes"
"io"
"net/http"
"net/http/httptest"
"strconv"
"strings"
"sync/atomic"
"testing"
"time"

Expand Down Expand Up @@ -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)
}
}
Loading
Loading

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