diff --git a/docs/architecture.md b/docs/architecture.md index 0d37a60..c96ae33 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -189,8 +189,19 @@ metadata_cache ( updated_at DATETIME ) -- indexes: (ecosystem, name) unique + +metadata_dates ( + ecosystem TEXT NOT NULL, + proxy_url TEXT NOT NULL, -- each proxy URL rewrites to its own download URLs + name TEXT NOT NULL, + etag TEXT NOT NULL, -- the rewritten representation being dated + last_modified INTEGER NOT NULL, -- Unix seconds; BIGINT on Postgres + PRIMARY KEY (ecosystem, proxy_url, name) +) ``` +`metadata_dates` holds the `Last-Modified` served with rewritten npm and Composer metadata. A package's date is the second it started serving its current representation, and a new representation is only dated in a later second. Keeping it in the database means replicas sharing one serve the same date, and never give one representation's date to another, so `If-Modified-Since` cannot return 304 for a body the client does not have. + On PostgreSQL, `INTEGER PRIMARY KEY` becomes `SERIAL`, `DATETIME` becomes `TIMESTAMP`, `INTEGER DEFAULT 0` booleans become `BOOLEAN DEFAULT FALSE`, and size/count columns use `BIGINT`. The `MigrateSchema()` function handles backward compatibility with older git-pkgs databases by running named migrations that add missing columns and tables. See [migrations.md](migrations.md) for how to add new schema changes. diff --git a/internal/database/database_test.go b/internal/database/database_test.go index 13f6828..f09a86a 100644 --- a/internal/database/database_test.go +++ b/internal/database/database_test.go @@ -613,7 +613,7 @@ func createTestPostgresDB(t *testing.T) *DB { // Drop and recreate every table CreateSchema creates for clean test state; // leftover migration records make the next CreateSchema fail on the // migrations primary key. - tables := []string{"artifacts", "pending_deletes", "versions", "packages", "vulnerabilities", "metadata_cache", "migrations", "schema_info"} + tables := []string{"artifacts", "pending_deletes", "versions", "packages", "vulnerabilities", "metadata_cache", "metadata_dates", "migrations", "schema_info"} for _, table := range tables { _, _ = db.Exec("DROP TABLE IF EXISTS " + table + " CASCADE") } diff --git a/internal/database/metadata_dates.go b/internal/database/metadata_dates.go new file mode 100644 index 0000000..f705f29 --- /dev/null +++ b/internal/database/metadata_dates.go @@ -0,0 +1,77 @@ +package database + +import ( + "database/sql" + "errors" + "fmt" + "time" +) + +// MetadataDate is the Last-Modified a package's rewritten metadata is served +// with, and the ETag of the rewrite it dates. +// +// Rewritten metadata has no date of its own, so the proxy assigns one when a +// package starts serving a new rewrite. Keeping it in the database rather than +// in process memory is what lets replicas sharing the database agree on it: a +// date one replica gave out is never given by another to a different rewrite. +type MetadataDate struct { + ETag string + LastModified time.Time +} + +type metadataDateRow struct { + ETag string `db:"etag"` + LastModified int64 `db:"last_modified"` +} + +// GetMetadataDate returns the date recorded for a package's metadata as +// rewritten for proxyURL, or nil when there is none. +func (db *DB) GetMetadataDate(ecosystem, proxyURL, name string) (*MetadataDate, error) { + var row metadataDateRow + err := db.Get(&row, db.Rebind(` + SELECT etag, last_modified FROM metadata_dates + WHERE ecosystem = ? AND proxy_url = ? AND name = ? + `), ecosystem, proxyURL, name) + if errors.Is(err, sql.ErrNoRows) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("getting metadata date: %w", err) + } + return &MetadataDate{ETag: row.ETag, LastModified: time.Unix(row.LastModified, 0).UTC()}, nil +} + +// InsertMetadataDate records the first date of a package's metadata and +// reports whether it did; it leaves a date another replica recorded first. +func (db *DB) InsertMetadataDate(ecosystem, proxyURL, name string, date MetadataDate) (bool, error) { + result, err := db.Exec(db.Rebind(` + INSERT INTO metadata_dates (ecosystem, proxy_url, name, etag, last_modified) + VALUES (?, ?, ?, ?, ?) + ON CONFLICT (ecosystem, proxy_url, name) DO NOTHING + `), ecosystem, proxyURL, name, date.ETag, date.LastModified.Unix()) + if err != nil { + return false, fmt.Errorf("inserting metadata date: %w", err) + } + return affectedOne(result) +} + +// SwapMetadataDate replaces a package's date with next if it is still old, +// and reports whether it did; it leaves a date another replica swapped in. +func (db *DB) SwapMetadataDate(ecosystem, proxyURL, name string, old, next MetadataDate) (bool, error) { + result, err := db.Exec(db.Rebind(` + UPDATE metadata_dates SET etag = ?, last_modified = ? + WHERE ecosystem = ? AND proxy_url = ? AND name = ? AND etag = ? AND last_modified = ? + `), next.ETag, next.LastModified.Unix(), ecosystem, proxyURL, name, old.ETag, old.LastModified.Unix()) + if err != nil { + return false, fmt.Errorf("swapping metadata date: %w", err) + } + return affectedOne(result) +} + +func affectedOne(result sql.Result) (bool, error) { + n, err := result.RowsAffected() + if err != nil { + return false, fmt.Errorf("counting affected rows: %w", err) + } + return n == 1, nil +} diff --git a/internal/database/metadata_dates_test.go b/internal/database/metadata_dates_test.go new file mode 100644 index 0000000..66c178a --- /dev/null +++ b/internal/database/metadata_dates_test.go @@ -0,0 +1,177 @@ +package database + +import ( + "database/sql" + "strings" + "testing" + "time" +) + +const testProxyURL = "http://proxy.example" + +func TestMetadataDateMissing(t *testing.T) { + runWithBothDatabases(t, func(t *testing.T, db *DB) { + got, err := db.GetMetadataDate(testEcosystemNPM, testProxyURL, "missing") + if err != nil || got != nil { + t.Errorf("GetMetadataDate() = %+v, %v; want nil, nil", got, err) + } + }) +} + +func TestMetadataDateInsertOnce(t *testing.T) { + runWithBothDatabases(t, func(t *testing.T, db *DB) { + first := MetadataDate{ETag: `"a"`, LastModified: time.Date(2026, 10, 10, 12, 0, 1, 0, time.UTC)} + second := MetadataDate{ETag: `"b"`, LastModified: first.LastModified.Add(time.Second)} + + inserted, err := db.InsertMetadataDate(testEcosystemNPM, testProxyURL, "left-pad", first) + if err != nil || !inserted { + t.Fatalf("first InsertMetadataDate() = %v, %v; want true", inserted, err) + } + inserted, err = db.InsertMetadataDate(testEcosystemNPM, testProxyURL, "left-pad", second) + if err != nil || inserted { + t.Fatalf("second InsertMetadataDate() = %v, %v; want false, the row exists", inserted, err) + } + + got, err := db.GetMetadataDate(testEcosystemNPM, testProxyURL, "left-pad") + if err != nil || got == nil || *got != first { + t.Errorf("GetMetadataDate() = %+v, %v; want %+v", got, err, first) + } + }) +} + +// A package's date is kept per proxy URL: each one rewrites the document to +// different download URLs, so each serves a different representation. +func TestMetadataDateKeyedByProxyURL(t *testing.T) { + runWithBothDatabases(t, func(t *testing.T, db *DB) { + date := MetadataDate{ETag: `"a"`, LastModified: time.Date(2026, 10, 10, 12, 0, 1, 0, time.UTC)} + if _, err := db.InsertMetadataDate(testEcosystemNPM, testProxyURL, "left-pad", date); err != nil { + t.Fatal(err) + } + inserted, err := db.InsertMetadataDate(testEcosystemNPM, "http://other.example", "left-pad", date) + if err != nil || !inserted { + t.Errorf("InsertMetadataDate() for another proxy URL = %v, %v; want true", inserted, err) + } + }) +} + +// SwapMetadataDate is a compare-and-swap: it only replaces the date it was +// given, so of two replicas swapping from the same date only one succeeds. +func TestMetadataDateSwap(t *testing.T) { + runWithBothDatabases(t, func(t *testing.T, db *DB) { + a := MetadataDate{ETag: `"a"`, LastModified: time.Date(2026, 10, 10, 12, 0, 1, 0, time.UTC)} + b := MetadataDate{ETag: `"b"`, LastModified: a.LastModified.Add(time.Second)} + c := MetadataDate{ETag: `"c"`, LastModified: b.LastModified.Add(time.Second)} + if _, err := db.InsertMetadataDate(testEcosystemNPM, testProxyURL, "left-pad", a); err != nil { + t.Fatal(err) + } + + swapped, err := db.SwapMetadataDate(testEcosystemNPM, testProxyURL, "left-pad", a, b) + if err != nil || !swapped { + t.Fatalf("SwapMetadataDate(a, b) = %v, %v; want true", swapped, err) + } + swapped, err = db.SwapMetadataDate(testEcosystemNPM, testProxyURL, "left-pad", a, c) + if err != nil || swapped { + t.Fatalf("SwapMetadataDate(a, c) after a was replaced = %v, %v; want false", swapped, err) + } + + got, err := db.GetMetadataDate(testEcosystemNPM, testProxyURL, "left-pad") + if err != nil || got == nil || *got != b { + t.Errorf("GetMetadataDate() = %+v, %v; want %+v", got, err, b) + } + }) +} + +// Dates are whole seconds in UTC whatever the local zone, so every replica +// reads back the date another one wrote. +func TestMetadataDateIsUTCSeconds(t *testing.T) { + tokyo, err := time.LoadLocation("Asia/Tokyo") + if err != nil { + t.Skipf("time zone data unavailable: %v", err) + } + prev := time.Local + time.Local = tokyo + t.Cleanup(func() { time.Local = prev }) + + runWithBothDatabases(t, func(t *testing.T, db *DB) { + written := time.Date(2026, 10, 10, 21, 0, 1, 700_000_000, tokyo) + if _, err := db.InsertMetadataDate(testEcosystemNPM, testProxyURL, "tz", MetadataDate{ETag: `"a"`, LastModified: written}); err != nil { + t.Fatal(err) + } + got, err := db.GetMetadataDate(testEcosystemNPM, testProxyURL, "tz") + if err != nil || got == nil { + t.Fatalf("GetMetadataDate() = %+v, %v", got, err) + } + if want := time.Date(2026, 10, 10, 12, 0, 1, 0, time.UTC); !got.LastModified.Equal(want) || got.LastModified.Location() != time.UTC { + t.Errorf("LastModified read back as %v, want %v", got.LastModified, want) + } + }) +} + +// TestMetadataDatesMigrationKeepsCachedMetadata upgrades a database from +// before migration 011 that holds cached npm and Composer metadata, and checks +// that the cached rows come through unchanged and dates can be recorded. +func TestMetadataDatesMigrationKeepsCachedMetadata(t *testing.T) { + runWithBothDatabases(t, func(t *testing.T, db *DB) { + rollBackMetadataDates(t, db) + seeded := map[string]string{testEcosystemNPM: "left-pad", "composer": "vendor/pkg"} + before := make(map[string]*MetadataCacheEntry, len(seeded)) + for ecosystem, name := range seeded { + before[ecosystem] = seedMetadataCache(t, db, ecosystem, name) + } + + if err := db.MigrateSchema(); err != nil { + t.Fatalf("MigrateSchema() error = %v", err) + } + + for ecosystem, name := range seeded { + after, err := db.GetMetadataCache(ecosystem, name) + if err != nil || after == nil || *after != *before[ecosystem] { + t.Errorf("metadata row %s/%s after migration = %+v, %v; want %+v", ecosystem, name, after, err, before[ecosystem]) + } + date := MetadataDate{ETag: `"a"`, LastModified: time.Date(2026, 10, 10, 12, 0, 1, 0, time.UTC)} + if inserted, err := db.InsertMetadataDate(ecosystem, testProxyURL, name, date); err != nil || !inserted { + t.Errorf("InsertMetadataDate(%s/%s) after migration = %v, %v; want true", ecosystem, name, inserted, err) + } + } + }) +} + +// seedMetadataCache stores a metadata row as the metadata cache writes one +// and returns it as read back. +func seedMetadataCache(t *testing.T, db *DB, ecosystem, name string) *MetadataCacheEntry { + t.Helper() + err := db.UpsertMetadataCache(&MetadataCacheEntry{ + Ecosystem: ecosystem, + Name: name, + StoragePath: "_metadata/" + ecosystem + "/" + name + "/metadata", + ETag: sql.NullString{String: `"upstream"`, Valid: true}, + ContentType: sql.NullString{String: "application/json", Valid: true}, + ContentDigest: sql.NullString{String: "sha256:" + strings.Repeat("ab", 32), Valid: true}, + Size: sql.NullInt64{Int64: 123, Valid: true}, + LastModified: sql.NullTime{Time: time.Date(2020, 1, 2, 3, 4, 5, 0, time.UTC), Valid: true}, + FetchedAt: sql.NullTime{Time: time.Now().UTC().Truncate(time.Second), Valid: true}, + }) + if err != nil { + t.Fatal(err) + } + entry, err := db.GetMetadataCache(ecosystem, name) + if err != nil || entry == nil { + t.Fatalf("GetMetadataCache(%s/%s) = %v, %v", ecosystem, name, entry, err) + } + return entry +} + +// rollBackMetadataDates returns db to the schema from before migration 011, +// which only added the metadata_dates table. +func rollBackMetadataDates(t *testing.T, db *DB) { + t.Helper() + if _, err := db.Exec("DROP TABLE metadata_dates"); err != nil { + t.Fatal(err) + } + if _, err := db.Exec("DELETE FROM migrations WHERE name = '011_add_metadata_dates'"); err != nil { + t.Fatal(err) + } + if has, err := db.HasTable("metadata_dates"); err != nil || has { + t.Fatalf("HasTable(metadata_dates) after rollback = %v, %v; want false", has, err) + } +} diff --git a/internal/database/schema.go b/internal/database/schema.go index 4aef75c..b8d3f93 100644 --- a/internal/database/schema.go +++ b/internal/database/schema.go @@ -119,6 +119,15 @@ CREATE TABLE IF NOT EXISTS metadata_cache ( ); CREATE UNIQUE INDEX IF NOT EXISTS idx_metadata_eco_name ON metadata_cache(ecosystem, name); +CREATE TABLE IF NOT EXISTS metadata_dates ( + ecosystem TEXT NOT NULL, + proxy_url TEXT NOT NULL, + name TEXT NOT NULL, + etag TEXT NOT NULL, + last_modified INTEGER NOT NULL, + PRIMARY KEY (ecosystem, proxy_url, name) +); + CREATE TABLE IF NOT EXISTS migrations ( name TEXT NOT NULL PRIMARY KEY, applied_at DATETIME NOT NULL @@ -228,6 +237,15 @@ CREATE TABLE IF NOT EXISTS metadata_cache ( ); CREATE UNIQUE INDEX IF NOT EXISTS idx_metadata_eco_name ON metadata_cache(ecosystem, name); +CREATE TABLE IF NOT EXISTS metadata_dates ( + ecosystem TEXT NOT NULL, + proxy_url TEXT NOT NULL, + name TEXT NOT NULL, + etag TEXT NOT NULL, + last_modified BIGINT NOT NULL, + PRIMARY KEY (ecosystem, proxy_url, name) +); + CREATE TABLE IF NOT EXISTS migrations ( name TEXT NOT NULL PRIMARY KEY, applied_at TIMESTAMP NOT NULL @@ -382,6 +400,7 @@ var migrations = []migration{ {"008_add_metadata_content_encoding", migrateAddMetadataContentEncoding}, {"009_add_pending_deletes", migrateAddPendingDeletes}, {"010_add_pending_deletes_queued_at_index", migrateAddPendingDeletesQueuedAtIndex}, + {"011_add_metadata_dates", migrateAddMetadataDates}, } // isTableNotFound returns true if the error indicates a missing table. @@ -681,6 +700,27 @@ func migrateAddPendingDeletesQueuedAtIndex(db *DB) error { return nil } +// migrateAddMetadataDates creates the table of Last-Modified dates for +// rewritten metadata, which replicas sharing the database agree on. +func migrateAddMetadataDates(db *DB) error { + seconds := "INTEGER" + if db.dialect == DialectPostgres { + seconds = "BIGINT" + } + query := fmt.Sprintf(`CREATE TABLE IF NOT EXISTS metadata_dates ( + ecosystem TEXT NOT NULL, + proxy_url TEXT NOT NULL, + name TEXT NOT NULL, + etag TEXT NOT NULL, + last_modified %s NOT NULL, + PRIMARY KEY (ecosystem, proxy_url, name) + )`, seconds) + if _, err := db.Exec(query); err != nil { + return fmt.Errorf("creating metadata_dates table: %w", err) + } + return nil +} + // EnsureMetadataCacheTable creates the metadata_cache table if it doesn't exist. func (db *DB) EnsureMetadataCacheTable() error { has, err := db.HasTable("metadata_cache") diff --git a/internal/handler/composer.go b/internal/handler/composer.go index f7b40ae..b348024 100644 --- a/internal/handler/composer.go +++ b/internal/handler/composer.go @@ -128,14 +128,13 @@ func (h *ComposerHandler) handlePackageMetadata(w http.ResponseWriter, r *http.R repoURL, cacheKey := h.sourceFor(packageName) upstreamURL := fmt.Sprintf("%s/p2/%s/%s.json", repoURL, vendor, pkg) - if rewritten, ok := h.proxy.storedRewrite("composer", cacheKey, h.proxyURL, cacheKey); ok { - w.Header().Set(headerContentType, "application/json") - _, _ = w.Write(rewritten) + if stored, ok := h.proxy.storedRewrite("composer", cacheKey, h.proxyURL, cacheKey); ok { + serveRewrittenMetadata(w, r, stored) return } - body, _, err := h.proxy.FetchOrCacheMetadata(r.Context(), "composer", cacheKey, upstreamURL) - if err != nil { + upstream := h.proxy.fetchMetadataDocument(r.Context(), "composer", cacheKey, upstreamURL, contentTypeJSON) + if err := upstream.err; err != nil { if errors.Is(err, ErrUpstreamNotFound) { http.Error(w, "not found", http.StatusNotFound) return @@ -145,19 +144,18 @@ func (h *ComposerHandler) handlePackageMetadata(w http.ResponseWriter, r *http.R return } - rewritten, err := h.proxy.cachedRewrite(r.Context(), "composer", h.proxyURL, cacheKey, body, h.rewriteMetadata) + rewritten, err := h.proxy.cachedRewrite(r.Context(), "composer", h.proxyURL, cacheKey, upstream.body, upstream.digest, h.rewriteMetadataKeeping) if err != nil { if r.Context().Err() != nil { return // the client left while waiting on a shared rewrite } h.proxy.Logger.Warn("failed to rewrite metadata, proxying original", "error", err) w.Header().Set(headerContentType, "application/json") - _, _ = w.Write(body) + _, _ = w.Write(upstream.body) return } - w.Header().Set(headerContentType, "application/json") - _, _ = w.Write(rewritten) + serveRewrittenMetadata(w, r, rewritten) } // rewriteMetadata rewrites dist URLs in Composer metadata to point at this proxy. @@ -170,16 +168,24 @@ func (h *ComposerHandler) handlePackageMetadata(w http.ResponseWriter, r *http.R // of the original bytes: only dist values are written fresh, and minified // metadata stays minified. func (h *ComposerHandler) rewriteMetadata(body []byte) ([]byte, error) { + out, _, err := h.rewriteMetadataKeeping(body) + return out, err +} + +// rewriteMetadataKeeping is rewriteMetadata that also reports the versions +// it kept, each as its package name and version. +func (h *ComposerHandler) rewriteMetadataKeeping(body []byte) ([]byte, []string, error) { if !json.Valid(body) { - return nil, errors.New("composer metadata is not valid JSON") + return nil, nil, errors.New("composer metadata is not valid JSON") } format, _, err := lookupJSONString(body, "minified") if err != nil { - return nil, err + return nil, nil, err } minified := format == composerMinified var out bytes.Buffer + var kept []string out.Grow(len(body) + len(body)/8) err = rewriteJSONMembers(&out, body, func(out *bytes.Buffer, m jsonMember) (bool, error) { packages := m.value(body) @@ -195,24 +201,25 @@ func (h *ComposerHandler) rewriteMetadata(body []byte) ([]byte, error) { if err != nil { return false, err } - return true, h.writeVersions(out, packageName, versions, minified) + return true, h.writeVersions(out, packageName, versions, minified, &kept) }) }) if err != nil { - return nil, err + return nil, nil, err } - return out.Bytes(), nil + return out.Bytes(), kept, nil } // writeVersions writes one package's version list without the versions in -// cooldown and with each dist pointing at this proxy. +// cooldown and with each dist pointing at this proxy, and appends each version +// it writes to kept as its package name and version. // // Minified lists stay minified. Each version written carries the fields that // differ from what the client has expanded so far, which is the upstream // entry itself unless filtered versions were dropped before it; then it also // carries the changes those versions made. Every version gets its own dist, // since the proxied URL names the version. -func (h *ComposerHandler) writeVersions(out *bytes.Buffer, packageName string, versions []byte, minified bool) error { +func (h *ComposerHandler) writeVersions(out *bytes.Buffer, packageName string, versions []byte, minified bool, kept *[]string) error { packagePURL := canonicalPackagePURL("composer", packageName) upstream, written := newComposerFields(), newComposerFields() devReset, first := false, true @@ -254,6 +261,7 @@ func (h *ComposerHandler) writeVersions(out *bytes.Buffer, packageName string, v disableDefaultNotification(upstream) } h.writeVersion(out, packageName, version, upstream, written) + *kept = append(*kept, packageName+"\x00"+version) return nil }) out.WriteByte(']') diff --git a/internal/handler/handler.go b/internal/handler/handler.go index 036e2a5..e867961 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -206,6 +206,11 @@ type Proxy struct { // rewrites caches metadata documents after their handler rewrites them. // Nil leaves every request to rewrite its own copy. rewrites *rewriteCache + + // now is the clock rewritten metadata is dated by, and started the time + // the process started; see metadataDate. + now func() time.Time + started time.Time } // CooldownPolicy supplies version-age filtering and package-specific durations. @@ -230,6 +235,8 @@ func NewProxy(db *database.DB, store storage.Storage, fetcher fetch.FetcherInter HTTPClient: &http.Client{ Timeout: defaultHTTPTimeout, }, + now: time.Now, + started: time.Now(), } } @@ -1086,24 +1093,35 @@ func (p *Proxy) FetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, u // joined a fetch gets its own validation error directly. It must not modify the // body, which joined callers share. func (p *Proxy) fetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, upstreamURL, acceptEncoding string, validate func([]byte) error, acceptHeaders ...string) ([]byte, string, string, error) { + accept := contentTypeJSON + if len(acceptHeaders) > 0 && acceptHeaders[0] != "" { + accept = acceptHeaders[0] + } + res := p.lookupMetadata(ctx, ecosystem, cacheKey, upstreamURL, accept, acceptEncoding, validate) + return res.body, res.contentType, res.contentEncoding, res.err +} + +// fetchMetadataDocument is FetchOrCacheMetadata for callers that rewrite the +// document and need to know which one they got: the result carries its +// digest along with the body. +func (p *Proxy) fetchMetadataDocument(ctx context.Context, ecosystem, cacheKey, upstreamURL, accept string) metadataResult { + return p.lookupMetadata(ctx, ecosystem, cacheKey, upstreamURL, accept, "", nil) +} + +// lookupMetadata implements fetchOrCacheMetadata and fetchMetadataDocument. +func (p *Proxy) lookupMetadata(ctx context.Context, ecosystem, cacheKey, upstreamURL, accept, acceptEncoding string, validate func([]byte) error) metadataResult { if containsPathTraversal(cacheKey) { - return nil, "", "", fmt.Errorf("invalid cache key: %q", cacheKey) + return metadataResult{err: fmt.Errorf("invalid cache key: %q", cacheKey)} } // Serve from cache if within TTL (skip upstream entirely) if _, hit := p.cachedMetadataState(ctx, ecosystem, cacheKey, validate); hit != nil { metrics.RecordCacheHit(ecosystem) - return hit.body, hit.contentType, hit.contentEncoding, nil + return *hit } p.recordMetadataCacheMiss(ecosystem) - accept := contentTypeJSON - if len(acceptHeaders) > 0 && acceptHeaders[0] != "" { - accept = acceptHeaders[0] - } - - res := p.coalescedMetadataMiss(ctx, ecosystem, cacheKey, upstreamURL, accept, acceptEncoding, validate) - return res.body, res.contentType, res.contentEncoding, res.err + return p.coalescedMetadataMiss(ctx, ecosystem, cacheKey, upstreamURL, accept, acceptEncoding, validate) } // coalescedMetadataMiss handles a metadata cache miss, sharing one upstream @@ -1144,7 +1162,8 @@ func (p *Proxy) cachedMetadataState(ctx context.Context, ecosystem, cacheKey str if entry != nil && p.MetadataTTL > 0 && entry.FetchedAt.Valid && time.Since(entry.FetchedAt.Time) < p.MetadataTTL { data, ct, err := p.readCachedMetadata(ctx, entry, validate) if err == nil { - return entry, &metadataResult{body: data, contentType: ct, contentEncoding: entry.ContentEncoding.String} + hit := metadataResult{body: data, contentType: ct, contentEncoding: entry.ContentEncoding.String}.describedBy(entry) + return entry, &hit } if validate != nil { return nil, nil @@ -1167,10 +1186,11 @@ func (p *Proxy) fetchMetadataFromUpstream(ctx context.Context, ecosystem, cacheK err = validate(meta.body) } if err == nil { + var digest string if p.CacheMetadata { - p.cacheMetadataBlob(ctx, ecosystem, cacheKey, metadataStoragePath(ecosystem, cacheKey), meta) + digest = p.cacheMetadataBlob(ctx, ecosystem, cacheKey, metadataStoragePath(ecosystem, cacheKey), meta) } - return metadataResult{body: meta.body, contentType: meta.contentType, contentEncoding: meta.contentEncoding} + return metadataResult{body: meta.body, contentType: meta.contentType, contentEncoding: meta.contentEncoding, digest: digest} } // Upstream failed -- fall back to cache if available @@ -1193,7 +1213,7 @@ func (p *Proxy) fetchMetadataFromUpstream(ctx context.Context, ecosystem, cacheK p.Logger.Info("serving metadata from cache", "ecosystem", ecosystem, "key", cacheKey) - return metadataResult{body: data, contentType: ct, contentEncoding: entry.ContentEncoding.String} + return metadataResult{body: data, contentType: ct, contentEncoding: entry.ContentEncoding.String}.describedBy(entry) } // metadataResult is what a metadata lookup hands back. A shared fetch gives @@ -1202,7 +1222,19 @@ type metadataResult struct { body []byte contentType string contentEncoding string - err error + // digest identifies body when the metadata cache stored it, and is + // empty otherwise. + digest string + err error +} + +// describedBy returns res with the digest that entry, the cache row its body +// was read for, records. +func (res metadataResult) describedBy(entry *database.MetadataCacheEntry) metadataResult { + if entry.ContentDigest.Valid { + res.digest = entry.ContentDigest.String + } + return res } // inflightMetadata is one metadata fetch that concurrent callers share. res is @@ -1385,16 +1417,22 @@ func (p *Proxy) fetchUpstreamMetadata(ctx context.Context, upstreamURL string, e return meta, nil } -// cacheMetadataBlob stores metadata bytes in storage and updates the database. -func (p *Proxy) cacheMetadataBlob(ctx context.Context, ecosystem, cacheKey, storagePath string, meta *upstreamMetadata) { +// cacheMetadataBlob stores metadata bytes in storage and updates the +// database. It returns the digest it recorded for the bytes, or "" when they +// were not cached. +func (p *Proxy) cacheMetadataBlob(ctx context.Context, ecosystem, cacheKey, storagePath string, meta *upstreamMetadata) string { if p.DB == nil || p.Storage == nil { - return + return "" } size, hash, err := p.Storage.Store(ctx, storagePath, bytes.NewReader(meta.body)) if err != nil { p.Logger.Warn("failed to cache metadata", "ecosystem", ecosystem, "key", cacheKey, "error", err) - return + return "" + } + var digest string + if hash != "" { + digest = "sha256:" + hash } err = p.DB.UpsertMetadataCache(&database.MetadataCacheEntry{ @@ -1406,7 +1444,7 @@ func (p *Proxy) cacheMetadataBlob(ctx context.Context, ecosystem, cacheKey, stor ContentEncoding: sql.NullString{String: meta.contentEncoding, Valid: meta.contentEncoding != ""}, // The digest identifies the stored bytes, so a rewrite cached for them // can be found without reading them back (see storedRewrite). - ContentDigest: sql.NullString{String: "sha256:" + hash, Valid: hash != ""}, + ContentDigest: sql.NullString{String: digest, Valid: digest != ""}, Size: sql.NullInt64{Int64: size, Valid: true}, LastModified: sql.NullTime{Time: meta.lastModified, Valid: !meta.lastModified.IsZero()}, FetchedAt: sql.NullTime{Time: time.Now(), Valid: true}, @@ -1420,7 +1458,9 @@ func (p *Proxy) cacheMetadataBlob(ctx context.Context, ecosystem, cacheKey, stor if delErr := p.Storage.Delete(ctx, storagePath); delErr != nil { p.Logger.Warn("failed to discard metadata blob", "ecosystem", ecosystem, "key", cacheKey, "error", delErr) } + return "" } + return digest } // currentMetadataEntry re-reads the metadata cache row and returns it, or diff --git a/internal/handler/metadata_dates.go b/internal/handler/metadata_dates.go new file mode 100644 index 0000000..fca5a1f --- /dev/null +++ b/internal/handler/metadata_dates.go @@ -0,0 +1,83 @@ +package handler + +import ( + "time" + + "github.com/git-pkgs/proxy/internal/database" +) + +// metadataDateAttempts bounds how often metadataDate retries when other +// replicas keep changing a package's date between its read and its write. +const metadataDateAttempts = 3 + +// metadataDate returns the Last-Modified to serve a package's metadata with, +// as rewritten for proxyURL into the representation with the given ETag, or +// zero for none. +// +// The date is the second the package started serving that representation, +// recorded in the database so that every replica sharing it serves the same +// date. A switch to another representation is dated only in a second later +// than the recorded date, and then with that second, so a date is never given +// to two representations of a package: a client holding one never gets 304 +// for another, whichever replica it asks. Until such a second the +// representation goes undated, and If-Modified-Since is not evaluated. +// +// A package's first date is likewise later than the second this process +// started, as the process it replaced may have dated the package then without +// recording it. A date ahead of this process's clock, recorded by a replica +// whose clock is ahead, is not served until the clock catches up. Without a +// database, or when it fails, nothing is dated. +func (p *Proxy) metadataDate(ecosystem, proxyURL, name, etag string) time.Time { + if p.DB == nil || etag == "" { + return time.Time{} + } + now := p.clock().Truncate(time.Second) + for range metadataDateAttempts { + date, settled, err := p.claimMetadataDate(ecosystem, proxyURL, name, etag, now) + if err != nil { + p.Logger.Warn("failed to date rewritten metadata", "ecosystem", ecosystem, "name", name, "error", err) + return time.Time{} + } + if settled { + if date.After(now) { + return time.Time{} + } + return date + } + } + return time.Time{} +} + +// claimMetadataDate makes one attempt at recording the date of etag for a +// package at now. It reports false when another replica changed the date +// first, and the attempt should be made again. +func (p *Proxy) claimMetadataDate(ecosystem, proxyURL, name, etag string, now time.Time) (time.Time, bool, error) { + current, err := p.DB.GetMetadataDate(ecosystem, proxyURL, name) + if err != nil { + return time.Time{}, false, err + } + next := database.MetadataDate{ETag: etag, LastModified: now} + switch { + case current == nil: + if !now.After(p.started.Truncate(time.Second)) { + return time.Time{}, true, nil + } + inserted, err := p.DB.InsertMetadataDate(ecosystem, proxyURL, name, next) + return now, inserted, err + case current.ETag == etag: + return current.LastModified, true, nil + case !now.After(current.LastModified): + return time.Time{}, true, nil + default: + swapped, err := p.DB.SwapMetadataDate(ecosystem, proxyURL, name, *current, next) + return now, swapped, err + } +} + +// clock reads the time metadata is dated by. +func (p *Proxy) clock() time.Time { + if p.now == nil { + return time.Now() + } + return p.now() +} diff --git a/internal/handler/metadata_dates_upgrade_test.go b/internal/handler/metadata_dates_upgrade_test.go new file mode 100644 index 0000000..4d1ca74 --- /dev/null +++ b/internal/handler/metadata_dates_upgrade_test.go @@ -0,0 +1,234 @@ +package handler + +import ( + "context" + "crypto/rand" + "encoding/hex" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "net/url" + "os" + "strings" + "testing" + "time" + + "github.com/git-pkgs/proxy/internal/database" + "github.com/git-pkgs/registries/fetch" +) + +// upgradeDatabases opens an empty database with the current schema for each +// dialect the proxy supports. A nil database means the dialect is unavailable. +var upgradeDatabases = []struct { + dialect string + open func(t *testing.T) *database.DB +}{ + {"sqlite", func(t *testing.T) *database.DB { + db, err := database.Create(t.TempDir() + "/test.db") + if err != nil { + t.Fatalf("Create failed: %v", err) + } + t.Cleanup(func() { _ = db.Close() }) + return db + }}, + {"postgres", openTestPostgres}, +} + +// openTestPostgres creates a database of its own on the server at +// PROXY_DATABASE_URL, so the test shares no tables with tests elsewhere that +// use the same server, and drops it afterwards. +func openTestPostgres(t *testing.T) *database.DB { + t.Helper() + serverURL := os.Getenv("PROXY_DATABASE_URL") + if serverURL == "" { + return nil + } + server, err := database.OpenPostgres(serverURL) + if err != nil { + t.Fatalf("OpenPostgres failed: %v", err) + } + t.Cleanup(func() { _ = server.Close() }) + + suffix := make([]byte, 8) + _, _ = rand.Read(suffix) + name := "proxy_upgrade_" + hex.EncodeToString(suffix) + if _, err := server.Exec("CREATE DATABASE " + name); err != nil { + t.Fatalf("creating database %s: %v", name, err) + } + t.Cleanup(func() { _, _ = server.Exec("DROP DATABASE IF EXISTS " + name + " WITH (FORCE)") }) + + dbURL, err := url.Parse(serverURL) + if err != nil { + t.Fatal(err) + } + dbURL.Path = "/" + name + db, err := database.OpenPostgres(dbURL.String()) + if err != nil { + t.Fatalf("OpenPostgres(%s) failed: %v", name, err) + } + t.Cleanup(func() { _ = db.Close() }) + if err := db.CreateSchema(); err != nil { + t.Fatalf("CreateSchema failed: %v", err) + } + return db +} + +// rollBackMetadataDates returns db to the schema from before migration 011, +// which only added the metadata_dates table. +func rollBackMetadataDates(t *testing.T, db *database.DB) { + t.Helper() + if _, err := db.Exec("DROP TABLE metadata_dates"); err != nil { + t.Fatal(err) + } + if _, err := db.Exec("DELETE FROM migrations WHERE name = '011_add_metadata_dates'"); err != nil { + t.Fatal(err) + } +} + +// upgradeProxy returns a proxy on db that dates metadata by a clock the test +// moves, a second after the process start it models. +func upgradeProxy(t *testing.T, db *database.DB, u *validatorUpstream) (*Proxy, *testClock) { + t.Helper() + proxy := NewProxy(db, newMockStorage(), &mockFetcher{}, fetch.NewResolver(), slog.New(slog.NewTextHandler(io.Discard, nil))) + proxy.HTTPClient = u.Client() + proxy.CacheMetadata = true + proxy.MetadataTTL = time.Hour + clock := &testClock{t: time.Date(2026, 10, 10, 12, 0, 0, 0, time.UTC)} + restartProxy(proxy, clock) + clock.advance(time.Second) + return proxy, clock +} + +// cachedMetadata is a metadata cache row and the bytes stored for it. +type cachedMetadata struct { + entry database.MetadataCacheEntry + body string +} + +func readCachedMetadata(t *testing.T, proxy *Proxy, ecosystem, name string) cachedMetadata { + t.Helper() + entry, err := proxy.DB.GetMetadataCache(ecosystem, name) + if err != nil || entry == nil { + t.Fatalf("metadata row %s/%s = %v, %v", ecosystem, name, entry, err) + } + r, err := proxy.Storage.Open(context.Background(), entry.StoragePath) + if err != nil { + t.Fatalf("opening stored metadata %s: %v", entry.StoragePath, err) + } + defer func() { _ = r.Close() }() + body, err := io.ReadAll(r) + if err != nil { + t.Fatal(err) + } + return cachedMetadata{entry: *entry, body: string(body)} +} + +// TestRewrittenMetadataAfterMetadataDatesMigration upgrades a database from +// before migration 011 that holds cached npm and Composer metadata. The cached +// rows and stored bytes must come through unchanged, and two replicas sharing +// the upgraded database must serve the cached document with validators that +// revalidate, then date a changed document alike. +func TestRewrittenMetadataAfterMetadataDatesMigration(t *testing.T) { + for _, d := range upgradeDatabases { + for _, c := range validatorCases { + t.Run(d.dialect+"/"+c.ecosystem, func(t *testing.T) { + db := d.open(t) + if db == nil { + t.Skip("PROXY_DATABASE_URL not set, skipping postgres test") + } + u := newValidatorUpstream(t) + a, clock := upgradeProxy(t, db, u) + upgradeWithCachedMetadata(t, a, u, c) + + replicas := map[string]http.Handler{"A": c.handler(a, u), "B": c.handler(validatorReplica(a, clock), u)} + cached := serve(replicas["A"], c.path, nil) + if cached.Code != http.StatusOK || strings.Contains(cached.Body.String(), "2.0.0") { + t.Fatalf("first request after the upgrade: status %d, body %s; want 200 with the document cached before it", cached.Code, cached.Body) + } + expectRevalidates(t, replicas, c.path, cached) + + expireMetadata(t, db, c.ecosystem, c.name) + clock.advance(time.Second) + changed := serve(replicas["A"], c.path, nil) + expectChangedDocument(t, cached, changed) + expectChangedForCachedValidators(t, replicas, c.path, cached, changed) + expectRevalidates(t, replicas, c.path, changed) + }) + } + } +} + +// upgradeWithCachedMetadata rolls proxy's database back to before migration +// 011, caches c's document as the proxy did then, moves upstream on to version +// 2.0.0 and migrates. The cached row and stored bytes must survive. +func upgradeWithCachedMetadata(t *testing.T, proxy *Proxy, u *validatorUpstream, c validatorCase) { + t.Helper() + rollBackMetadataDates(t, proxy.DB) + if _, _, err := proxy.FetchOrCacheMetadata(context.Background(), c.ecosystem, c.name, u.URL+c.path); err != nil { + t.Fatalf("seeding metadata before the upgrade: %v", err) + } + seeded := readCachedMetadata(t, proxy, c.ecosystem, c.name) + u.set(npmV("2.0.0"), composerV("2.0.0")) + + if err := proxy.DB.MigrateSchema(); err != nil { + t.Fatalf("MigrateSchema() error = %v", err) + } + if upgraded := readCachedMetadata(t, proxy, c.ecosystem, c.name); upgraded != seeded { + t.Fatalf("cached metadata after the upgrade = %+v, want %+v", upgraded, seeded) + } +} + +// expectChangedDocument checks that changed is the new upstream document, +// with an ETag of its own and a later date than cached. +func expectChangedDocument(t *testing.T, cached, changed *httptest.ResponseRecorder) { + t.Helper() + if changed.Code != http.StatusOK || !strings.Contains(changed.Body.String(), "2.0.0") { + t.Fatalf("after upstream changed: status %d, body %s; want 200 with the new document", changed.Code, changed.Body) + } + if changed.Header().Get("ETag") == cached.Header().Get("ETag") { + t.Error("the changed document has the cached document's ETag") + } + was, _ := http.ParseTime(cached.Header().Get("Last-Modified")) + if now, err := http.ParseTime(changed.Header().Get("Last-Modified")); err != nil || !now.After(was) { + t.Errorf("changed document Last-Modified %q, want later than %q", changed.Header().Get("Last-Modified"), cached.Header().Get("Last-Modified")) + } +} + +// expectChangedForCachedValidators checks that no replica answers a client +// holding the cached document's validators with 304 once it changed. +func expectChangedForCachedValidators(t *testing.T, replicas map[string]http.Handler, path string, cached, changed *httptest.ResponseRecorder) { + t.Helper() + held := []http.Header{ + {"If-None-Match": {cached.Header().Get("ETag")}}, + {"If-Modified-Since": {cached.Header().Get("Last-Modified")}}, + } + for name, handler := range replicas { + for _, header := range held { + if rec := serve(handler, path, header); rec.Code != http.StatusOK || rec.Body.String() != changed.Body.String() { + t.Errorf("replica %s, %v of the cached document: status %d; want 200 with the changed document", name, header, rec.Code) + } + } + } +} + +// expectRevalidates checks that every replica serves want's validators and +// answers a conditional request with either of them with 304. +func expectRevalidates(t *testing.T, replicas map[string]http.Handler, path string, want *httptest.ResponseRecorder) { + t.Helper() + etag, lastModified := want.Header().Get("ETag"), want.Header().Get("Last-Modified") + if etag == "" || lastModified == "" { + t.Fatalf("ETag %q, Last-Modified %q; want both", etag, lastModified) + } + for name, handler := range replicas { + rec := serve(handler, path, nil) + if rec.Header().Get("ETag") != etag || rec.Header().Get("Last-Modified") != lastModified { + t.Errorf("replica %s: ETag %q, Last-Modified %q; want %q, %q", name, rec.Header().Get("ETag"), rec.Header().Get("Last-Modified"), etag, lastModified) + } + for _, header := range []http.Header{{"If-None-Match": {etag}}, {"If-Modified-Since": {lastModified}}} { + if rec := serve(handler, path, header); rec.Code != http.StatusNotModified { + t.Errorf("replica %s, %v: status %d, want 304", name, header, rec.Code) + } + } + } +} diff --git a/internal/handler/metadata_validators_test.go b/internal/handler/metadata_validators_test.go new file mode 100644 index 0000000..a057e5e --- /dev/null +++ b/internal/handler/metadata_validators_test.go @@ -0,0 +1,756 @@ +package handler + +import ( + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + "time" + + "github.com/git-pkgs/cooldown" + "github.com/git-pkgs/proxy/internal/database" +) + +// validatorUpstream serves one npm packument and one Composer document whose +// bodies a test can change, with a fixed Last-Modified. +type validatorUpstream struct { + *httptest.Server + mu sync.Mutex + npm string + composer string + lastModified time.Time +} + +func newValidatorUpstream(t *testing.T) *validatorUpstream { + t.Helper() + u := &validatorUpstream{ + npm: `{"name":"left-pad","versions":{"1.3.0":{"dist":{"tarball":"https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz"}}}}`, + composer: `{"packages":{"vendor/pkg":[{"version":"1.0.0","dist":{"type":"zip","url":"https://example.com/pkg-1.0.0.zip"}}]}}`, + lastModified: time.Date(2020, 1, 2, 3, 4, 5, 0, time.UTC), + } + u.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + u.mu.Lock() + defer u.mu.Unlock() + w.Header().Set("Content-Type", "application/json") + w.Header().Set("Last-Modified", u.lastModified.Format(http.TimeFormat)) + switch r.URL.Path { + case "/left-pad": + _, _ = w.Write([]byte(u.npm)) + case "/p2/vendor/pkg.json": + _, _ = w.Write([]byte(u.composer)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(u.Close) + return u +} + +func (u *validatorUpstream) set(npm, composer string) { + u.mu.Lock() + defer u.mu.Unlock() + u.npm, u.composer = npm, composer +} + +func (u *validatorUpstream) setLastModified(lastModified time.Time) { + u.mu.Lock() + defer u.mu.Unlock() + u.lastModified = lastModified +} + +type validatorCase struct { + ecosystem, name, path string + handler func(*Proxy, *validatorUpstream) http.Handler +} + +var validatorCases = []validatorCase{ + {"npm", "left-pad", "/left-pad", func(p *Proxy, u *validatorUpstream) http.Handler { + return NewNPMHandler(p, "http://proxy.example", u.URL).Routes() + }}, + {"composer", "vendor/pkg", "/p2/vendor/pkg.json", func(p *Proxy, u *validatorUpstream) http.Handler { + return NewComposerHandlerWithUpstreams(p, "http://proxy.example", u.URL, u.URL).Routes() + }}, +} + +func validatorProxy(t *testing.T, u *validatorUpstream, ttl time.Duration) *Proxy { + t.Helper() + proxy, _, _ := validatorProxyWithDB(t, u, ttl) + return proxy +} + +// validatorProxyWithDB returns a proxy that dates metadata by a clock the +// test moves, starting a second after the process start it models. +func validatorProxyWithDB(t *testing.T, u *validatorUpstream, ttl time.Duration) (*Proxy, *database.DB, *testClock) { + t.Helper() + proxy, db, _, _ := setupTestProxy(t) + proxy.HTTPClient = u.Client() + proxy.CacheMetadata = true + proxy.MetadataTTL = ttl + clock := &testClock{t: time.Date(2026, 10, 10, 12, 0, 0, 0, time.UTC)} + restartProxy(proxy, clock) + clock.advance(time.Second) + return proxy, db, clock +} + +// restartProxy gives proxy a new, empty rewrite cache and makes it date +// metadata by clock from the clock's current time on, as a restart of the +// process would. What the database holds survives. +func restartProxy(proxy *Proxy, clock *testClock) { + proxy.SetMetadataRewriteCacheSize(1 << 20) + proxy.now = clock.now + proxy.started = clock.now() +} + +// validatorReplica returns a second proxy process sharing proxy's database +// and storage, as another replica behind the same load balancer would, with +// its own rewrite cache and its own clock. +func validatorReplica(proxy *Proxy, clock *testClock) *Proxy { + replica := NewProxy(proxy.DB, proxy.Storage, proxy.Fetcher, proxy.Resolver, proxy.Logger) + replica.HTTPClient = proxy.HTTPClient + replica.CacheMetadata = proxy.CacheMetadata + replica.MetadataTTL = proxy.MetadataTTL + replica.SetMetadataRewriteCacheSize(1 << 20) + replica.now = clock.now + replica.started = proxy.started + return replica +} + +type testClock struct { + mu sync.Mutex + t time.Time +} + +func (c *testClock) now() time.Time { + c.mu.Lock() + defer c.mu.Unlock() + return c.t +} + +func (c *testClock) advance(d time.Duration) { + c.mu.Lock() + defer c.mu.Unlock() + c.t = c.t.Add(d) +} + +// npmV and composerV are upstream documents with a single version v. +func npmV(v string) string { + return `{"name":"left-pad","versions":{"` + v + `":{"dist":{"tarball":"https://registry.npmjs.org/left-pad/-/left-pad-` + v + `.tgz"}}}}` +} + +func composerV(v string) string { + return `{"packages":{"vendor/pkg":[{"version":"` + v + `","dist":{"type":"zip","url":"https://example.com/pkg-` + v + `.zip"}}]}}` +} + +// expireMetadata ages a metadata cache row past any TTL, so the next request +// refreshes it from upstream. +func expireMetadata(t *testing.T, db *database.DB, ecosystem, name string) { + t.Helper() + entry, err := db.GetMetadataCache(ecosystem, name) + if err != nil || entry == nil { + t.Fatalf("metadata row %s/%s = %v, %v", ecosystem, name, entry, err) + } + entry.FetchedAt.Time = time.Now().Add(-48 * time.Hour) + if err := db.UpsertMetadataCache(entry); err != nil { + t.Fatal(err) + } +} + +func serve(handler http.Handler, path string, header http.Header) *httptest.ResponseRecorder { + req := httptest.NewRequest(http.MethodGet, path, nil) + for k, v := range header { + req.Header[k] = v + } + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + return rec +} + +// TestRewrittenMetadataETag checks that rewritten metadata carries an ETag +// that clients can revalidate with, on the first fetch, on the rewrite fast +// path and when the TTL forces a revalidation. +func TestRewrittenMetadataETag(t *testing.T) { + for _, c := range validatorCases { + for _, ttl := range []time.Duration{time.Hour, 0} { + t.Run(c.ecosystem+"/ttl="+ttl.String(), func(t *testing.T) { + u := newValidatorUpstream(t) + handler := c.handler(validatorProxy(t, u, ttl), u) + + first := serve(handler, c.path, nil) + etag := first.Header().Get("ETag") + if first.Code != http.StatusOK || etag == "" { + t.Fatalf("first response: status %d, ETag %q; want 200 with an ETag", first.Code, etag) + } + for _, ifNoneMatch := range []string{etag, "W/" + etag, `"other", ` + etag} { + expectNotModified(t, serve(handler, c.path, http.Header{"If-None-Match": {ifNoneMatch}}), etag) + } + if rec := serve(handler, c.path, nil); rec.Header().Get("ETag") != etag || rec.Body.String() != first.Body.String() { + t.Errorf("repeat response: ETag %q, want %q, and the same body", rec.Header().Get("ETag"), etag) + } + }) + } + } +} + +func expectNotModified(t *testing.T, rec *httptest.ResponseRecorder, etag string) { + t.Helper() + if rec.Code != http.StatusNotModified || rec.Body.Len() != 0 { + t.Errorf("status %d with %d bytes, want an empty 304", rec.Code, rec.Body.Len()) + } + if got := rec.Header().Get("ETag"); got != etag { + t.Errorf("304 ETag = %q, want %q", got, etag) + } +} + +// TestRewrittenMetadataETagFollowsUpstream checks that a new upstream +// document gets a new ETag, so a client's old copy no longer matches. +func TestRewrittenMetadataETagFollowsUpstream(t *testing.T) { + for _, c := range validatorCases { + t.Run(c.ecosystem, func(t *testing.T) { + u := newValidatorUpstream(t) + handler := c.handler(validatorProxy(t, u, 0), u) + etag := serve(handler, c.path, nil).Header().Get("ETag") + + u.set( + `{"name":"left-pad","versions":{"1.3.1":{"dist":{"tarball":"https://registry.npmjs.org/left-pad/-/left-pad-1.3.1.tgz"}}}}`, + `{"packages":{"vendor/pkg":[{"version":"1.0.1","dist":{"type":"zip","url":"https://example.com/pkg-1.0.1.zip"}}]}}`, + ) + rec := serve(handler, c.path, http.Header{"If-None-Match": {etag}}) + if rec.Code != http.StatusOK || rec.Header().Get("ETag") == etag { + t.Errorf("status %d, ETag %q; want 200 with a new ETag", rec.Code, rec.Header().Get("ETag")) + } + }) + } +} + +// TestRewrittenMetadataLastModified checks the validator Composer revalidates +// with. It is when this process started serving the rewrite, not upstream's +// date: the rewrite also depends on configuration read at startup, so the +// same upstream document can be rewritten differently after a restart. It +// stays the same while the rewrite does. +func TestRewrittenMetadataLastModified(t *testing.T) { + for _, c := range validatorCases { + for _, ttl := range []time.Duration{time.Hour, 0} { + t.Run(c.ecosystem+"/ttl="+ttl.String(), func(t *testing.T) { + u := newValidatorUpstream(t) + proxy, _, clock := validatorProxyWithDB(t, u, ttl) + handler := c.handler(proxy, u) + + clock.advance(400 * time.Millisecond) + first := serve(handler, c.path, nil) + lastModified, err := http.ParseTime(first.Header().Get("Last-Modified")) + if err != nil { + t.Fatalf("Last-Modified %q: %v", first.Header().Get("Last-Modified"), err) + } + if want := clock.now().Truncate(time.Second); !lastModified.Equal(want) { + t.Errorf("Last-Modified %v, want %v, the second of the rewrite", lastModified, want) + } + clock.advance(time.Hour) + + current := serve(handler, c.path, http.Header{"If-Modified-Since": {first.Header().Get("Last-Modified")}}) + if current.Code != http.StatusNotModified || current.Body.Len() != 0 { + t.Errorf("If-Modified-Since Last-Modified: status %d with %d bytes, want an empty 304", current.Code, current.Body.Len()) + } + + older := lastModified.Add(-time.Hour).Format(http.TimeFormat) + if rec := serve(handler, c.path, http.Header{"If-Modified-Since": {older}}); rec.Code != http.StatusOK { + t.Errorf("If-Modified-Since an hour earlier: status %d, want 200", rec.Code) + } + + // If-None-Match takes precedence over If-Modified-Since. + rec := serve(handler, c.path, http.Header{ + "If-None-Match": {`"other"`}, + "If-Modified-Since": {first.Header().Get("Last-Modified")}, + }) + if rec.Code != http.StatusOK { + t.Errorf("mismatched If-None-Match with a current If-Modified-Since: status %d, want 200", rec.Code) + } + }) + } + } +} + +// TestRewrittenMetadataLastModifiedFollowsRefresh is a regression test for a +// false 304. Upstream dates both documents before this process started, the +// cached one and the newer one a refresh fetches, and neither may give the +// newer document a Last-Modified a client holding the older one already has: +// If-Modified-Since from the first response must fetch the new body. +func TestRewrittenMetadataLastModifiedFollowsRefresh(t *testing.T) { + for _, c := range validatorCases { + for _, ttl := range []time.Duration{time.Hour, 0} { + t.Run(c.ecosystem+"/ttl="+ttl.String(), func(t *testing.T) { + u := newValidatorUpstream(t) + proxy, db, clock := validatorProxyWithDB(t, u, ttl) + handler := c.handler(proxy, u) + + first := serve(handler, c.path, nil) + firstModified := first.Header().Get("Last-Modified") + if first.Code != http.StatusOK || firstModified == "" { + t.Fatalf("first response: status %d, Last-Modified %q; want 200 with a Last-Modified", first.Code, firstModified) + } + + clock.advance(time.Minute) + u.set(npmV("1.3.1"), composerV("1.0.1")) + u.setLastModified(u.lastModified.Add(24 * time.Hour)) + expireMetadata(t, db, c.ecosystem, c.name) + + rec := serve(handler, c.path, http.Header{"If-Modified-Since": {firstModified}}) + if rec.Code != http.StatusOK || rec.Body.String() == first.Body.String() { + t.Fatalf("If-Modified-Since from before the refresh: status %d, body changed %t; want 200 with the new body", + rec.Code, rec.Body.String() != first.Body.String()) + } + was, _ := http.ParseTime(firstModified) + now, err := http.ParseTime(rec.Header().Get("Last-Modified")) + if err != nil || !now.After(was) { + t.Errorf("Last-Modified after the refresh = %q, want later than %q", rec.Header().Get("Last-Modified"), firstModified) + } + if rec := serve(handler, c.path, http.Header{"If-Modified-Since": {rec.Header().Get("Last-Modified")}}); rec.Code != http.StatusNotModified { + t.Errorf("If-Modified-Since the new Last-Modified: status %d, want 304", rec.Code) + } + }) + } + } +} + +// TestRewrittenMetadataLastModifiedReturningDocument is a regression test +// for a false 304. Upstream serves A, then B, then A again; A's cached +// rewrite must not bring back the date it was first served with, which is +// earlier than B's, or a client holding B gets 304 for A. +func TestRewrittenMetadataLastModifiedReturningDocument(t *testing.T) { + for _, c := range validatorCases { + for _, ttl := range []time.Duration{time.Hour, 0} { + t.Run(c.ecosystem+"/ttl="+ttl.String(), func(t *testing.T) { + u := newValidatorUpstream(t) + proxy, db, clock := validatorProxyWithDB(t, u, ttl) + handler := c.handler(proxy, u) + change := func(v string) { + clock.advance(10 * time.Second) + u.set(npmV(v), composerV(v)) + if ttl > 0 { + expireMetadata(t, db, c.ecosystem, c.name) + } + } + + u.set(npmV("1.0.0"), composerV("1.0.0")) + a := serve(handler, c.path, nil) + change("2.0.0") + b := serve(handler, c.path, nil) + if b.Body.String() == a.Body.String() || b.Header().Get("Last-Modified") == "" { + t.Fatalf("B: Last-Modified %q, body changed %t; want a new body with a date", b.Header().Get("Last-Modified"), b.Body.String() != a.Body.String()) + } + change("1.0.0") + + rec := serve(handler, c.path, http.Header{"If-Modified-Since": {b.Header().Get("Last-Modified")}}) + if rec.Code != http.StatusOK || rec.Body.String() != a.Body.String() { + t.Fatalf("A again, If-Modified-Since B's date: status %d, body is A %t; want 200 with A", rec.Code, rec.Body.String() == a.Body.String()) + } + was, _ := http.ParseTime(b.Header().Get("Last-Modified")) + if now, err := http.ParseTime(rec.Header().Get("Last-Modified")); err != nil || !now.After(was) { + t.Errorf("A again: Last-Modified %q, want later than B's %q", rec.Header().Get("Last-Modified"), b.Header().Get("Last-Modified")) + } + }) + } + } +} + +// TestRewrittenMetadataLastModifiedSameSecond checks that a document that +// replaces another within the second the other was dated gets no date of +// its own until a later second, rather than one ahead of the clock. The +// clock-time date it would share is one a client may already hold. +func TestRewrittenMetadataLastModifiedSameSecond(t *testing.T) { + for _, c := range validatorCases { + t.Run(c.ecosystem, func(t *testing.T) { + u := newValidatorUpstream(t) + proxy, _, clock := validatorProxyWithDB(t, u, 0) + handler := c.handler(proxy, u) + + a := serve(handler, c.path, nil) + aModified := a.Header().Get("Last-Modified") + u.set(npmV("2.0.0"), composerV("2.0.0")) + for i, header := range []http.Header{nil, {"If-Modified-Since": {aModified}}} { + rec := serve(handler, c.path, header) + if rec.Code != http.StatusOK || rec.Body.String() == a.Body.String() { + t.Fatalf("request %d for B: status %d; want 200 with B", i, rec.Code) + } + if lm := rec.Header().Get("Last-Modified"); lm != "" { + t.Errorf("request %d for B in A's second: Last-Modified %q, want none", i, lm) + } + } + + clock.advance(time.Second) + rec := serve(handler, c.path, nil) + lastModified, err := http.ParseTime(rec.Header().Get("Last-Modified")) + if want := clock.now().Truncate(time.Second); err != nil || !lastModified.Equal(want) { + t.Errorf("B a second later: Last-Modified %q, want %v", rec.Header().Get("Last-Modified"), want) + } + }) + } +} + +// TestRewrittenMetadataLastModifiedAcrossRestart is a regression test for a +// false 304 after a restart. Dates are never ahead of the clock, and the +// restarted process dates nothing in the second it starts, the last in which +// the previous process may have dated a document. +func TestRewrittenMetadataLastModifiedAcrossRestart(t *testing.T) { + for _, c := range validatorCases { + t.Run(c.ecosystem, func(t *testing.T) { + u := newValidatorUpstream(t) + proxy, _, clock := validatorProxyWithDB(t, u, 0) + handler := c.handler(proxy, u) + + // Two documents in one second; the client keeps the latest date. + latest := serve(handler, c.path, nil).Header().Get("Last-Modified") + u.set(npmV("2.0.0"), composerV("2.0.0")) + if lm := serve(handler, c.path, nil).Header().Get("Last-Modified"); lm != "" { + latest = lm + } + if held, _ := http.ParseTime(latest); held.After(clock.now()) { + t.Errorf("Last-Modified %q is ahead of the clock at %v", latest, clock.now()) + } + + restartProxy(proxy, clock) + u.set(npmV("3.0.0"), composerV("3.0.0")) + rec := serve(handler, c.path, http.Header{"If-Modified-Since": {latest}}) + if rec.Code != http.StatusOK || !strings.Contains(rec.Body.String(), "3.0.0") { + t.Fatalf("changed document after a restart, If-Modified-Since %q: status %d; want 200 with it", latest, rec.Code) + } + if lm := rec.Header().Get("Last-Modified"); lm != "" { + t.Errorf("Last-Modified %q in the second the process started, want none", lm) + } + + clock.advance(time.Second) + rec = serve(handler, c.path, http.Header{"If-Modified-Since": {latest}}) + if rec.Code != http.StatusOK || rec.Header().Get("Last-Modified") == "" { + t.Errorf("a second after the restart: status %d, Last-Modified %q; want 200 with a date", rec.Code, rec.Header().Get("Last-Modified")) + } + }) + } +} + +// TestRewrittenMetadataValidatorsWithCooldown checks that with cooldown on, +// rewritten metadata keeps an exact ETag but has no Last-Modified: cooldown +// changes the rewrite as versions age in, while upstream stays the same. +func TestRewrittenMetadataValidatorsWithCooldown(t *testing.T) { + for _, c := range validatorCases { + t.Run(c.ecosystem, func(t *testing.T) { + u := newValidatorUpstream(t) + proxy := validatorProxy(t, u, time.Hour) + proxy.Cooldown = &cooldown.Config{Default: "3d"} + handler := c.handler(proxy, u) + + first := serve(handler, c.path, nil) + etag := first.Header().Get("ETag") + if etag == "" { + t.Fatal("no ETag with cooldown on") + } + if lm := first.Header().Get("Last-Modified"); lm != "" { + t.Errorf("Last-Modified = %q with cooldown on, want none", lm) + } + if rec := serve(handler, c.path, http.Header{"If-None-Match": {etag}}); rec.Code != http.StatusNotModified { + t.Errorf("If-None-Match: status %d, want 304", rec.Code) + } + future := time.Now().Add(time.Hour).Format(http.TimeFormat) + if rec := serve(handler, c.path, http.Header{"If-Modified-Since": {future}}); rec.Code != http.StatusOK { + t.Errorf("If-Modified-Since: status %d, want 200", rec.Code) + } + }) + } +} + +// TestUnrewrittenMetadataHasNoValidators checks that metadata proxied as it +// is, because rewriting it failed, does not claim the rewrite's validators. +func TestUnrewrittenMetadataHasNoValidators(t *testing.T) { + u := newValidatorUpstream(t) + u.set(`{"name":"left-pad","versions":`, `{"packages":`) + for _, c := range validatorCases { + t.Run(c.ecosystem, func(t *testing.T) { + rec := serve(c.handler(validatorProxy(t, u, time.Hour), u), c.path, nil) + if rec.Code != http.StatusOK { + t.Fatalf("status %d: %s", rec.Code, rec.Body.String()) + } + if etag, lm := rec.Header().Get("ETag"), rec.Header().Get("Last-Modified"); etag != "" || lm != "" { + t.Errorf("ETag %q, Last-Modified %q; want neither on unrewritten metadata", etag, lm) + } + }) + } +} + +func TestRewriteCacheETag(t *testing.T) { + c := newRewriteCache(1 << 20) + exclaim := func(b []byte) ([]byte, error) { return []byte(string(b) + "!"), nil } + key := rewriteCacheKey("npm", "http://proxy", "demo", []byte("doc")) + + out, etag, err := c.rewrite(t.Context(), key, []byte("doc"), exclaim) + if err != nil || string(out) != "doc!" { + t.Fatalf("rewrite = %q, %v", out, err) + } + if want := metadataETag([]byte("doc!")); etag.etag != want { + t.Errorf("rewrite ETag = %q, want %q", etag.etag, want) + } + if _, cached, ok := c.get(key); !ok || cached != etag { + t.Errorf("get validators = %+v, %v; want %+v", cached, ok, etag) + } + if metadataETag([]byte("other")) == etag.etag { + t.Error("different output has the same ETag") + } +} + +// TestRewrittenMetadataLastModifiedWithoutRewriteCache checks that dates do +// not depend on the rewrite cache: the database records them. +func TestRewrittenMetadataLastModifiedWithoutRewriteCache(t *testing.T) { + for _, c := range validatorCases { + t.Run(c.ecosystem, func(t *testing.T) { + u := newValidatorUpstream(t) + proxy, _, clock := validatorProxyWithDB(t, u, time.Hour) + proxy.SetMetadataRewriteCacheSize(0) + handler := c.handler(proxy, u) + + first := serve(handler, c.path, nil) + if first.Code != http.StatusOK || first.Header().Get("ETag") == "" { + t.Fatalf("status %d, ETag %q; want 200 with an ETag", first.Code, first.Header().Get("ETag")) + } + lastModified, err := http.ParseTime(first.Header().Get("Last-Modified")) + if want := clock.now().Truncate(time.Second); err != nil || !lastModified.Equal(want) { + t.Errorf("Last-Modified %q without the rewrite cache, want %v", first.Header().Get("Last-Modified"), want) + } + clock.advance(time.Minute) + if rec := serve(handler, c.path, http.Header{"If-Modified-Since": {first.Header().Get("Last-Modified")}}); rec.Code != http.StatusNotModified { + t.Errorf("If-Modified-Since Last-Modified: status %d, want 304", rec.Code) + } + }) + } +} + +// TestRewrittenMetadataLastModifiedAcrossReplicas is a regression test for a +// false 304 between replicas sharing a database and storage. One replica +// dates a document, upstream changes in that second, and a client holding +// the date asks another replica: the date must not suppress the new body. +// Once a second has passed, both replicas date the new document alike. +func TestRewrittenMetadataLastModifiedAcrossReplicas(t *testing.T) { + for _, c := range validatorCases { + t.Run(c.ecosystem, func(t *testing.T) { + u := newValidatorUpstream(t) + a, _, clock := validatorProxyWithDB(t, u, 0) + b := validatorReplica(a, clock) + replicas := map[string]http.Handler{"A": c.handler(a, u), "B": c.handler(b, u)} + + first := serve(replicas["A"], c.path, nil) + held := first.Header().Get("Last-Modified") + if first.Code != http.StatusOK || held == "" { + t.Fatalf("replica A: status %d, Last-Modified %q; want 200 with a date", first.Code, held) + } + u.set(npmV("2.0.0"), composerV("2.0.0")) + for _, name := range []string{"B", "A"} { + expectChangedUndated(t, name, serve(replicas[name], c.path, http.Header{"If-Modified-Since": {held}}), first) + } + + clock.advance(time.Second) + shared := expectSharedDate(t, replicas, c.path, held) + for name, handler := range replicas { + if rec := serve(handler, c.path, http.Header{"If-Modified-Since": {shared}}); rec.Code != http.StatusNotModified { + t.Errorf("replica %s, If-Modified-Since the shared date: status %d, want 304", name, rec.Code) + } + if rec := serve(handler, c.path, http.Header{"If-Modified-Since": {held}}); rec.Code != http.StatusOK { + t.Errorf("replica %s, If-Modified-Since the old date: status %d, want 200", name, rec.Code) + } + } + }) + } +} + +// expectChangedUndated checks that a replica answered a conditional request +// with a body and ETag other than first's, and no date: the second first was +// dated in may not date another representation. +func expectChangedUndated(t *testing.T, replica string, rec *httptest.ResponseRecorder, first *httptest.ResponseRecorder) { + t.Helper() + if rec.Code != http.StatusOK || rec.Body.String() == first.Body.String() { + t.Fatalf("replica %s after upstream changed: status %d; want 200 with the new body", replica, rec.Code) + } + if rec.Header().Get("ETag") == first.Header().Get("ETag") { + t.Errorf("replica %s: the new body has the old ETag", replica) + } + if lm := rec.Header().Get("Last-Modified"); lm != "" { + t.Errorf("replica %s: Last-Modified %q in the second the old body was dated, want none", replica, lm) + } +} + +// expectSharedDate checks that every replica serves the same Last-Modified, +// later than held, and returns it. +func expectSharedDate(t *testing.T, replicas map[string]http.Handler, path, held string) string { + t.Helper() + was, _ := http.ParseTime(held) + shared := "" + for name, handler := range replicas { + lm := serve(handler, path, nil).Header().Get("Last-Modified") + if now, err := http.ParseTime(lm); err != nil || !now.After(was) { + t.Fatalf("replica %s: Last-Modified %q, want later than %q", name, lm, held) + } + if shared != "" && lm != shared { + t.Fatalf("replica %s: Last-Modified %q, another replica serves %q", name, lm, shared) + } + shared = lm + } + return shared +} + +// TestRewrittenMetadataLastModifiedFromBeforeTheFix checks that dates clients +// hold from before dates were shared cannot suppress a body. Those were never +// later than the clock of the process that gave them, which stopped before +// this one started, so no date is given in the second this process started +// and every later one is later than any of them. +func TestRewrittenMetadataLastModifiedFromBeforeTheFix(t *testing.T) { + for _, c := range validatorCases { + t.Run(c.ecosystem, func(t *testing.T) { + u := newValidatorUpstream(t) + a, _, clock := validatorProxyWithDB(t, u, 0) + restartProxy(a, clock) + b := validatorReplica(a, clock) + held := clock.now().Truncate(time.Second).Format(http.TimeFormat) + + for name, handler := range map[string]http.Handler{"A": c.handler(a, u), "B": c.handler(b, u)} { + rec := serve(handler, c.path, http.Header{"If-Modified-Since": {held}}) + if rec.Code != http.StatusOK || rec.Header().Get("Last-Modified") != "" { + t.Errorf("replica %s in its first second, If-Modified-Since %q: status %d, Last-Modified %q; want 200 without a date", + name, held, rec.Code, rec.Header().Get("Last-Modified")) + } + } + + clock.advance(time.Second) + for name, handler := range map[string]http.Handler{"A": c.handler(a, u), "B": c.handler(b, u)} { + rec := serve(handler, c.path, http.Header{"If-Modified-Since": {held}}) + was, _ := http.ParseTime(held) + now, err := http.ParseTime(rec.Header().Get("Last-Modified")) + if rec.Code != http.StatusOK || err != nil || !now.After(was) { + t.Errorf("replica %s a second later, If-Modified-Since %q: status %d, Last-Modified %q; want 200 with a later date", + name, held, rec.Code, rec.Header().Get("Last-Modified")) + } + } + }) + } +} + +// TestRewrittenMetadataLastModifiedWithClockSkew checks that a replica whose +// clock is behind gives no date ahead of its own clock, neither for a new +// document nor for one another replica dated. +func TestRewrittenMetadataLastModifiedWithClockSkew(t *testing.T) { + for _, c := range validatorCases { + t.Run(c.ecosystem, func(t *testing.T) { + u := newValidatorUpstream(t) + a, _, clock := validatorProxyWithDB(t, u, 0) + behind := &testClock{t: clock.now().Add(-2 * time.Second)} + b := validatorReplica(a, behind) + handlerA, handlerB := c.handler(a, u), c.handler(b, u) + + held := serve(handlerA, c.path, nil).Header().Get("Last-Modified") + u.set(npmV("2.0.0"), composerV("2.0.0")) + if rec := serve(handlerB, c.path, http.Header{"If-Modified-Since": {held}}); rec.Code != http.StatusOK || rec.Header().Get("Last-Modified") != "" { + t.Fatalf("lagging replica, new document: status %d, Last-Modified %q; want 200 without a date", rec.Code, rec.Header().Get("Last-Modified")) + } + + clock.advance(time.Second) + dated := serve(handlerA, c.path, nil).Header().Get("Last-Modified") + if dated == "" { + t.Fatal("replica A gave no date a second later") + } + if lm := serve(handlerB, c.path, nil).Header().Get("Last-Modified"); lm != "" { + t.Errorf("lagging replica: Last-Modified %q ahead of its clock at %v, want none", lm, behind.now()) + } + behind.advance(3 * time.Second) + if lm := serve(handlerB, c.path, nil).Header().Get("Last-Modified"); lm != dated { + t.Errorf("lagging replica once its clock caught up: Last-Modified %q, want %q", lm, dated) + } + }) + } +} + +// cooldownUpstream sets documents with one version published long ago and +// one published two hours ago, so a cooldown of a day hides the recent one +// and a cooldown of an hour keeps it. +func cooldownUpstream(t *testing.T) *validatorUpstream { + t.Helper() + recent := time.Now().Add(-2 * time.Hour).UTC().Format(time.RFC3339) + u := newValidatorUpstream(t) + u.set( + `{"name":"left-pad","dist-tags":{"latest":"1.4.0"},"versions":{`+ + `"1.3.0":{"dist":{"tarball":"https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz"}},`+ + `"1.4.0":{"dist":{"tarball":"https://registry.npmjs.org/left-pad/-/left-pad-1.4.0.tgz"}}},`+ + `"time":{"1.3.0":"2016-01-01T00:00:00Z","1.4.0":"`+recent+`"}}`, + `{"packages":{"vendor/pkg":[`+ + `{"version":"1.0.0","time":"2016-01-01T00:00:00Z","dist":{"type":"zip","url":"https://example.com/pkg-1.0.0.zip"}},`+ + `{"version":"1.1.0","time":"`+recent+`","dist":{"type":"zip","url":"https://example.com/pkg-1.1.0.zip"}}]}}`, + ) + return u +} + +// TestRewrittenMetadataETagWithCooldownFollowsKeptVersions checks that with +// cooldown on, the ETag changes when a version leaves cooldown although the +// upstream document stays the same, and comes back when the same versions do. +func TestRewrittenMetadataETagWithCooldownFollowsKeptVersions(t *testing.T) { + for _, c := range validatorCases { + t.Run(c.ecosystem, func(t *testing.T) { + u := cooldownUpstream(t) + proxy := validatorProxy(t, u, time.Hour) + handler := c.handler(proxy, u) + + proxy.Cooldown = &cooldown.Config{Default: "1d"} + etag := serve(handler, c.path, nil).Header().Get("ETag") + + proxy.Cooldown = &cooldown.Config{Default: "1h"} + rec := serve(handler, c.path, http.Header{"If-None-Match": {etag}}) + if rec.Code != http.StatusOK || rec.Header().Get("ETag") == etag { + t.Errorf("after a version left cooldown: status %d, ETag %q; want 200 with a new ETag", rec.Code, rec.Header().Get("ETag")) + } + + proxy.Cooldown = &cooldown.Config{Default: "1d"} + expectNotModified(t, serve(handler, c.path, http.Header{"If-None-Match": {etag}}), etag) + }) + } +} + +// TestRewrittenMetadataETagWithCooldownComesFromInputs checks that with +// cooldown on, the ETag is derived from what fixes the rewrite rather than +// from the rewritten body, when the metadata cache knows the upstream +// digest, and falls back to the body when it does not. +func TestRewrittenMetadataETagWithCooldownComesFromInputs(t *testing.T) { + for _, c := range validatorCases { + for _, cacheMetadata := range []bool{true, false} { + t.Run(fmt.Sprintf("%s/cacheMetadata=%t", c.ecosystem, cacheMetadata), func(t *testing.T) { + u := cooldownUpstream(t) + proxy := validatorProxy(t, u, time.Hour) + proxy.CacheMetadata = cacheMetadata + proxy.Cooldown = &cooldown.Config{Default: "1d"} + handler := c.handler(proxy, u) + + rec := serve(handler, c.path, nil) + bodyETag := metadataETag(rec.Body.Bytes()) + if got := rec.Header().Get("ETag"); (got == bodyETag) == cacheMetadata { + t.Errorf("ETag %q, body hash %q; want them to differ only with a known digest", got, bodyETag) + } + etag := rec.Header().Get("ETag") + expectNotModified(t, serve(handler, c.path, http.Header{"If-None-Match": {etag}}), etag) + }) + } + } +} + +func TestKeptVersionsETag(t *testing.T) { + base := keptVersionsETag("sha256:abc", "http://proxy", "left-pad", []string{"1.0.0", "1.1.0"}) + if got := keptVersionsETag("sha256:abc", "http://proxy", "left-pad", []string{"1.1.0", "1.0.0"}); got != base { + t.Errorf("kept versions in another order: ETag %q, want %q", got, base) + } + for name, etag := range map[string]string{ + "digest": keptVersionsETag("sha256:abd", "http://proxy", "left-pad", []string{"1.0.0", "1.1.0"}), + "proxy URL": keptVersionsETag("sha256:abc", "http://other", "left-pad", []string{"1.0.0", "1.1.0"}), + "name": keptVersionsETag("sha256:abc", "http://proxy", "right-pad", []string{"1.0.0", "1.1.0"}), + "kept versions": keptVersionsETag("sha256:abc", "http://proxy", "left-pad", []string{"1.0.0"}), + "field bounds": keptVersionsETag("sha256:abc", "http://proxy", "left-pad", []string{"1.0.0\x001.1.0"}), + } { + if etag == base { + t.Errorf("a different %s gives the same ETag", name) + } + } + if !strings.HasPrefix(base, `"`) || !strings.HasSuffix(base, `"`) { + t.Errorf("ETag %q is not a quoted strong validator", base) + } +} diff --git a/internal/handler/npm.go b/internal/handler/npm.go index 9ce667b..96ab10e 100644 --- a/internal/handler/npm.go +++ b/internal/handler/npm.go @@ -10,6 +10,7 @@ import ( "net/http" "net/url" "reflect" + "slices" "sort" "strings" "sync" @@ -215,10 +216,8 @@ func (h *NPMHandler) handlePackageMetadata(w http.ResponseWriter, r *http.Reques accept = contentTypeJSON } - if rewritten, ok := h.proxy.storedRewrite("npm", metadataCacheKey, h.proxyURL, metadataCacheKey); ok { - w.Header().Set(headerContentType, contentTypeJSON) - w.WriteHeader(http.StatusOK) - _, _ = w.Write(rewritten) + if stored, ok := h.proxy.storedRewrite("npm", metadataCacheKey, h.proxyURL, metadataCacheKey); ok { + serveRewrittenMetadata(w, r, stored) return } @@ -228,8 +227,8 @@ func (h *NPMHandler) handlePackageMetadata(w http.ResponseWriter, r *http.Reques return } - body, _, err := h.proxy.FetchOrCacheMetadata(r.Context(), "npm", metadataCacheKey, upstreamURL, accept) - if err != nil { + upstream := h.proxy.fetchMetadataDocument(r.Context(), "npm", metadataCacheKey, upstreamURL, accept) + if err := upstream.err; err != nil { if errors.Is(err, ErrUpstreamNotFound) { if versionLookup { JSONError(w, http.StatusNotFound, ErrVersionNotFound.Error()) @@ -243,8 +242,8 @@ func (h *NPMHandler) handlePackageMetadata(w http.ResponseWriter, r *http.Reques return } - rewritten, err := h.proxy.cachedRewrite(r.Context(), "npm", h.proxyURL, metadataCacheKey, body, func(b []byte) ([]byte, error) { - return h.rewriteMetadata(packageName, b) + rewritten, err := h.proxy.cachedRewrite(r.Context(), "npm", h.proxyURL, metadataCacheKey, upstream.body, upstream.digest, func(b []byte) ([]byte, []string, error) { + return h.rewriteMetadataKeeping(packageName, b) }) if err != nil { if r.Context().Err() != nil { @@ -266,13 +265,11 @@ func (h *NPMHandler) handlePackageMetadata(w http.ResponseWriter, r *http.Reques h.proxy.Logger.Warn("failed to rewrite metadata, proxying original", "error", err) w.Header().Set(headerContentType, contentTypeJSON) w.WriteHeader(http.StatusOK) - _, _ = w.Write(body) + _, _ = w.Write(upstream.body) return } - w.Header().Set(headerContentType, contentTypeJSON) - w.WriteHeader(http.StatusOK) - _, _ = w.Write(rewritten) + serveRewrittenMetadata(w, r, rewritten) } // handleFilteredNPMVersionMetadata serves /pkg/ and /pkg/ from the @@ -290,8 +287,9 @@ func (h *NPMHandler) handleFilteredNPMVersionMetadata(w http.ResponseWriter, r * return } - rewritten, err := h.proxy.cachedRewrite(r.Context(), "npm", h.proxyURL, metadataCacheKey, packument, func(b []byte) ([]byte, error) { - return h.rewriteNPMVersionFromPackument(packageName, specifier, b) + rewritten, err := h.proxy.cachedRewrite(r.Context(), "npm", h.proxyURL, metadataCacheKey, packument, "", func(b []byte) ([]byte, []string, error) { + out, err := h.rewriteNPMVersionFromPackument(packageName, specifier, b) + return out, nil, err }) if err != nil { if r.Context().Err() != nil { @@ -316,7 +314,7 @@ func (h *NPMHandler) handleFilteredNPMVersionMetadata(w http.ResponseWriter, r * w.Header().Set(headerContentType, contentTypeJSON) w.WriteHeader(http.StatusOK) - _, _ = w.Write(rewritten) + _, _ = w.Write(rewritten.body) } // npmMetadataCacheKey qualifies routed-package metadata cache entries with the @@ -392,29 +390,36 @@ func (h *NPMHandler) rewriteNPMVersionFromPackument(packageName, specifier strin // map and dist-tags when filtering changed them, are written fresh. Everything // else reaches the client byte for byte as upstream sent it. func (h *NPMHandler) rewriteMetadata(packageName string, body []byte) ([]byte, error) { + out, _, err := h.rewriteMetadataKeeping(packageName, body) + return out, err +} + +// rewriteMetadataKeeping is rewriteMetadata that also reports the versions it +// kept. +func (h *NPMHandler) rewriteMetadataKeeping(packageName string, body []byte) ([]byte, []string, error) { doc, err := indexNPMPackument(body) if err != nil { - return nil, err + return nil, nil, err } versionsRaw := doc.value(doc.versionsAt) if len(versionsRaw) == 0 || versionsRaw[0] != '{' { version, ok, err := lookupJSONString(body, "version") if err != nil { - return nil, err + return nil, nil, err } if ok { if h.proxy.versionDenied("npm", packageName, version) { - return nil, ErrVersionDenied + return nil, nil, ErrVersionDenied } var out bytes.Buffer h.writeNPMVersion(&out, packageName, version, body) - return out.Bytes(), nil + return out.Bytes(), []string{version}, nil } if len(h.proxy.Denylist.Versions(canonicalPackagePURL("npm", packageName))) != 0 { - return nil, errors.New("npm metadata has no versions object") + return nil, nil, errors.New("npm metadata has no versions object") } - return body, nil // No versions to rewrite + return body, nil, nil // No versions to rewrite } // The filters only look at which versions exist, so they run on a map of @@ -425,7 +430,7 @@ func (h *NPMHandler) rewriteMetadata(packageName string, body []byte) ([]byte, e versions[version] = nil return err }); err != nil { - return nil, err + return nil, nil, err } metadata := map[string]any{} timeMap := doc.decodeObject(doc.timeAt, metadata, "time") @@ -438,7 +443,7 @@ func (h *NPMHandler) rewriteMetadata(packageName string, body []byte) ([]byte, e // Untouched members are copied as they are; the three the filters // handled are written from their filtered state, and only when it changed. - return doc.write(func(out *bytes.Buffer, i int) error { + out, err := doc.write(func(out *bytes.Buffer, i int) error { switch { case i == doc.versionsAt: return h.writeNPMVersions(out, packageName, versionsRaw, versions) @@ -456,6 +461,7 @@ func (h *NPMHandler) rewriteMetadata(packageName string, body []byte) ([]byte, e return nil } }) + return out, slices.Collect(maps.Keys(versions)), err } // npmPackument is a packument's top-level members as offsets into its bytes, diff --git a/internal/handler/npm_rewrite_test.go b/internal/handler/npm_rewrite_test.go index 6503ae1..d9a787e 100644 --- a/internal/handler/npm_rewrite_test.go +++ b/internal/handler/npm_rewrite_test.go @@ -247,7 +247,7 @@ func TestNPMStoredRewriteSkippedWhenStale(t *testing.T) { body := []byte(`{}`) proxy.cacheMetadataBlob(t.Context(), "npm", "demo", metadataStoragePath("npm", "demo"), &upstreamMetadata{body: body}) key := rewriteCacheKey("npm", "http://proxy", "demo", body) - if _, err := proxy.rewrites.rewrite(t.Context(), key, body, func(b []byte) ([]byte, error) { return b, nil }); err != nil { + if _, _, err := proxy.rewrites.rewrite(t.Context(), key, body, func(b []byte) ([]byte, error) { return b, nil }); err != nil { t.Fatal(err) } if _, ok := proxy.storedRewrite("npm", "demo", "http://proxy", "demo"); !ok { diff --git a/internal/handler/rewrite_cache.go b/internal/handler/rewrite_cache.go index 23096d6..7348bbe 100644 --- a/internal/handler/rewrite_cache.go +++ b/internal/handler/rewrite_cache.go @@ -3,9 +3,14 @@ package handler import ( "container/list" "context" + "crypto/rand" "crypto/sha256" + "encoding/binary" "encoding/hex" "errors" + "io" + "net/http" + "slices" "strings" "sync" "time" @@ -26,6 +31,11 @@ import ( // are rewritten again rather than served stale. Everything else a rewrite // depends on is fixed for the life of the process: the proxy URL is part of // the key, and the denylist is loaded at startup. +// +// Each entry keeps the ETag of its output, so clients can revalidate their +// copy without the output being hashed again on every request. Dates are not +// the cache's to give: they must agree across replicas, so the database holds +// them (see metadataDate). type rewriteCache struct { maxBytes int64 @@ -37,19 +47,67 @@ type rewriteCache struct { } type rewriteEntry struct { - key string - out []byte + key string + out []byte + etag string } -// inflightRewrite is one rewrite that concurrent callers share. out and err -// are written before done closes and read only after, so the close is the -// handoff. +// inflightRewrite is one rewrite that concurrent callers share. out, etag +// and err are written before done closes and read only after, so the close is +// the handoff. type inflightRewrite struct { done chan struct{} out []byte + etag string err error } +// metadataValidators are what a client revalidates its copy of rewritten +// metadata with. lastModified is zero when there is none to give. +type metadataValidators struct { + etag string + lastModified time.Time +} + +// rewrittenMetadata is a rewritten metadata document with its validators. +type rewrittenMetadata struct { + body []byte + metadataValidators +} + +// metadataETag returns the strong ETag of a rewritten document. +func metadataETag(out []byte) string { + sum := sha256.Sum256(out) + return `"` + hex.EncodeToString(sum[:]) + `"` +} + +// rewriteConfigNonce stands for the configuration a rewrite reads at startup, +// such as the denylist and cooldown rules, in ETags derived from a rewrite's +// inputs. It changes on every start, so no ETag outlives the configuration it +// was derived under. +var rewriteConfigNonce = rand.Text() + +// keptVersionsETag returns the strong ETag of a rewrite from what fixes its +// output: the upstream document's digest, the proxy URL and package name its +// download URLs carry, the versions cooldown and the denylist kept, and the +// startup configuration. Hashing these costs far less than hashing the +// rewritten document. Each field is length-prefixed, so different inputs never +// hash the same bytes. +func keptVersionsETag(digest, proxyURL, name string, kept []string) string { + h := sha256.New() + var length []byte + for _, field := range append([]string{rewriteConfigNonce, digest, proxyURL, name}, slices.Sorted(slices.Values(kept))...) { + length = binary.AppendUvarint(length[:0], uint64(len(field))) + _, _ = h.Write(length) + _, _ = io.WriteString(h, field) + } + return `"` + hex.EncodeToString(h.Sum(nil)) + `"` +} + +// keepingRewrite rewrites a metadata document and reports the versions it +// kept. Together with the document they fix the rewrite's output. +type keepingRewrite func([]byte) ([]byte, []string, error) + // errSharedRewriteAbandoned is what waiters see if the caller running a // shared rewrite panicked out of it. var errSharedRewriteAbandoned = errors.New("shared metadata rewrite did not complete") @@ -81,44 +139,46 @@ func rewriteCacheKeyForDigest(ecosystem, proxyURL, name, digest string) string { return strings.Join([]string{ecosystem, proxyURL, name, digest}, "\x00") } -// get returns the cached rewrite for key, if there is one. -func (c *rewriteCache) get(key string) ([]byte, bool) { +// get returns the cached rewrite for key and its ETag, if there is one. +func (c *rewriteCache) get(key string) ([]byte, metadataValidators, bool) { if c == nil { - return nil, false + return nil, metadataValidators{}, false } c.mu.Lock() defer c.mu.Unlock() el, ok := c.entries[key] if !ok { - return nil, false + return nil, metadataValidators{}, false } c.order.MoveToFront(el) - return el.Value.(*rewriteEntry).out, true + e := el.Value.(*rewriteEntry) + return e.out, metadataValidators{etag: e.etag}, true } -// rewrite returns rewrite(in), from the cache when it can. Callers must treat -// the returned bytes as read-only: a cached result is shared. A caller waiting -// on another's rewrite leaves when ctx ends; the rewrite itself always runs -// to completion, so the result is cached for the next request. -func (c *rewriteCache) rewrite(ctx context.Context, key string, in []byte, rewrite func([]byte) ([]byte, error)) ([]byte, error) { +// rewrite returns rewrite(in) and its ETag, from the cache when it can. +// Callers must treat the returned bytes as read-only: a cached result is +// shared. A caller waiting on another's rewrite leaves when ctx ends; the +// rewrite itself always runs to completion, so the result is cached for the +// next request. +func (c *rewriteCache) rewrite(ctx context.Context, key string, in []byte, rewrite func([]byte) ([]byte, error)) (out []byte, v metadataValidators, err error) { if c == nil { - return rewrite(in) + return rewriteWithETag(in, rewrite) } c.mu.Lock() if el, ok := c.entries[key]; ok { c.order.MoveToFront(el) - out := el.Value.(*rewriteEntry).out + e := el.Value.(*rewriteEntry) c.mu.Unlock() - return out, nil + return e.out, metadataValidators{etag: e.etag}, nil } if f, ok := c.inFlight[key]; ok { c.mu.Unlock() select { case <-ctx.Done(): - return nil, ctx.Err() + return nil, metadataValidators{}, ctx.Err() case <-f.done: - return f.out, f.err + return f.out, metadataValidators{etag: f.etag}, f.err } } f := &inflightRewrite{done: make(chan struct{}), err: errSharedRewriteAbandoned} @@ -129,24 +189,35 @@ func (c *rewriteCache) rewrite(ctx context.Context, key string, in []byte, rewri c.mu.Lock() delete(c.inFlight, key) if f.err == nil { - c.add(key, f.out) + c.add(&rewriteEntry{key: key, out: f.out, etag: f.etag}) } c.mu.Unlock() close(f.done) }() - f.out, f.err = rewrite(in) - return f.out, f.err + var fv metadataValidators + f.out, fv, f.err = rewriteWithETag(in, rewrite) + f.etag = fv.etag + return f.out, fv, f.err +} + +// rewriteWithETag runs rewrite and computes the ETag of what it returns. +func rewriteWithETag(in []byte, rewrite func([]byte) ([]byte, error)) ([]byte, metadataValidators, error) { + out, err := rewrite(in) + if err != nil { + return nil, metadataValidators{}, err + } + return out, metadataValidators{etag: metadataETag(out)}, nil } -// add stores out under key and evicts least recently used entries until the -// cache is back under its limit. Output larger than the whole cache is not -// stored. Called with mu held. -func (c *rewriteCache) add(key string, out []byte) { - n := int64(len(out)) +// add stores e and evicts least recently used entries until the cache is +// back under its limit. Output larger than the whole cache is not stored. +// Called with mu held. +func (c *rewriteCache) add(e *rewriteEntry) { + n := int64(len(e.out)) if n > c.maxBytes { return } - c.entries[key] = c.order.PushFront(&rewriteEntry{key: key, out: out}) + c.entries[e.key] = c.order.PushFront(e) c.size += n for c.size > c.maxBytes { oldest := c.order.Back() @@ -168,14 +239,41 @@ func (c *rewriteCache) len() int { } // cachedRewrite rewrites a metadata document through the proxy's rewrite -// cache. Cooldown filtering depends on the current time, so with cooldown on -// a cached rewrite could keep hiding a version past its cooldown; those -// rewrites always run. -func (p *Proxy) cachedRewrite(ctx context.Context, ecosystem, proxyURL, name string, in []byte, rewrite func([]byte) ([]byte, error)) ([]byte, error) { - if p.rewrites == nil || (p.Cooldown != nil && p.Cooldown.Enabled()) { - return rewrite(in) +// cache and returns it with its validators. digest identifies in when the +// metadata cache stored it, and is empty otherwise. Cooldown filtering depends +// on the current time, so with cooldown on a cached rewrite could keep hiding +// a version past its cooldown; those rewrites always run, with no +// Last-Modified, and when digest is known their ETag comes from their inputs +// rather than from hashing each output again. Their ETag also changes with +// every restart, so replicas would never agree on a date for them. +func (p *Proxy) cachedRewrite(ctx context.Context, ecosystem, proxyURL, name string, in []byte, digest string, rewrite keepingRewrite) (rewrittenMetadata, error) { + if p.cooldownEnabled() && digest != "" { + out, kept, err := rewrite(in) + if err != nil { + return rewrittenMetadata{}, err + } + return rewrittenMetadata{out, metadataValidators{etag: keptVersionsETag(digest, proxyURL, name, kept)}}, nil + } + rewriteOnly := func(b []byte) ([]byte, error) { + out, _, err := rewrite(b) + return out, err + } + var out []byte + var v metadataValidators + var err error + if p.rewrites == nil || p.cooldownEnabled() { + out, v, err = rewriteWithETag(in, rewriteOnly) + } else { + out, v, err = p.rewrites.rewrite(ctx, rewriteCacheKey(ecosystem, proxyURL, name, in), in, rewriteOnly) + } + if err == nil && !p.cooldownEnabled() { + v.lastModified = p.metadataDate(ecosystem, proxyURL, name, v.etag) } - return p.rewrites.rewrite(ctx, rewriteCacheKey(ecosystem, proxyURL, name, in), in, rewrite) + return rewrittenMetadata{out, v}, err +} + +func (p *Proxy) cooldownEnabled() bool { + return p.Cooldown != nil && p.Cooldown.Enabled() } // storedRewrite returns the cached rewrite of the metadata stored for @@ -184,25 +282,56 @@ func (p *Proxy) cachedRewrite(ctx context.Context, ecosystem, proxyURL, name str // document, so a repeated request for a large packument costs a database // lookup instead of reading and hashing the whole document again. When it // reports false the caller takes the usual fetch and rewrite path. -func (p *Proxy) storedRewrite(ecosystem, cacheKey, proxyURL, name string) ([]byte, bool) { - if p.rewrites == nil || (p.Cooldown != nil && p.Cooldown.Enabled()) { - return nil, false +func (p *Proxy) storedRewrite(ecosystem, cacheKey, proxyURL, name string) (rewrittenMetadata, bool) { + if p.rewrites == nil || p.cooldownEnabled() { + return rewrittenMetadata{}, false } if !p.CacheMetadata || p.DB == nil || p.MetadataTTL <= 0 { - return nil, false + return rewrittenMetadata{}, false } entry, err := p.DB.GetMetadataCache(ecosystem, cacheKey) if err != nil || entry == nil || !entry.ContentDigest.Valid || entry.ContentEncoding.String != "" { - return nil, false + return rewrittenMetadata{}, false } if !entry.FetchedAt.Valid || time.Since(entry.FetchedAt.Time) >= p.MetadataTTL { - return nil, false + return rewrittenMetadata{}, false + } + out, v, ok := p.rewrites.get(rewriteCacheKeyForDigest(ecosystem, proxyURL, name, entry.ContentDigest.String)) + if !ok { + return rewrittenMetadata{}, false + } + v.lastModified = p.metadataDate(ecosystem, proxyURL, name, v.etag) + metrics.RecordCacheHit(ecosystem) + return rewrittenMetadata{out, v}, true +} + +// serveRewrittenMetadata writes rewritten metadata with its validators, or +// 304 Not Modified when they show the client's copy is current. +func serveRewrittenMetadata(w http.ResponseWriter, r *http.Request, m rewrittenMetadata) { + w.Header().Set(headerETag, m.etag) + if !m.lastModified.IsZero() { + w.Header().Set(headerLastModified, m.lastModified.UTC().Format(http.TimeFormat)) + } + if notModified(r, m.etag, m.lastModified) { + w.WriteHeader(http.StatusNotModified) + return + } + w.Header().Set(headerContentType, contentTypeJSON) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(m.body) +} + +// notModified reports whether a conditional request's validators match. +// If-Modified-Since only counts when the request has no If-None-Match. +func notModified(r *http.Request, etag string, lastModified time.Time) bool { + if header := r.Header.Get("If-None-Match"); header != "" { + return ifNoneMatchHits(header, etag) } - out, ok := p.rewrites.get(rewriteCacheKeyForDigest(ecosystem, proxyURL, name, entry.ContentDigest.String)) - if ok { - metrics.RecordCacheHit(ecosystem) + if lastModified.IsZero() { + return false } - return out, ok + since, err := http.ParseTime(r.Header.Get("If-Modified-Since")) + return err == nil && !lastModified.Truncate(time.Second).After(since) } // SetMetadataRewriteCacheSize enables the cache of rewritten metadata with diff --git a/internal/handler/rewrite_cache_test.go b/internal/handler/rewrite_cache_test.go index 0edb94f..473f80a 100644 --- a/internal/handler/rewrite_cache_test.go +++ b/internal/handler/rewrite_cache_test.go @@ -31,13 +31,18 @@ func (r *countingRewrite) fn(in []byte) ([]byte, error) { return []byte(strings.ToUpper(string(in))), nil } +func (r *countingRewrite) keeping(in []byte) ([]byte, []string, error) { + out, err := r.fn(in) + return out, nil, err +} + func TestRewriteCache_RepeatServesCachedRewrite(t *testing.T) { c := newRewriteCache(1 << 20) var r countingRewrite key := rewriteCacheKey("npm", "http://proxy", "left-pad", []byte("doc")) for range 3 { - out, err := c.rewrite(context.Background(), key, []byte("doc"), r.fn) + out, _, err := c.rewrite(context.Background(), key, []byte("doc"), r.fn) if err != nil || string(out) != "DOC" { t.Fatalf("rewrite = %q, %v", out, err) } @@ -53,7 +58,7 @@ func TestRewriteCache_NewUpstreamBytesRewriteAgain(t *testing.T) { for _, doc := range []string{"v1", "v2"} { key := rewriteCacheKey("npm", "http://proxy", "left-pad", []byte(doc)) - out, err := c.rewrite(context.Background(), key, []byte(doc), r.fn) + out, _, err := c.rewrite(context.Background(), key, []byte(doc), r.fn) if err != nil || string(out) != strings.ToUpper(doc) { t.Fatalf("rewrite(%s) = %q, %v", doc, out, err) } @@ -75,7 +80,7 @@ func TestRewriteCache_ConcurrentRequestsShareRewrite(t *testing.T) { outs := make(chan string, n) run := func() { - out, err := c.rewrite(context.Background(), key, []byte("doc"), r.fn) + out, _, err := c.rewrite(context.Background(), key, []byte("doc"), r.fn) if err != nil { t.Error(err) } @@ -108,20 +113,20 @@ func TestRewriteCache_WaiterLeavingKeepsRewrite(t *testing.T) { first := make(chan error, 1) go func() { - _, err := c.rewrite(context.Background(), key, []byte("doc"), r.fn) + _, _, err := c.rewrite(context.Background(), key, []byte("doc"), r.fn) first <- err }() <-r.entered ctx, cancel := context.WithCancel(context.Background()) cancel() - if _, err := c.rewrite(ctx, key, []byte("doc"), r.fn); !errors.Is(err, context.Canceled) { + if _, _, err := c.rewrite(ctx, key, []byte("doc"), r.fn); !errors.Is(err, context.Canceled) { t.Errorf("waiter err = %v, want context.Canceled", err) } close(r.release) if err := <-first; err != nil { t.Fatal(err) } - if out, err := c.rewrite(context.Background(), key, []byte("doc"), r.fn); err != nil || string(out) != "DOC" { + if out, _, err := c.rewrite(context.Background(), key, []byte("doc"), r.fn); err != nil || string(out) != "DOC" { t.Errorf("cached rewrite = %q, %v", out, err) } if got := r.calls.Load(); got != 1 { @@ -135,18 +140,18 @@ func TestRewriteCache_EvictsLeastRecentlyUsed(t *testing.T) { keyFor := func(doc string) string { return rewriteCacheKey("npm", "http://proxy", doc, []byte(doc)) } for _, doc := range []string{"aaa", "bbb"} { - _, _ = c.rewrite(context.Background(), keyFor(doc), []byte(doc), r.fn) + _, _, _ = c.rewrite(context.Background(), keyFor(doc), []byte(doc), r.fn) } - _, _ = c.rewrite(context.Background(), keyFor("aaa"), []byte("aaa"), r.fn) // aaa is now the most recent - _, _ = c.rewrite(context.Background(), keyFor("ccc"), []byte("ccc"), r.fn) // evicts bbb + _, _, _ = c.rewrite(context.Background(), keyFor("aaa"), []byte("aaa"), r.fn) // aaa is now the most recent + _, _, _ = c.rewrite(context.Background(), keyFor("ccc"), []byte("ccc"), r.fn) // evicts bbb before := r.calls.Load() - _, _ = c.rewrite(context.Background(), keyFor("aaa"), []byte("aaa"), r.fn) - _, _ = c.rewrite(context.Background(), keyFor("ccc"), []byte("ccc"), r.fn) + _, _, _ = c.rewrite(context.Background(), keyFor("aaa"), []byte("aaa"), r.fn) + _, _, _ = c.rewrite(context.Background(), keyFor("ccc"), []byte("ccc"), r.fn) if got := r.calls.Load() - before; got != 0 { t.Errorf("aaa and ccc rewritten %d times, want both still cached", got) } - _, _ = c.rewrite(context.Background(), keyFor("bbb"), []byte("bbb"), r.fn) + _, _, _ = c.rewrite(context.Background(), keyFor("bbb"), []byte("bbb"), r.fn) if got := r.calls.Load() - before; got != 1 { t.Errorf("bbb should have been evicted and rewritten") } @@ -157,7 +162,7 @@ func TestRewriteCache_OutputLargerThanCacheIsNotStored(t *testing.T) { var r countingRewrite key := rewriteCacheKey("npm", "http://proxy", "big", []byte("big")) - _, _ = c.rewrite(context.Background(), key, []byte("big"), r.fn) + _, _, _ = c.rewrite(context.Background(), key, []byte("big"), r.fn) if c.len() != 0 { t.Errorf("cached %d entries, want 0", c.len()) } @@ -170,7 +175,7 @@ func TestRewriteCache_ErrorsAreNotCached(t *testing.T) { key := rewriteCacheKey("npm", "http://proxy", "broken", []byte("doc")) for range 2 { - if _, err := c.rewrite(context.Background(), key, []byte("doc"), failing); err == nil { + if _, _, err := c.rewrite(context.Background(), key, []byte("doc"), failing); err == nil { t.Fatal("expected the rewrite error") } } @@ -185,7 +190,7 @@ func TestCachedRewrite_DisabledRewritesEveryTime(t *testing.T) { var r countingRewrite for range 2 { - _, _ = proxy.cachedRewrite(context.Background(), "npm", "http://proxy", "left-pad", []byte("doc"), r.fn) + _, _ = proxy.cachedRewrite(context.Background(), "npm", "http://proxy", "left-pad", []byte("doc"), "", r.keeping) } if got := r.calls.Load(); got != 2 { t.Errorf("rewrites = %d, want 2", got) @@ -202,7 +207,7 @@ func TestCachedRewrite_CooldownBypassesCache(t *testing.T) { var r countingRewrite for range 2 { - _, _ = proxy.cachedRewrite(context.Background(), "npm", "http://proxy", "left-pad", []byte("doc"), r.fn) + _, _ = proxy.cachedRewrite(context.Background(), "npm", "http://proxy", "left-pad", []byte("doc"), "", r.keeping) } if got := r.calls.Load(); got != 2 { t.Errorf("rewrites = %d, want 2", got)