From 611feb003adb611813a1944f73764f28641ba4f5 Mon Sep 17 00:00:00 2001 From: Milas Bowman Date: Wed, 4 Nov 2026 18:29:37 -0500 Subject: [PATCH 1/2] fix(debian): route packages by filename rather than `/pool` prefix --- docs/architecture.md | 9 +++ docs/configuration.md | 4 ++ internal/handler/debian.go | 28 +++++++- internal/handler/debian_test.go | 112 ++++++++++++++++++++++++++++++++ 4 files changed, 150 insertions(+), 3 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index d8b294c5..cbd6dab9 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -304,6 +304,11 @@ HTTP protocol handlers for each registry type. - `handleIndex()` - Proxy sparse index - `handleDownload()` - Serve cached crate +**DebianHandler:** +- Serves `/debian` and `/debian/{name}` mirrors of APT archives (`upstream.debian_repositories`) +- Routes `.deb` packages to the artifact cache, source-package files directly to upstream, `dists/…` to the metadata cache, and proxies all other paths +- Keys the artifact cache on name, version, and architecture, scoped by repository for named archives + **SwiftHandler:** - Proxies the Swift Package Registry v1 read endpoints - Rewrites release URLs and caches source archives @@ -429,6 +434,10 @@ An object a record stops pointing at, because a refetch replaced it or its entry - Set `cache_metadata: true` or use the mirror command to enable metadata caching for offline use via the `metadata_cache` table - OCI manifests and tag lists are exceptions: they are cached automatically so previously fetched images remain pullable and tag resolution works when the registry or token service is unavailable +**Why are Debian source-package files streamed instead of cached?** +- Source-package files (`.dsc`, source tarballs, `.buildinfo`, `.changes`) can be re-uploaded or re-signed in place under the same path, whereas the artifact cache keys entries by version and filename without an expected digest from the `Sources` index +- Streaming them avoids serving stale bytes when an upstream file is replaced or caching them under the `dists/` metadata TTL in flat repositories + **Why stream artifacts?** - Memory efficient - don't load large files into RAM - Better latency - start sending while still receiving diff --git a/docs/configuration.md b/docs/configuration.md index 63cc175d..05914ebb 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -805,6 +805,10 @@ and `dists` are refused at config load for that reason, and other root paths an archive may serve (`indices`, `project`, `doc`, `tools`) are not, so avoid those names unless the shadowing is intended. +Binary packages (`.deb`) are cached; source-package files (`.dsc`, source +tarballs, `.buildinfo`, `.changes`) are streamed directly from upstream because +their contents can be replaced in place under the same path. + This field has no environment variable, as with the other named upstream maps. `PROXY_UPSTREAM_DEBIAN` still sets `upstream.debian`. diff --git a/internal/handler/debian.go b/internal/handler/debian.go index b2419f62..87dbd650 100644 --- a/internal/handler/debian.go +++ b/internal/handler/debian.go @@ -108,11 +108,13 @@ func (h *DebianHandler) Routes() http.Handler { } } - // Route based on path type + // Route by filename rather than path prefix because flat repositories + // place pools under dists//pool/.... switch { - case strings.HasPrefix(rest, "pool/"): - // Package downloads - cache these + case isDebPackageFile(rest): h.handlePackageDownload(w, r, repository, upstreamURL, rest) + case isSourcePackageFile(rest): + h.proxyFile(w, r, upstreamURL, rest) case strings.HasPrefix(rest, "dists/"): // Repository metadata - served through the metadata cache h.handleMetadata(w, r, repository, upstreamURL, rest) @@ -123,6 +125,26 @@ func (h *DebianHandler) Routes() http.Handler { })) } +func isDebPackageFile(path string) bool { + return strings.HasSuffix(path, ".deb") +} + +// isSourcePackageFile reports whether path is a Debian source-package artifact. +// Source files can be re-uploaded or re-signed in place under the same filename, +// so they must be streamed rather than cached without digest verification. +func isSourcePackageFile(path string) bool { + switch { + case strings.HasSuffix(path, ".dsc"), + strings.HasSuffix(path, ".buildinfo"), + strings.HasSuffix(path, ".changes"): + return true + } + // Match Debian source tarballs without matching suite tarballs (e.g. installer netboot.tar.gz). + return strings.Contains(path, ".orig.tar.") || + strings.Contains(path, ".orig-") || + strings.Contains(path, ".debian.tar.") +} + // repositoryNotFound answers a request addressed to a repository that is not // configured, naming the ones that are so a misspelling is self-evident. func (h *DebianHandler) repositoryNotFound(w http.ResponseWriter, name string) { diff --git a/internal/handler/debian_test.go b/internal/handler/debian_test.go index f31202fa..848f9e24 100644 --- a/internal/handler/debian_test.go +++ b/internal/handler/debian_test.go @@ -251,6 +251,45 @@ func TestDebianHandler_PackageCacheScopedPerRepository(t *testing.T) { } } +// TestDebianHandler_FlatArchivePackageIsCached verifies that .deb packages +// served under dists//pool/ (as used by flat repositories like Docker's) +// are cached as artifacts rather than treated as uncached/TTL metadata. +func TestDebianHandler_FlatArchivePackageIsCached(t *testing.T) { + const ( + flatPath = "/docker/dists/trixie/pool/stable/amd64/docker-ce_5.0.1-1~debian.13~trixie_amd64.deb" + pkgBody = "docker-ce package bytes" + ) + + var upstreamHits int + + archive := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + upstreamHits++ + _, _ = fmt.Fprint(w, pkgBody) + })) + defer archive.Close() + + proxy, _, _, _ := setupTestProxy(t) + fetcher := fetch.NewFetcher(fetch.WithHTTPClient(archive.Client()), fetch.WithMaxRetries(0)) + proxy.Fetcher = fetcher + t.Cleanup(func() { _ = fetcher.Close() }) + + h := NewDebianHandler(proxy, "http://proxy.example", "https://archive.test", map[string]string{ + "docker": archive.URL, + }) + + for attempt := 1; attempt <= 2; attempt++ { + w := serveDebianRequest(h, flatPath) + if w.Code != http.StatusOK || w.Body.String() != pkgBody { + t.Fatalf("request %d: status = %d, body = %q, want 200 %q", + attempt, w.Code, w.Body.String(), pkgBody) + } + } + + if upstreamHits != 1 { + t.Errorf("upstream fetched %d times for two requests, want 1: the package is not cached", upstreamHits) + } +} + func serveDebianRequest(h *DebianHandler, target string) *httptest.ResponseRecorder { w := httptest.NewRecorder() h.Routes().ServeHTTP(w, httptest.NewRequest(http.MethodGet, target, nil)) @@ -285,3 +324,76 @@ func TestDebRetentionKey(t *testing.T) { t.Error("debRetentionKey(nil) accepted") } } + +func TestDebianHandler_ClassifiesFilesByExtension(t *testing.T) { + tests := []struct { + path string + pkg bool + source bool + why string + }{ + {"pool/main/h/hello/hello_2.10-3_amd64.deb", true, false, "canonical pool"}, + {"dists/trixie/pool/stable/amd64/docker-ce_5.0.1-1~debian.13~trixie_amd64.deb", true, false, "flat archive"}, + {"pool/main/h/hello/hello_2.10-3.udeb", false, false, "udeb not matched by debPackagePattern"}, + {"pool/main/h/hello/hello_2.10-3_amd64.ddeb", false, false, "ddeb not matched by debPackagePattern"}, + {"pool/main/h/hello/hello_2.10-3.dsc", false, true, "source control file"}, + {"pool/main/h/hello/hello_2.10.orig.tar.xz", false, true, "source tarball"}, + {"pool/main/h/hello/hello_2.10.orig-tex.tar.gz", false, true, "component tarball"}, + {"pool/main/h/hello/hello_2.10-3.debian.tar.xz", false, true, "debian tarball"}, + {"pool/main/h/hello/hello_2.10-3_amd64.buildinfo", false, true, "buildinfo"}, + {"pool/main/h/hello/hello_2.10-3_amd64.changes", false, true, "changes"}, + {"dists/trixie/pool/stable/amd64/docker-ce_5.0.1-1.dsc", false, true, "flat archive source file"}, + {"dists/trixie/InRelease", false, false, "metadata"}, + {"dists/trixie/main/binary-amd64/Packages.gz", false, false, "metadata"}, + {"dists/trixie/main/installer-amd64/current/images/netboot/netboot.tar.gz", false, false, "suite tarball"}, + } + for _, tt := range tests { + if got := isDebPackageFile(tt.path); got != tt.pkg { + t.Errorf("isDebPackageFile(%q) = %v, want %v (%s)", tt.path, got, tt.pkg, tt.why) + } + if got := isSourcePackageFile(tt.path); got != tt.source { + t.Errorf("isSourcePackageFile(%q) = %v, want %v (%s)", tt.path, got, tt.source, tt.why) + } + } +} + +func TestDebianHandler_SourcePackageFilesAreNotCached(t *testing.T) { + const dscBody = "hello source control bytes" + + var upstreamHits int + + archive := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + upstreamHits++ + _, _ = fmt.Fprint(w, dscBody) + })) + defer archive.Close() + + proxy, _, store, _ := setupTestProxy(t) + proxy.HTTPClient = archive.Client() + proxy.CacheMetadata = true + proxy.MetadataTTL = time.Hour + + h := NewDebianHandler(proxy, "http://proxy.example", archive.URL, map[string]string{ + "docker": archive.URL, + }) + + for _, target := range []string{ + "/pool/main/h/hello/hello_2.10-3.dsc", + "/docker/dists/trixie/pool/stable/amd64/docker-ce_5.0.1-1.dsc", + } { + for attempt := 1; attempt <= 2; attempt++ { + w := serveDebianRequest(h, target) + if w.Code != http.StatusOK || w.Body.String() != dscBody { + t.Fatalf("%s request %d: status = %d, body = %q, want 200 %q", + target, attempt, w.Code, w.Body.String(), dscBody) + } + } + } + + if upstreamHits != 4 { + t.Errorf("upstream fetched %d times for four requests, want 4: source-package files must not be cached", upstreamHits) + } + if store.stores != 0 { + t.Errorf("storage wrote %d object(s), want 0: a source-package file must not be stored", store.stores) + } +} From 7d84bf768211382ba40995ee0bbd25ab4c0288df Mon Sep 17 00:00:00 2001 From: Milas Bowman Date: Sat, 7 Nov 2026 15:28:26 -0500 Subject: [PATCH 2/2] chore: remove deb source pkg logic / fix root-level pkg caching --- docs/architecture.md | 8 +-- docs/configuration.md | 4 -- internal/handler/debian.go | 29 +-------- internal/handler/debian_test.go | 101 +++++--------------------------- 4 files changed, 19 insertions(+), 123 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index cbd6dab9..19d9c84f 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -306,8 +306,8 @@ HTTP protocol handlers for each registry type. **DebianHandler:** - Serves `/debian` and `/debian/{name}` mirrors of APT archives (`upstream.debian_repositories`) -- Routes `.deb` packages to the artifact cache, source-package files directly to upstream, `dists/…` to the metadata cache, and proxies all other paths -- Keys the artifact cache on name, version, and architecture, scoped by repository for named archives +- Routes `.deb` packages to the artifact cache, `dists/…` to the metadata cache, and proxies all other paths +- Keys the artifact cache on name, version, and filename, scoped by repository for named archives **SwiftHandler:** - Proxies the Swift Package Registry v1 read endpoints @@ -434,10 +434,6 @@ An object a record stops pointing at, because a refetch replaced it or its entry - Set `cache_metadata: true` or use the mirror command to enable metadata caching for offline use via the `metadata_cache` table - OCI manifests and tag lists are exceptions: they are cached automatically so previously fetched images remain pullable and tag resolution works when the registry or token service is unavailable -**Why are Debian source-package files streamed instead of cached?** -- Source-package files (`.dsc`, source tarballs, `.buildinfo`, `.changes`) can be re-uploaded or re-signed in place under the same path, whereas the artifact cache keys entries by version and filename without an expected digest from the `Sources` index -- Streaming them avoids serving stale bytes when an upstream file is replaced or caching them under the `dists/` metadata TTL in flat repositories - **Why stream artifacts?** - Memory efficient - don't load large files into RAM - Better latency - start sending while still receiving diff --git a/docs/configuration.md b/docs/configuration.md index 05914ebb..63cc175d 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -805,10 +805,6 @@ and `dists` are refused at config load for that reason, and other root paths an archive may serve (`indices`, `project`, `doc`, `tools`) are not, so avoid those names unless the shadowing is intended. -Binary packages (`.deb`) are cached; source-package files (`.dsc`, source -tarballs, `.buildinfo`, `.changes`) are streamed directly from upstream because -their contents can be replaced in place under the same path. - This field has no environment variable, as with the other named upstream maps. `PROXY_UPSTREAM_DEBIAN` still sets `upstream.debian`. diff --git a/internal/handler/debian.go b/internal/handler/debian.go index 87dbd650..565f181a 100644 --- a/internal/handler/debian.go +++ b/internal/handler/debian.go @@ -108,13 +108,11 @@ func (h *DebianHandler) Routes() http.Handler { } } - // Route by filename rather than path prefix because flat repositories - // place pools under dists//pool/.... + // Route by filename rather than path prefix because repositories such as + // Docker's nest the pool under dists//. switch { case isDebPackageFile(rest): h.handlePackageDownload(w, r, repository, upstreamURL, rest) - case isSourcePackageFile(rest): - h.proxyFile(w, r, upstreamURL, rest) case strings.HasPrefix(rest, "dists/"): // Repository metadata - served through the metadata cache h.handleMetadata(w, r, repository, upstreamURL, rest) @@ -129,22 +127,6 @@ func isDebPackageFile(path string) bool { return strings.HasSuffix(path, ".deb") } -// isSourcePackageFile reports whether path is a Debian source-package artifact. -// Source files can be re-uploaded or re-signed in place under the same filename, -// so they must be streamed rather than cached without digest verification. -func isSourcePackageFile(path string) bool { - switch { - case strings.HasSuffix(path, ".dsc"), - strings.HasSuffix(path, ".buildinfo"), - strings.HasSuffix(path, ".changes"): - return true - } - // Match Debian source tarballs without matching suite tarballs (e.g. installer netboot.tar.gz). - return strings.Contains(path, ".orig.tar.") || - strings.Contains(path, ".orig-") || - strings.Contains(path, ".debian.tar.") -} - // repositoryNotFound answers a request addressed to a repository that is not // configured, naming the ones that are so a misspelling is self-evident. func (h *DebianHandler) repositoryNotFound(w http.ResponseWriter, name string) { @@ -248,12 +230,7 @@ var debPackagePattern = regexp.MustCompile(`^(.+)_([^_]+)_([^_]+)\.deb$`) // parsePoolPath extracts package info from a pool path. // Example: pool/main/n/nginx/nginx_1.18.0-6_amd64.deb func (h *DebianHandler) parsePoolPath(path string) (name, version, arch string) { - // Get the filename - idx := strings.LastIndex(path, "/") - if idx < 0 { - return "", "", "" - } - filename := path[idx+1:] + filename := path[strings.LastIndex(path, "/")+1:] // Parse the filename matches := debPackagePattern.FindStringSubmatch(filename) diff --git a/internal/handler/debian_test.go b/internal/handler/debian_test.go index 848f9e24..54a72b61 100644 --- a/internal/handler/debian_test.go +++ b/internal/handler/debian_test.go @@ -25,6 +25,7 @@ func TestDebianHandler_parsePoolPath(t *testing.T) { "nmap", "7.91+dfsg1+really7.80+dfsg1-2ubuntu0.1", "amd64", }, {"pool/main/o/openssl/openssl_3.0.2-0ubuntu1.15~build1_amd64.deb", "openssl", "3.0.2-0ubuntu1.15~build1", "amd64"}, + {"nginx_1.18.0-6_amd64.deb", "nginx", "1.18.0-6", "amd64"}, {"invalid/path", "", "", ""}, {"pool/main/n/nginx/nginx.deb", "", "", ""}, }) @@ -251,14 +252,8 @@ func TestDebianHandler_PackageCacheScopedPerRepository(t *testing.T) { } } -// TestDebianHandler_FlatArchivePackageIsCached verifies that .deb packages -// served under dists//pool/ (as used by flat repositories like Docker's) -// are cached as artifacts rather than treated as uncached/TTL metadata. func TestDebianHandler_FlatArchivePackageIsCached(t *testing.T) { - const ( - flatPath = "/docker/dists/trixie/pool/stable/amd64/docker-ce_5.0.1-1~debian.13~trixie_amd64.deb" - pkgBody = "docker-ce package bytes" - ) + const pkgBody = "docker-ce package bytes" var upstreamHits int @@ -277,16 +272,21 @@ func TestDebianHandler_FlatArchivePackageIsCached(t *testing.T) { "docker": archive.URL, }) - for attempt := 1; attempt <= 2; attempt++ { - w := serveDebianRequest(h, flatPath) - if w.Code != http.StatusOK || w.Body.String() != pkgBody { - t.Fatalf("request %d: status = %d, body = %q, want 200 %q", - attempt, w.Code, w.Body.String(), pkgBody) + for _, path := range []string{ + "/docker/dists/trixie/pool/stable/amd64/docker-ce_5.0.1-1~debian.13~trixie_amd64.deb", + "/docker/foo_1.0_amd64.deb", + } { + for attempt := 1; attempt <= 2; attempt++ { + w := serveDebianRequest(h, path) + if w.Code != http.StatusOK || w.Body.String() != pkgBody { + t.Fatalf("%s request %d: status = %d, body = %q, want 200 %q", + path, attempt, w.Code, w.Body.String(), pkgBody) + } } } - if upstreamHits != 1 { - t.Errorf("upstream fetched %d times for two requests, want 1: the package is not cached", upstreamHits) + if upstreamHits != 2 { + t.Errorf("upstream fetched %d times for four requests, want 2: the package is not cached", upstreamHits) } } @@ -324,76 +324,3 @@ func TestDebRetentionKey(t *testing.T) { t.Error("debRetentionKey(nil) accepted") } } - -func TestDebianHandler_ClassifiesFilesByExtension(t *testing.T) { - tests := []struct { - path string - pkg bool - source bool - why string - }{ - {"pool/main/h/hello/hello_2.10-3_amd64.deb", true, false, "canonical pool"}, - {"dists/trixie/pool/stable/amd64/docker-ce_5.0.1-1~debian.13~trixie_amd64.deb", true, false, "flat archive"}, - {"pool/main/h/hello/hello_2.10-3.udeb", false, false, "udeb not matched by debPackagePattern"}, - {"pool/main/h/hello/hello_2.10-3_amd64.ddeb", false, false, "ddeb not matched by debPackagePattern"}, - {"pool/main/h/hello/hello_2.10-3.dsc", false, true, "source control file"}, - {"pool/main/h/hello/hello_2.10.orig.tar.xz", false, true, "source tarball"}, - {"pool/main/h/hello/hello_2.10.orig-tex.tar.gz", false, true, "component tarball"}, - {"pool/main/h/hello/hello_2.10-3.debian.tar.xz", false, true, "debian tarball"}, - {"pool/main/h/hello/hello_2.10-3_amd64.buildinfo", false, true, "buildinfo"}, - {"pool/main/h/hello/hello_2.10-3_amd64.changes", false, true, "changes"}, - {"dists/trixie/pool/stable/amd64/docker-ce_5.0.1-1.dsc", false, true, "flat archive source file"}, - {"dists/trixie/InRelease", false, false, "metadata"}, - {"dists/trixie/main/binary-amd64/Packages.gz", false, false, "metadata"}, - {"dists/trixie/main/installer-amd64/current/images/netboot/netboot.tar.gz", false, false, "suite tarball"}, - } - for _, tt := range tests { - if got := isDebPackageFile(tt.path); got != tt.pkg { - t.Errorf("isDebPackageFile(%q) = %v, want %v (%s)", tt.path, got, tt.pkg, tt.why) - } - if got := isSourcePackageFile(tt.path); got != tt.source { - t.Errorf("isSourcePackageFile(%q) = %v, want %v (%s)", tt.path, got, tt.source, tt.why) - } - } -} - -func TestDebianHandler_SourcePackageFilesAreNotCached(t *testing.T) { - const dscBody = "hello source control bytes" - - var upstreamHits int - - archive := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - upstreamHits++ - _, _ = fmt.Fprint(w, dscBody) - })) - defer archive.Close() - - proxy, _, store, _ := setupTestProxy(t) - proxy.HTTPClient = archive.Client() - proxy.CacheMetadata = true - proxy.MetadataTTL = time.Hour - - h := NewDebianHandler(proxy, "http://proxy.example", archive.URL, map[string]string{ - "docker": archive.URL, - }) - - for _, target := range []string{ - "/pool/main/h/hello/hello_2.10-3.dsc", - "/docker/dists/trixie/pool/stable/amd64/docker-ce_5.0.1-1.dsc", - } { - for attempt := 1; attempt <= 2; attempt++ { - w := serveDebianRequest(h, target) - if w.Code != http.StatusOK || w.Body.String() != dscBody { - t.Fatalf("%s request %d: status = %d, body = %q, want 200 %q", - target, attempt, w.Code, w.Body.String(), dscBody) - } - } - } - - if upstreamHits != 4 { - t.Errorf("upstream fetched %d times for four requests, want 4: source-package files must not be cached", upstreamHits) - } - if store.stores != 0 { - t.Errorf("storage wrote %d object(s), want 0: a source-package file must not be stored", store.stores) - } -}