From 20855cae210505557be584d72435d3ec14893575 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sat, 10 Oct 2026 23:51:55 +0200 Subject: [PATCH 1/2] Enable storage retention for gem Gems are stored under the name from the gem filename, and purl keeps gem names as they are, so package rules use the default mapping and match the name as published, case included. Packages that git-pkgs wrote as "rubygems" fall under the gem rules too. The filename parser splits platforms that end in a number (universal-darwin-22) at the wrong hyphen and files those gems under a package like sorbet-static-0.5.11002-universal-darwin, so package rules don't reach them; the docs row says so until the parser is fixed. The mirror can't fetch gems today (it asks the resolver for "rubygems", see #415), so there's no mirror test. Refs #306 --- docs/configuration.md | 2 +- internal/handler/gem.go | 12 ++ internal/server/retention_gem_test.go | 230 ++++++++++++++++++++++++++ 3 files changed, 243 insertions(+), 1 deletion(-) create mode 100644 internal/server/retention_gem_test.go diff --git a/docs/configuration.md b/docs/configuration.md index 41ba4d0d..06e0b7c0 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -95,7 +95,7 @@ A package rule wins over an ecosystem rule, which wins over `default`. Package r | `conda` | no | - | | `cran` | no | - | | `deb` | no | - | -| `gem` | no | - | +| `gem` | yes | `pkg:gem/rails`, the gem name as published, case included. Package rules don't reach gem files whose platform ends in a number (`universal-darwin-22`, `universal-java-17`) yet, which the proxy files under a wrong package name; the gem rule and `default` still cover them. | | `golang` | yes | `pkg:golang/github.com/stretchr/testify`, the module path as in `go.mod`; case doesn't matter. A major version like `/v2` (or `.v3` on gopkg.in) is a module of its own and needs its own rule. The `!`-escaped form from proxy URLs and the module cache (`github.com/!azure/...`) is refused. | | `helm` | no | - | | `hex` | no | - | diff --git a/internal/handler/gem.go b/internal/handler/gem.go index efe964b6..93e9989b 100644 --- a/internal/handler/gem.go +++ b/internal/handler/gem.go @@ -7,12 +7,24 @@ import ( "net/http" "strings" "time" + + "github.com/git-pkgs/proxy/internal/retention" ) const ( gemUpstream = "https://rubygems.org" ) +// Gems are stored under pkg:gem/{name} with the name from the gem filename, +// and purl keeps gem names as they are, so package rules use the default +// key mapping. parseGemFilename splits at the last hyphen before a digit, +// so a platform like universal-darwin-22 lands under a wrong package name +// that package rules don't reach. The spec also claims the "rubygems" +// spelling git-pkgs writes into packages.ecosystem. +func init() { + retention.Default.Register(retention.Spec{Key: "gem", CanonicalPackage: retention.DefaultCanonical("gem")}) +} + // GemHandler handles RubyGems registry protocol requests. type GemHandler struct { proxy *Proxy diff --git a/internal/server/retention_gem_test.go b/internal/server/retention_gem_test.go new file mode 100644 index 00000000..ebbc029a --- /dev/null +++ b/internal/server/retention_gem_test.go @@ -0,0 +1,230 @@ +package server + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/git-pkgs/proxy/internal/config" + "github.com/git-pkgs/proxy/internal/database" + "github.com/git-pkgs/proxy/internal/handler" + "github.com/git-pkgs/proxy/internal/retention" + "github.com/git-pkgs/registries/fetch" +) + +// gemRetentionUpstream serves gem files the way rubygems.org does, +// counting downloads. +func gemRetentionUpstream(t *testing.T) (*httptest.Server, *atomic.Int64) { + t.Helper() + var downloads atomic.Int64 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/gems/rack-3.0.8.gem", "/gems/RedCloth-4.3.3.gem": + downloads.Add(1) + _, _ = io.WriteString(w, "gem bytes") + default: + http.NotFound(w, r) + } + })) + t.Cleanup(srv.Close) + return srv, &downloads +} + +func newGemRetentionProxy(t *testing.T, upstream *httptest.Server) (*httptest.Server, *database.DB) { + t.Helper() + db, store := setupEvictionTest(t) + // The upstream is a plain-HTTP loopback server; the default fetcher + // refuses loopback, a stock client reaches it. + fetcher := fetch.NewFetcher(fetch.WithHTTPClient(upstream.Client()), fetch.WithMaxRetries(0)) + t.Cleanup(func() { _ = fetcher.Close() }) + proxy := handler.NewProxy(db, store, fetcher, fetch.NewResolver(), discardLogger()) + proxy.HTTPClient = upstream.Client() + h := handler.NewGemHandlerWithUpstream(proxy, "http://proxy.test", upstream.URL) + srv := httptest.NewServer(h.Routes()) + t.Cleanup(srv.Close) + return srv, db +} + +func gemGet(t *testing.T, url string) { + t.Helper() + resp, err := http.Get(url) + if err != nil { + t.Fatalf("GET %s: %v", url, err) + } + body, _ := io.ReadAll(resp.Body) + _ = resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("GET %s = %d %s, want 200", url, resp.StatusCode, body) + } +} + +// gemArtifact returns the record of the gem file named filename. +func gemArtifact(t *testing.T, db *database.DB, filename string) database.Artifact { + t.Helper() + var art database.Artifact + if err := db.Get(&art, db.Rebind(`SELECT * FROM artifacts WHERE filename = ?`), filename); err != nil { + t.Fatalf("reading artifact %s: %v", filename, err) + } + return art +} + +func gemBackdate(t *testing.T, db *database.DB, filename string, fetched, accessed time.Time) { + t.Helper() + query := db.Rebind(`UPDATE artifacts SET fetched_at = ?, last_accessed_at = ? WHERE filename = ?`) + if _, err := db.Exec(query, fetched, accessed, filename); err != nil { + t.Fatalf("backdating %s: %v", filename, err) + } +} + +func TestGemRegistersForRetention(t *testing.T) { + if _, ok := retention.Default.Lookup("gem"); !ok { + t.Fatal("gem is not registered for retention") + } + for _, value := range []string{"gem", "rubygems"} { + if key, ok := retention.Default.KeyForDBEcosystem(value); !ok || key != "gem" { + t.Errorf("KeyForDBEcosystem(%s) = %q, %v, want gem, true", value, key, ok) + } + } +} + +// A gem served from the cache stays while it is used, is evicted once its +// rule runs out, comes back from upstream on the next download, and stays +// after that even with the evicted copy's old access time on the record. +func TestGemRetentionEvictsAndRefetches(t *testing.T) { + upstream, downloads := gemRetentionUpstream(t) + srv, db := newGemRetentionProxy(t, upstream) + const gem = "rack-3.0.8.gem" + + gemGet(t, srv.URL+"/gems/rack-3.0.8.gem") + gemGet(t, srv.URL+"/gems/rack-3.0.8.gem") + if got := downloads.Load(); got != 1 { + t.Fatalf("upstream downloads = %d, want 1", got) + } + if !gemArtifact(t, db, gem).LastAccessedAt.Valid { + t.Fatal("cache hit did not record an access") + } + + rules := retention.Rules{Ecosystems: map[string]time.Duration{"gem": retentionDay}} + now := time.Now() + sweepRetention(context.Background(), db, discardLogger(), retention.Default, rules, now) + if !gemArtifact(t, db, gem).StoragePath.Valid { + t.Fatal("gem used today was evicted") + } + + old := now.Add(-2 * retentionDay) + gemBackdate(t, db, gem, old, old) + sweepRetention(context.Background(), db, discardLogger(), retention.Default, rules, now) + if gemArtifact(t, db, gem).StoragePath.Valid { + t.Fatal("gem unused for two days kept under a one-day rule") + } + + gemGet(t, srv.URL+"/gems/rack-3.0.8.gem") + if got := downloads.Load(); got != 2 { + t.Errorf("upstream downloads = %d, want 2 after the eviction", got) + } + art := gemArtifact(t, db, gem) + gemBackdate(t, db, gem, art.FetchedAt.Time, now.Add(-30*retentionDay)) + sweepRetention(context.Background(), db, discardLogger(), retention.Default, rules, time.Now()) + if !gemArtifact(t, db, gem).StoragePath.Valid { + t.Error("refetched gem evicted again by the next sweep") + } +} + +// A key with a namespace parses as a gem PURL but can never match a +// stored gem, so it is refused at validation. A plain key is accepted. +func TestGemRetentionRefusesNamespacedKeys(t *testing.T) { + cfg := config.Default() + cfg.Storage.Retention.Packages = map[string]string{"pkg:gem/rack": "0"} + if _, err := cfg.RetentionRules(retention.Default); err != nil { + t.Fatalf("RetentionRules(pkg:gem/rack): %v", err) + } + cfg.Storage.Retention.Packages = map[string]string{"pkg:gem/rails/rack": "0"} + _, err := cfg.RetentionRules(retention.Default) + if err == nil || !strings.Contains(err.Error(), "does not match how gem packages are stored") { + t.Errorf("RetentionRules(pkg:gem/rails/rack) error = %v, want the key refused", err) + } +} + +// A package rule reaches a gem under its published name, case included; +// a rule with the name in another case covers nothing. +func TestGemRetentionPackageRules(t *testing.T) { + upstream, _ := gemRetentionUpstream(t) + srv, db := newGemRetentionProxy(t, upstream) + const redcloth, rack = "RedCloth-4.3.3.gem", "rack-3.0.8.gem" + + tests := []struct { + key string + keepRedCloth bool + }{ + {"pkg:gem/RedCloth", true}, + {"pkg:gem/redcloth", false}, + } + for _, tt := range tests { + gemGet(t, srv.URL+"/gems/RedCloth-4.3.3.gem") + gemGet(t, srv.URL+"/gems/rack-3.0.8.gem") + old := time.Now().Add(-30 * retentionDay) + gemBackdate(t, db, redcloth, old, old) + gemBackdate(t, db, rack, old, old) + + cfg := config.Default() + cfg.Storage.Retention.Ecosystems = map[string]string{"gem": "1d"} + cfg.Storage.Retention.Packages = map[string]string{tt.key: "0"} + rules, err := cfg.RetentionRules(retention.Default) + if err != nil { + t.Fatalf("RetentionRules(%s): %v", tt.key, err) + } + sweepRetention(context.Background(), db, discardLogger(), retention.Default, rules, time.Now()) + + if got := gemArtifact(t, db, redcloth).StoragePath.Valid; got != tt.keepRedCloth { + t.Errorf("with %s: RedCloth cached = %v, want %v", tt.key, got, tt.keepRedCloth) + } + if gemArtifact(t, db, rack).StoragePath.Valid { + t.Errorf("with %s: rack kept past the gem rule", tt.key) + } + } +} + +// git-pkgs writes gem packages with ecosystem "rubygems", and the record +// keeps the ecosystem of whoever wrote it first. A gem the proxy then caches +// under such a record still falls under the gem rule and its package rules. +func TestGemRetentionCoversRubygemsSpelling(t *testing.T) { + upstream, _ := gemRetentionUpstream(t) + srv, db := newGemRetentionProxy(t, upstream) + if err := db.UpsertPackage(&database.Package{PURL: "pkg:gem/rack", Ecosystem: "rubygems", Name: "rack"}); err != nil { + t.Fatalf("UpsertPackage: %v", err) + } + if err := db.UpsertPackage(&database.Package{PURL: "pkg:gem/RedCloth", Ecosystem: "rubygems", Name: "RedCloth"}); err != nil { + t.Fatalf("UpsertPackage: %v", err) + } + const redcloth, rack = "RedCloth-4.3.3.gem", "rack-3.0.8.gem" + gemGet(t, srv.URL+"/gems/rack-3.0.8.gem") + gemGet(t, srv.URL+"/gems/RedCloth-4.3.3.gem") + var ecosystem string + if err := db.Get(&ecosystem, db.Rebind(`SELECT ecosystem FROM packages WHERE purl = ?`), "pkg:gem/rack"); err != nil || ecosystem != "rubygems" { + t.Fatalf("rack package ecosystem = %q, %v, want rubygems", ecosystem, err) + } + + old := time.Now().Add(-30 * retentionDay) + gemBackdate(t, db, redcloth, old, old) + gemBackdate(t, db, rack, old, old) + cfg := config.Default() + cfg.Storage.Retention.Ecosystems = map[string]string{"gem": "1d"} + cfg.Storage.Retention.Packages = map[string]string{"pkg:gem/RedCloth": "0"} + rules, err := cfg.RetentionRules(retention.Default) + if err != nil { + t.Fatalf("RetentionRules: %v", err) + } + sweepRetention(context.Background(), db, discardLogger(), retention.Default, rules, time.Now()) + + if gemArtifact(t, db, rack).StoragePath.Valid { + t.Error("rack under a rubygems record kept past the gem rule") + } + if !gemArtifact(t, db, redcloth).StoragePath.Valid { + t.Error("RedCloth under a rubygems record evicted despite its never-evict rule") + } +} From 680a2daba168ae8acfe6df5c5219c2b01e1bb1ec Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Mon, 12 Oct 2026 19:55:10 +0200 Subject: [PATCH 2/2] Fix gem filenames whose platform ends in a number parseGemFilename split at the last hyphen before a digit, so sorbet-static-0.5.11002-universal-darwin-22.gem was stored as package sorbet-static-0.5.11002-universal-darwin. It now splits at the first one, which only misreads gem names with a - segment. The docs caveat is gone. Refs #306 --- docs/configuration.md | 2 +- internal/handler/gem.go | 20 ++++++++++---------- internal/handler/gem_test.go | 6 ++++++ 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 06e0b7c0..36f4e791 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -95,7 +95,7 @@ A package rule wins over an ecosystem rule, which wins over `default`. Package r | `conda` | no | - | | `cran` | no | - | | `deb` | no | - | -| `gem` | yes | `pkg:gem/rails`, the gem name as published, case included. Package rules don't reach gem files whose platform ends in a number (`universal-darwin-22`, `universal-java-17`) yet, which the proxy files under a wrong package name; the gem rule and `default` still cover them. | +| `gem` | yes | `pkg:gem/rails`, the gem name as published, case included. | | `golang` | yes | `pkg:golang/github.com/stretchr/testify`, the module path as in `go.mod`; case doesn't matter. A major version like `/v2` (or `.v3` on gopkg.in) is a module of its own and needs its own rule. The `!`-escaped form from proxy URLs and the module cache (`github.com/!azure/...`) is refused. | | `helm` | no | - | | `hex` | no | - | diff --git a/internal/handler/gem.go b/internal/handler/gem.go index 93e9989b..0657b6b1 100644 --- a/internal/handler/gem.go +++ b/internal/handler/gem.go @@ -17,10 +17,8 @@ const ( // Gems are stored under pkg:gem/{name} with the name from the gem filename, // and purl keeps gem names as they are, so package rules use the default -// key mapping. parseGemFilename splits at the last hyphen before a digit, -// so a platform like universal-darwin-22 lands under a wrong package name -// that package rules don't reach. The spec also claims the "rubygems" -// spelling git-pkgs writes into packages.ecosystem. +// key mapping. The spec also claims the "rubygems" spelling git-pkgs writes +// into packages.ecosystem. func init() { retention.Default.Register(retention.Spec{Key: "gem", CanonicalPackage: retention.DefaultCanonical("gem")}) } @@ -89,14 +87,16 @@ func (h *GemHandler) handleDownload(w http.ResponseWriter, r *http.Request) { // parseGemFilename extracts name and version from a gem filename. // e.g., "rails-7.1.0.gem" -> ("rails", "7.1.0") // e.g., "aws-sdk-s3-1.142.0.gem" -> ("aws-sdk-s3", "1.142.0") +// e.g., "sorbet-static-0.5.11002-universal-darwin-22.gem" -> ("sorbet-static", "0.5.11002-universal-darwin-22") +// +// The version starts at the first hyphen followed by a digit. Platforms can +// end in a number (universal-java-17, arm64-darwin-23), so splitting at the +// last one would cut the platform; only gem names with a "-" segment +// are misread this way, and those are rare. func (h *GemHandler) parseGemFilename(filename string) (name, version string) { - // Remove .gem extension base := strings.TrimSuffix(filename, ".gem") - - // Find the last hyphen followed by a version number - // Version starts with a digit - for i := len(base) - 1; i >= 0; i-- { - if base[i] == '-' && i+1 < len(base) && base[i+1] >= '0' && base[i+1] <= '9' { + for i := 0; i+1 < len(base); i++ { + if base[i] == '-' && base[i+1] >= '0' && base[i+1] <= '9' { return base[:i], base[i+1:] } } diff --git a/internal/handler/gem_test.go b/internal/handler/gem_test.go index 7d90946a..ce63deb5 100644 --- a/internal/handler/gem_test.go +++ b/internal/handler/gem_test.go @@ -25,6 +25,12 @@ func TestGemParseFilename(t *testing.T) { {"aws-sdk-s3-1.142.0.gem", "aws-sdk-s3", "1.142.0"}, {"nokogiri-1.15.4-x86_64-linux.gem", "nokogiri", "1.15.4-x86_64-linux"}, {"activerecord-7.0.8.gem", "activerecord", "7.0.8"}, + {"sorbet-static-0.5.11002-universal-darwin-22.gem", "sorbet-static", "0.5.11002-universal-darwin-22"}, + {"jruby-openssl-0.14.2-universal-java-17.gem", "jruby-openssl", "0.14.2-universal-java-17"}, + {"google-protobuf-3.25.3-arm64-darwin-23.gem", "google-protobuf", "3.25.3-arm64-darwin-23"}, + {"nokogiri-1.16.0-x64-mingw-ucrt.gem", "nokogiri", "1.16.0-x64-mingw-ucrt"}, + {"rake-13.1.0.pre.1.gem", "rake", "13.1.0.pre.1"}, + {"no-version.gem", "", ""}, {"invalid", "", ""}, }