From f094fe9b81d12d300b8f1d156efc535033c3e4a3 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sun, 18 Oct 2026 13:25:11 +0200 Subject: [PATCH 1/2] Pub downloads without .tar.gz fetched the HTML version page from pub.dev and cached it as the archive, so one request broke that version for everyone after; they now get a 404, and the upstream URL is built from the escaped name and version so an encoded ? or # can't become a query or fragment. --- internal/handler/pub.go | 25 +++++++++++---- internal/handler/pub_test.go | 62 ++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 6 deletions(-) diff --git a/internal/handler/pub.go b/internal/handler/pub.go index 12ff6d92..dae51e2b 100644 --- a/internal/handler/pub.go +++ b/internal/handler/pub.go @@ -5,6 +5,8 @@ import ( "errors" "fmt" "net/http" + "net/url" + "regexp" "strings" "time" @@ -12,10 +14,14 @@ import ( ) const ( - pubUpstream = "https://pub.dev" - pubPathParts = 2 // name + version in path split by /versions/ + pubUpstream = "https://pub.dev" + pubPathParts = 2 // name + version in path split by /versions/ + pubArchiveExt = ".tar.gz" ) +// pubPackageName matches pub package names, which have to be Dart identifiers. +var pubPackageName = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`) + // Packages are stored under pkg:pub/{name} with the name from the download // path, and purl keeps pub names as they are, so package rules use the // default key mapping. @@ -69,19 +75,26 @@ func (h *PubHandler) handleDownload(w http.ResponseWriter, r *http.Request) { return } name := parts[0] - version := strings.TrimSuffix(parts[1], ".tar.gz") + // Only the archive is cached. Without the suffix the upstream path is the + // HTML version page on pub.dev, which must not land under the archive key. + version, ok := strings.CutSuffix(parts[1], pubArchiveExt) + if !ok { + http.Error(w, "not found", http.StatusNotFound) + return + } - if name == "" || version == "" { + if !pubPackageName.MatchString(name) || version == "" || strings.Contains(version, "/") { http.Error(w, "invalid request", http.StatusBadRequest) return } - filename := fmt.Sprintf("%s-%s.tar.gz", name, version) + filename := name + "-" + version + pubArchiveExt h.proxy.Logger.Info("pub download request", "name", name, "version", version) - downloadURL := h.upstreamURL + r.URL.Path + downloadURL := fmt.Sprintf("%s/packages/%s/versions/%s%s", + h.upstreamURL, url.PathEscape(name), url.PathEscape(version), pubArchiveExt) result, err := h.proxy.GetOrFetchArtifactFromURL( r.Context(), "pub", name, version, filename, downloadURL, ) diff --git a/internal/handler/pub_test.go b/internal/handler/pub_test.go index 8a4c0982..f3dd4482 100644 --- a/internal/handler/pub_test.go +++ b/internal/handler/pub_test.go @@ -2,11 +2,16 @@ package handler import ( "encoding/json" + "io" "log/slog" + "net/http" + "net/http/httptest" + "strings" "testing" "time" "github.com/git-pkgs/cooldown" + "github.com/git-pkgs/registries/fetch" ) func TestPubRewriteMetadata(t *testing.T) { @@ -92,3 +97,60 @@ func TestPubRewriteMetadataCooldown(t *testing.T) { t.Errorf("latest version = %v, want 1.0.0", latest["version"]) } } + +// Without the .tar.gz suffix pub.dev answers with the HTML version page, which +// would otherwise be cached under the archive's key. +func TestPubDownloadRequiresArchiveSuffix(t *testing.T) { + proxy, db, _, fetcher := setupTestProxy(t) + fetcher.artifact = &fetch.Artifact{ + Body: io.NopCloser(strings.NewReader("version page")), + ContentType: "text/html", + } + h := NewPubHandlerWithUpstream(proxy, "http://localhost", "https://pub.example.com") + + req := httptest.NewRequest(http.MethodGet, "/packages/http/versions/1.2.0", nil) + w := httptest.NewRecorder() + h.Routes().ServeHTTP(w, req) + + if w.Code != http.StatusNotFound { + t.Errorf("status = %d, want %d", w.Code, http.StatusNotFound) + } + if fetcher.fetchCalled { + t.Errorf("upstream fetched %q", fetcher.fetchedURL) + } + art, err := db.GetArtifact("pkg:pub/http@1.2.0", "http-1.2.0.tar.gz") + if err != nil { + t.Fatalf("GetArtifact: %v", err) + } + if art != nil { + t.Errorf("artifact cached from %q", art.UpstreamURL) + } +} + +func TestPubDownloadRejectsEncodedQueryInName(t *testing.T) { + proxy, _, _, fetcher := setupTestProxy(t) + fetcher.artifact = &fetch.Artifact{ + Body: io.NopCloser(strings.NewReader("archive")), + ContentType: "application/octet-stream", + } + h := NewPubHandlerWithUpstream(proxy, "http://localhost", "https://pub.example.com") + + for _, path := range []string{ + "/packages/http%3Fx/versions/1.2.0.tar.gz", + "/packages/http%23x/versions/1.2.0.tar.gz", + "/packages/http/versions/1.2.0%3Fx.tar.gz", + } { + fetcher.fetchCalled = false + fetcher.fetchedURL = "" + req := httptest.NewRequest(http.MethodGet, path, nil) + w := httptest.NewRecorder() + h.Routes().ServeHTTP(w, req) + + if strings.ContainsAny(fetcher.fetchedURL, "?#") { + t.Errorf("%s: upstream URL %q carries a query or fragment", path, fetcher.fetchedURL) + } + if strings.Contains(path, "/http%") && fetcher.fetchCalled { + t.Errorf("%s: invalid package name reached upstream as %q", path, fetcher.fetchedURL) + } + } +} From 32c299c70a95a63547fc32c1c073a6b612e7de6e Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sun, 18 Oct 2026 16:55:58 +0200 Subject: [PATCH 2/2] Pub archives that older versions cached from a path without .tar.gz are now discarded and fetched again, so versions that were already broken heal on the next download. --- internal/handler/pub.go | 28 +++++++++++++++++++ internal/handler/pub_test.go | 52 ++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/internal/handler/pub.go b/internal/handler/pub.go index dae51e2b..1fc69977 100644 --- a/internal/handler/pub.go +++ b/internal/handler/pub.go @@ -10,6 +10,7 @@ import ( "strings" "time" + "github.com/git-pkgs/proxy/internal/database" "github.com/git-pkgs/proxy/internal/retention" ) @@ -95,6 +96,7 @@ func (h *PubHandler) handleDownload(w http.ResponseWriter, r *http.Request) { downloadURL := fmt.Sprintf("%s/packages/%s/versions/%s%s", h.upstreamURL, url.PathEscape(name), url.PathEscape(version), pubArchiveExt) + h.discardVersionPage(name, version, filename) result, err := h.proxy.GetOrFetchArtifactFromURL( r.Context(), "pub", name, version, filename, downloadURL, ) @@ -106,6 +108,32 @@ func (h *PubHandler) handleDownload(w http.ResponseWriter, r *http.Request) { ServeArtifactRequest(w, r, result) } +// discardVersionPage clears a cached archive that was fetched from a path +// without the archive suffix. Proxy versions before the suffix check cached +// pub.dev's HTML version page under the archive's name that way. +func (h *PubHandler) discardVersionPage(name, version, filename string) { + _, versionPURL, err := packagePURLStrings("pub", name, version) + if err != nil { + return + } + record, err := h.proxy.DB.GetArtifact(versionPURL, filename) + if err != nil { + h.proxy.Logger.Warn("failed to read pub cache record", + "purl", versionPURL, "filename", filename, "error", err) + return + } + if record == nil || !record.StoragePath.Valid || record.Source == database.SourceLocal || + strings.HasSuffix(record.UpstreamURL, pubArchiveExt) { + return + } + h.proxy.Logger.Warn("cached pub archive was fetched without the archive suffix, discarding", + "purl", versionPURL, "filename", filename, "upstream_url", record.UpstreamURL) + if err := h.proxy.DB.DiscardArtifact(versionPURL, filename, record.StoragePath.String); err != nil { + h.proxy.Logger.Warn("failed to discard pub cache record", + "purl", versionPURL, "filename", filename, "error", err) + } +} + // handlePackageMetadata proxies package metadata and rewrites archive URLs. func (h *PubHandler) handlePackageMetadata(w http.ResponseWriter, r *http.Request) { name := r.PathValue("name") diff --git a/internal/handler/pub_test.go b/internal/handler/pub_test.go index f3dd4482..29526e01 100644 --- a/internal/handler/pub_test.go +++ b/internal/handler/pub_test.go @@ -154,3 +154,55 @@ func TestPubDownloadRejectsEncodedQueryInName(t *testing.T) { } } } + +// Older versions cached the HTML version page as the archive. Such an entry +// is fetched again instead of being served. +func TestPubDownloadRefetchesCachedVersionPage(t *testing.T) { + proxy, db, store, fetcher := setupTestProxy(t) + seedPackage(t, db, store, "pub", "http", "1.2.0", "http-1.2.0.tar.gz", "version page") + art, err := db.GetArtifact("pkg:pub/http@1.2.0", "http-1.2.0.tar.gz") + if err != nil || art == nil { + t.Fatalf("GetArtifact: %v", err) + } + art.UpstreamURL = "https://pub.dev/packages/http/versions/1.2.0" + if err := db.UpsertArtifact(art); err != nil { + t.Fatalf("UpsertArtifact: %v", err) + } + fetcher.artifact = &fetch.Artifact{ + Body: io.NopCloser(strings.NewReader("archive")), + ContentType: "application/octet-stream", + } + h := NewPubHandlerWithUpstream(proxy, "http://localhost", "https://pub.example.com") + + req := httptest.NewRequest(http.MethodGet, "/packages/http/versions/1.2.0.tar.gz", nil) + w := httptest.NewRecorder() + h.Routes().ServeHTTP(w, req) + + if !fetcher.fetchCalled { + t.Fatal("cached version page was served without fetching upstream") + } + if got := w.Body.String(); got != "archive" { + t.Errorf("body = %q, want %q", got, "archive") + } +} + +func TestPubDownloadServesCachedArchive(t *testing.T) { + proxy, db, store, fetcher := setupTestProxy(t) + seedPackage(t, db, store, "pub", "http", "1.2.0", "http-1.2.0.tar.gz", "cached archive") + fetcher.artifact = &fetch.Artifact{ + Body: io.NopCloser(strings.NewReader("refetched")), + ContentType: "application/octet-stream", + } + h := NewPubHandlerWithUpstream(proxy, "http://localhost", "https://pub.example.com") + + req := httptest.NewRequest(http.MethodGet, "/packages/http/versions/1.2.0.tar.gz", nil) + w := httptest.NewRecorder() + h.Routes().ServeHTTP(w, req) + + if fetcher.fetchCalled { + t.Errorf("cached archive fetched again from %q", fetcher.fetchedURL) + } + if got := w.Body.String(); got != "cached archive" { + t.Errorf("body = %q, want %q", got, "cached archive") + } +}