From 7b53f0b4822094f7535f72fe774893b27c7c473c Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Sun, 18 Oct 2026 12:51:35 +0100 Subject: [PATCH 1/4] Use shared archives browser for cached artifacts --- go.mod | 4 +- go.sum | 8 +- internal/server/browse.go | 17 +- internal/server/browse_archive.go | 357 +------------------------ internal/server/browse_archive_test.go | 33 +++ 5 files changed, 51 insertions(+), 368 deletions(-) diff --git a/go.mod b/go.mod index a760f5b9..9b027c9f 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/Masterminds/semver/v3 v3.5.0 github.com/aws/aws-sdk-go-v2/config v1.33.7 github.com/aws/aws-sdk-go-v2/service/ecr v1.66.2 - github.com/git-pkgs/archives v0.8.1 + github.com/git-pkgs/archives v0.8.2-0.20261018114934-e96eb78b9948 github.com/git-pkgs/artifacts v0.2.1 github.com/git-pkgs/cooldown v0.3.0 github.com/git-pkgs/enrichment v0.7.2 @@ -106,7 +106,7 @@ require ( github.com/rogpeppe/go-internal v1.16.0 // indirect github.com/rs/dnscache v0.0.0-20230804202142-fc85eb664529 // indirect github.com/rubyist/circuitbreaker v2.2.1+incompatible // indirect - github.com/ulikunitz/xz v0.5.16 // indirect + github.com/ulikunitz/xz v0.5.17 // indirect go.opentelemetry.io/auto/sdk v1.2.1 // indirect go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.69.0 // indirect go.opentelemetry.io/otel v1.45.0 // indirect diff --git a/go.sum b/go.sum index 387062b9..bb4fc669 100644 --- a/go.sum +++ b/go.sum @@ -117,8 +117,8 @@ github.com/facebookgo/clock v0.0.0-20150410010913-600d898af40a h1:yDWHCSQ40h88yi github.com/facebookgo/clock v0.0.0-20150410010913-600d898af40a/go.mod h1:7Ga40egUymuWXxAe151lTNnCv97MddSOVsjpPPkityA= github.com/felixge/httpsnoop v1.1.0 h1:3YtUj32ZZkqZtt3sZZsClsymw/QDuVfpNhoA31zeORc= github.com/felixge/httpsnoop v1.1.0/go.mod h1:Zqxgdd+1Rkcz8euOqdr7lqgCRJztwr5hp9vDSi5UZCE= -github.com/git-pkgs/archives v0.8.1 h1:IcT1bVsAec8SqJ4m9vTnD644KiadQZ4ERrWnm/30t3o= -github.com/git-pkgs/archives v0.8.1/go.mod h1:gIyPXPQr5e+YrMqA5KQ7x6pLrFXHNF1+CjdedNJdGUU= +github.com/git-pkgs/archives v0.8.2-0.20261018114934-e96eb78b9948 h1:kLCEb8vPZtiXtMlOLbg+MbmF5FAKnsebsAQgFPXeR4s= +github.com/git-pkgs/archives v0.8.2-0.20261018114934-e96eb78b9948/go.mod h1:GJ/h7NPTUYiPinOUxpYNsHFCUikYZcJXvugBvP1VGNc= github.com/git-pkgs/artifacts v0.2.1 h1:VwdxR4yTDaqBZ34h0slxQBVyr2Xfa+QGOKCKviMx2xk= github.com/git-pkgs/artifacts v0.2.1/go.mod h1:Otosgq52pXT5UNN7lh6s/lszpWKVDO8XrOjN1M70/IA= github.com/git-pkgs/cooldown v0.3.0 h1:eAl2CQFwqXxluoVP9hHmropAf1VoV3gSxLMkN+NEu8A= @@ -273,8 +273,8 @@ github.com/stretchr/testify v1.12.1 h1:EuwCh5fleGS7H32xRwO3wRGT7DxrDhLAT6FF8MpWD github.com/stretchr/testify v1.12.1/go.mod h1:MDEgiDPPsNp5cuIrHPPCyornHKgEVbtFUmoNlxoYthg= github.com/swaggo/swag v1.16.6 h1:qBNcx53ZaX+M5dxVyTrgQ0PJ/ACK+NzhwcbieTt+9yI= github.com/swaggo/swag v1.16.6/go.mod h1:ngP2etMK5a0P3QBizic5MEwpRmluJZPHjXcMoj4Xesg= -github.com/ulikunitz/xz v0.5.16 h1:ld6NyySjx5lowVKwJvMRLnW5nxKX/xnpSiFYZ/Lxur0= -github.com/ulikunitz/xz v0.5.16/go.mod h1:H9Rt/W6/Qj27PGauhQc6nfCDy7vHpzsOThBSaYDoEhw= +github.com/ulikunitz/xz v0.5.17 h1:flR0y/x1hgM8EGV1AW3Xll6T413G0glV8UfBwR617V4= +github.com/ulikunitz/xz v0.5.17/go.mod h1:H9Rt/W6/Qj27PGauhQc6nfCDy7vHpzsOThBSaYDoEhw= go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64= go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y= go.opentelemetry.io/contrib/detectors/gcp v1.44.0 h1:NmLfL734pJhM0JKaYd2Y28+nY9dPRWYAAbxhRCrKXPw= diff --git a/internal/server/browse.go b/internal/server/browse.go index 43cb855c..cb6e5e8d 100644 --- a/internal/server/browse.go +++ b/internal/server/browse.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "io/fs" "net/http" "path" "strings" @@ -98,20 +99,14 @@ func detectSingleRootDir(reader archives.Reader) string { return "" } - var root rootDetector - for _, f := range files { - if !root.add(f.Path) { - break - } - } - return root.prefix() + return archives.CommonRoot(files) } // openArchive opens a cached artifact as a random-access archive reader, // auto-detecting and stripping a single top-level directory prefix (like // GitHub zipballs). For npm, the hardcoded "package/" prefix takes precedence. // The whole artifact is buffered, so only the version diff uses it; listing -// and file reads stream through browseArchive instead. +// and file reads use the streaming browser instead. func openArchive(filename string, content io.Reader, ecosystem string) (archives.Reader, error) { //nolint:ireturn // wraps multiple archive implementations data, err := readBrowseInput(content, maxBrowseArchiveSize) if err != nil { @@ -142,7 +137,7 @@ func (s *Server) browseLimitsOrDefault() archives.StreamOptions { // openBrowseArchive prepares a cached artifact for streaming. content is the // already open storage reader; later passes reopen the artifact. -func (s *Server) openBrowseArchive(r *http.Request, artifact *database.Artifact, content io.Reader, ecosystem string) (*browseArchive, error) { +func (s *Server) openBrowseArchive(r *http.Request, artifact *database.Artifact, content io.Reader, ecosystem string) (*archives.Browser, error) { storagePath := artifact.StoragePath.String reopen := func() (io.ReadCloser, error) { return s.storage.Open(r.Context(), storagePath) @@ -398,10 +393,10 @@ func (s *Server) browseFile(w http.ResponseWriter, r *http.Request, ecosystem, n // Find the file and stream it straight from the archive fileReader, err := archive.Extract(filePath) switch { - case errors.Is(err, errBrowseNotFound): + case errors.Is(err, fs.ErrNotExist): notFound(w, "file not found") return - case errors.Is(err, errBrowseIsDir): + case errors.Is(err, archives.ErrIsDir): s.logger.Error("failed to extract file", "error", err, "path", filePath) internalError(w, "failed to extract file") return diff --git a/internal/server/browse_archive.go b/internal/server/browse_archive.go index 4af6afe1..4545ed7a 100644 --- a/internal/server/browse_archive.go +++ b/internal/server/browse_archive.go @@ -1,15 +1,11 @@ package server import ( - "bufio" "errors" "fmt" "io" - "path" - "strings" "github.com/git-pkgs/archives" - "github.com/git-pkgs/magic" ) // Limits applied when listing or reading a single file. Bodies are streamed @@ -31,14 +27,7 @@ var defaultBrowseLimits = archives.StreamOptions{ MaxEntries: maxBrowseEntries, } -var ( - errArtifactTooLarge = errors.New("artifact too large for browsing") - errBrowseNotFound = errors.New("file not found") - errBrowseIsDir = errors.New("path is a directory") - - // errStopWalk ends a walk early without reporting an error. - errStopWalk = errors.New("stop walk") -) +var errArtifactTooLarge = errors.New("artifact too large for browsing") // isBrowseLimitError reports whether err comes from one of the browse size or // entry-count limits rather than a malformed archive. @@ -50,44 +39,6 @@ func isBrowseLimitError(err error) bool { errors.Is(err, archives.ErrEntryLimit) } -// browseArchive reads a cached artifact sequentially for the browse endpoints -// without holding expanded file bodies in memory. TAR based formats stream from -// storage, reopening it when a second pass is needed. ZIP and conda need random -// access, so their input is read once and shared by every pass. -type browseArchive struct { - filename string - ecosystem string - limits archives.StreamOptions - reopen func() (io.ReadCloser, error) - - // first is the caller's reader, used for the first pass only. - first io.Reader - // data holds buffered ZIP or conda input. - data []byte -} - -// newBrowseArchive prepares content for streaming. The caller keeps ownership -// of content; reopen supplies a fresh reader for any later pass. -func newBrowseArchive(filename, ecosystem string, content io.Reader, reopen func() (io.ReadCloser, error), limits archives.StreamOptions) (*browseArchive, error) { - buffered := bufio.NewReaderSize(content, browseSniffSize) - sniff, err := buffered.Peek(browseSniffSize) - if err != nil && err != io.EOF { - return nil, fmt.Errorf("reading artifact: %w", err) - } - - a := &browseArchive{filename: filename, ecosystem: ecosystem, limits: limits, reopen: reopen} - if !needsRandomAccess(filename, sniff) { - a.first = buffered - return a, nil - } - - a.data, err = readBrowseInput(buffered, limits.MaxInputBytes) - if err != nil { - return nil, err - } - return a, nil -} - // readBrowseInput reads all of r, failing once it exceeds limit bytes. func readBrowseInput(r io.Reader, limit int64) ([]byte, error) { data, err := io.ReadAll(io.LimitReader(r, limit+1)) @@ -100,306 +51,10 @@ func readBrowseInput(r io.Reader, limit int64) ([]byte, error) { return data, nil } -// needsRandomAccess reports whether the archives library buffers this format -// when streaming it, following the library's own detection order: a known -// extension first, then the content's leading bytes. -func needsRandomAccess(filename string, sniff []byte) bool { - name := strings.ToLower(filename) - for _, suffix := range []string{".tar.gz", ".tar.bz2", ".tar.xz", ".tar.zst"} { - if strings.HasSuffix(name, suffix) { - return false - } - } - switch path.Ext(name) { - case ".zip", ".jar", ".whl", ".nupkg", ".egg", ".vsix", ".conda": - return true - case ".tar", ".tgz", ".crate", ".gem": - return false - } - return magic.DetectPrefix(sniff).Format == "zip" -} - -// openStream starts a pass over the archive. The returned function releases -// the stream and any reader opened for it. -func (a *browseArchive) openStream() (*archives.Stream, func(), error) { - if a.data != nil { - stream, err := archives.OpenStreamBytes(a.filename, a.data, a.limits) - if err != nil { - return nil, nil, err - } - return stream, func() { _ = stream.Close() }, nil - } - - if a.first != nil { - content := a.first - a.first = nil - stream, err := archives.OpenStream(a.filename, content, a.limits) - if err != nil { - return nil, nil, err - } - return stream, func() { _ = stream.Close() }, nil - } - - content, err := a.reopen() - if err != nil { - return nil, nil, fmt.Errorf("reopening artifact: %w", err) - } - stream, err := archives.OpenStream(a.filename, content, a.limits) - if err != nil { - _ = content.Close() - return nil, nil, err - } - return stream, func() { - _ = stream.Close() - _ = content.Close() - }, nil -} - -// walk calls fn for each entry in archive order until fn returns an error or -// the archive ends. fn may read the entry body from the stream. -func (a *browseArchive) walk(fn func(entry *archives.StreamEntry) error) error { - stream, release, err := a.openStream() - if err != nil { - return err - } - defer release() - - for { - entry, err := stream.Next() - if err == io.EOF { - return nil - } - if err != nil { - return err - } - if err := fn(entry); err != nil { - if errors.Is(err, errStopWalk) { - return nil - } - return err - } - } -} - -// prefix returns the directory stripped from every path: npm's "package/", or -// a single top-level directory shared by all entries (like GitHub zipballs). -// For other ecosystems this costs one pass, which stops early once two -// top-level names differ. -func (a *browseArchive) prefix() (string, error) { - if a.ecosystem == "npm" { - return npmPackagePrefix, nil - } - - var root rootDetector - err := a.walk(func(entry *archives.StreamEntry) error { - if !root.add(entry.Path) { - return errStopWalk - } - return nil - }) - if err != nil { - return "", err - } - return root.prefix(), nil -} - -// ListDir returns the entries directly inside dirPath, after prefix stripping, -// in a single pass that keeps only entry metadata. -func (a *browseArchive) ListDir(dirPath string) ([]archives.FileInfo, error) { - var entries []archives.FileInfo - var root rootDetector - err := a.walk(func(entry *archives.StreamEntry) error { - entries = append(entries, entry.FileInfo) - root.add(entry.Path) - return nil - }) - if err != nil { - return nil, err - } - - prefix := npmPackagePrefix - if a.ecosystem != "npm" { - prefix = root.prefix() - } - return listBrowseDir(entries, prefix, dirPath), nil -} - -// Extract returns the body of the first entry at filePath, streamed from the -// archive. The caller must close the returned reader. -func (a *browseArchive) Extract(filePath string) (io.ReadCloser, error) { - prefix, err := a.prefix() - if err != nil { - return nil, err - } - target := prefix + filePath - - stream, release, err := a.openStream() - if err != nil { - return nil, err - } - for { - entry, err := stream.Next() - if err == io.EOF { - release() - return nil, fmt.Errorf("%w: %s", errBrowseNotFound, filePath) - } - if err != nil { - release() - return nil, err - } - if entry.Path != target { - continue - } - if entry.IsDir { - release() - return nil, fmt.Errorf("%w: %s", errBrowseIsDir, filePath) - } - return &streamedFile{Reader: stream, release: release}, nil - } -} - -// streamedFile is an entry body that releases its archive stream on Close. -type streamedFile struct { - io.Reader - release func() -} - -func (f *streamedFile) Close() error { - if f.release != nil { - f.release() - f.release = nil - } - return nil -} - -// rootDetector tracks whether every path seen so far lives under one -// top-level directory. -type rootDetector struct { - root string - mixed bool - seen bool -} - -// add records p and reports whether a single root is still possible. -func (d *rootDetector) add(p string) bool { - if d.mixed { - return false - } - d.seen = true - dir, _, _ := strings.Cut(p, "/") - if d.root == "" { - d.root = dir - } else if dir != d.root { - d.mixed = true - } - return !d.mixed -} - -// prefix returns the shared root with a trailing slash, or "" if paths have -// more than one root or none were seen. -func (d *rootDetector) prefix() string { - if d.mixed || !d.seen || d.root == "" { - return "" - } - return d.root + "/" -} - -// listBrowseDir lists dirPath from entry metadata the same way a reader from -// archives.OpenWithPrefix does: the prefix is joined to dirPath before -// listing, then stripped from the results. -func listBrowseDir(entries []archives.FileInfo, prefix, dirPath string) []archives.FileInfo { - files := listDir(entries, prefix+dirPath) - if prefix == "" { - return files - } - return stripBrowsePrefix(files, prefix) -} - -// listDir mirrors archives.Reader.ListDir: entries directly in dirPath are -// returned in archive order. Deeper paths add one synthesized entry for each -// immediate subdirectory. -func listDir(entries []archives.FileInfo, dirPath string) []archives.FileInfo { - dirPath = normalizeBrowseDir(dirPath) - var files []archives.FileInfo - seenDirs := make(map[string]bool) - - for _, f := range entries { - p := f.Path - - if isInBrowseDir(p, dirPath) { - if f.IsDir { - name := strings.TrimSuffix(strings.TrimPrefix(p, dirPath), "/") - if seenDirs[name] { - continue - } - seenDirs[name] = true - } - files = append(files, f) - continue - } - - if dirPath != "" && !strings.HasPrefix(p, dirPath) { - continue - } - rel := strings.TrimSuffix(strings.TrimPrefix(p, dirPath), "/") - name, _, nested := strings.Cut(rel, "/") - if nested && !seenDirs[name] { - seenDirs[name] = true - files = append(files, archives.FileInfo{ - Path: dirPath + name + "/", - Name: name, - IsDir: true, - }) - } - } - - return files -} - -func normalizeBrowseDir(dirPath string) string { - dirPath = strings.Trim(strings.TrimSpace(dirPath), "/") - if dirPath == "" { - return "" - } - return dirPath + "/" -} - -// isInBrowseDir reports whether filePath is directly in the normalized -// dirPath. An entry naming the directory itself counts as inside it. -func isInBrowseDir(filePath, dirPath string) bool { - filePath = strings.TrimSuffix(filePath, "/") - if dirPath == "" { - return !strings.Contains(filePath, "/") - } - if filePath == dirPath[:len(dirPath)-1] { - return true - } - rest, ok := strings.CutPrefix(filePath, dirPath) - return ok && !strings.Contains(rest, "/") -} - -// stripBrowsePrefix removes prefix from each path, dropping entries outside -// it and the prefix directory itself. -func stripBrowsePrefix(files []archives.FileInfo, prefix string) []archives.FileInfo { - result := make([]archives.FileInfo, 0, len(files)) - for _, f := range files { - stripped, ok := strings.CutPrefix(f.Path, prefix) - if !ok || stripped == "" || stripped == "/" { - continue - } - f.Path = stripped - f.Name = entryName(stripped) - result = append(result, f) - } - return result -} - -// entryName returns the last element of an archive path, ignoring a trailing -// slash. -func entryName(p string) string { - p = strings.TrimSuffix(p, "/") - if i := strings.LastIndexByte(p, '/'); i >= 0 { - return p[i+1:] +func newBrowseArchive(filename, ecosystem string, content io.Reader, reopen func() (io.ReadCloser, error), limits archives.StreamOptions) (*archives.Browser, error) { + options := archives.BrowseOptions{Limits: limits, DetectRoot: true} + if ecosystem == "npm" { + options.StripPrefix = npmPackagePrefix } - return p + return archives.OpenBrowser(filename, content, reopen, options) } diff --git a/internal/server/browse_archive_test.go b/internal/server/browse_archive_test.go index c3c7796a..a1c80cd8 100644 --- a/internal/server/browse_archive_test.go +++ b/internal/server/browse_archive_test.go @@ -347,6 +347,39 @@ func TestBrowseFileFirstDuplicateWins(t *testing.T) { } } +func TestBrowseSingleRootFile(t *testing.T) { + for _, format := range []string{"tar.gz", "zip"} { + t.Run(format, func(t *testing.T) { + ts := newTestServer(t) + defer ts.close() + entries := []archiveEntry{{name: "README.md", body: "single file\n"}} + data := buildTarGz(t, entries) + if format == "zip" { + data = buildZip(t, entries) + } + filename := "single." + format + cacheBrowseArtifact(t, ts, "cargo", "single", filename, data) + paths := listingPaths(decodeListing(t, browseGet(ts, browseListURL("cargo", "single", "")))) + if len(paths) != 1 || paths[0] != "README.md" { + t.Fatalf("listing = %v, want README.md", paths) + } + response := browseGet(ts, browseFileURL("cargo", "single", "README.md")) + if response.Code != http.StatusOK || response.Body.String() != "single file\n" { + t.Fatalf("file = %d %q", response.Code, response.Body.String()) + } + r, err := openArchive(filename, bytes.NewReader(data), "cargo") + if err != nil { + t.Fatal(err) + } + defer func() { _ = r.Close() }() + files, err := r.List() + if err != nil || len(files) != 1 || files[0].Path != "README.md" { + t.Fatalf("diff reader listing = %+v, %v", files, err) + } + }) + } +} + // TestBrowseFileEscapedPath requests files the way the source browser does, // with each path segment percent-encoded, so names holding URL syntax resolve. func TestBrowseFileEscapedPath(t *testing.T) { From f329879ec16a993c2077c1b8c7029b3cbfbdf4ba Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Sun, 18 Oct 2026 13:22:14 +0100 Subject: [PATCH 2/4] Pin archives to combined browser and Hex changes --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 9b027c9f..6b8feabb 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/Masterminds/semver/v3 v3.5.0 github.com/aws/aws-sdk-go-v2/config v1.33.7 github.com/aws/aws-sdk-go-v2/service/ecr v1.66.2 - github.com/git-pkgs/archives v0.8.2-0.20261018114934-e96eb78b9948 + github.com/git-pkgs/archives v0.8.2-0.20261018121304-37b9a6e2b526 github.com/git-pkgs/artifacts v0.2.1 github.com/git-pkgs/cooldown v0.3.0 github.com/git-pkgs/enrichment v0.7.2 diff --git a/go.sum b/go.sum index bb4fc669..a61b3891 100644 --- a/go.sum +++ b/go.sum @@ -117,8 +117,8 @@ github.com/facebookgo/clock v0.0.0-20150410010913-600d898af40a h1:yDWHCSQ40h88yi github.com/facebookgo/clock v0.0.0-20150410010913-600d898af40a/go.mod h1:7Ga40egUymuWXxAe151lTNnCv97MddSOVsjpPPkityA= github.com/felixge/httpsnoop v1.1.0 h1:3YtUj32ZZkqZtt3sZZsClsymw/QDuVfpNhoA31zeORc= github.com/felixge/httpsnoop v1.1.0/go.mod h1:Zqxgdd+1Rkcz8euOqdr7lqgCRJztwr5hp9vDSi5UZCE= -github.com/git-pkgs/archives v0.8.2-0.20261018114934-e96eb78b9948 h1:kLCEb8vPZtiXtMlOLbg+MbmF5FAKnsebsAQgFPXeR4s= -github.com/git-pkgs/archives v0.8.2-0.20261018114934-e96eb78b9948/go.mod h1:GJ/h7NPTUYiPinOUxpYNsHFCUikYZcJXvugBvP1VGNc= +github.com/git-pkgs/archives v0.8.2-0.20261018121304-37b9a6e2b526 h1:tOFUEAD+Dn1HxAKoIC4br3bAvMzskpO60KiTe04i7X4= +github.com/git-pkgs/archives v0.8.2-0.20261018121304-37b9a6e2b526/go.mod h1:GJ/h7NPTUYiPinOUxpYNsHFCUikYZcJXvugBvP1VGNc= github.com/git-pkgs/artifacts v0.2.1 h1:VwdxR4yTDaqBZ34h0slxQBVyr2Xfa+QGOKCKviMx2xk= github.com/git-pkgs/artifacts v0.2.1/go.mod h1:Otosgq52pXT5UNN7lh6s/lszpWKVDO8XrOjN1M70/IA= github.com/git-pkgs/cooldown v0.3.0 h1:eAl2CQFwqXxluoVP9hHmropAf1VoV3gSxLMkN+NEu8A= From 4aa927473b0635b9066e39d827a8528db296204f Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Mon, 19 Oct 2026 08:28:26 +0100 Subject: [PATCH 3/4] Use archives v0.9.0 release --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 6b8feabb..7878beef 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/Masterminds/semver/v3 v3.5.0 github.com/aws/aws-sdk-go-v2/config v1.33.7 github.com/aws/aws-sdk-go-v2/service/ecr v1.66.2 - github.com/git-pkgs/archives v0.8.2-0.20261018121304-37b9a6e2b526 + github.com/git-pkgs/archives v0.9.0 github.com/git-pkgs/artifacts v0.2.1 github.com/git-pkgs/cooldown v0.3.0 github.com/git-pkgs/enrichment v0.7.2 diff --git a/go.sum b/go.sum index a61b3891..36686db5 100644 --- a/go.sum +++ b/go.sum @@ -117,8 +117,8 @@ github.com/facebookgo/clock v0.0.0-20150410010913-600d898af40a h1:yDWHCSQ40h88yi github.com/facebookgo/clock v0.0.0-20150410010913-600d898af40a/go.mod h1:7Ga40egUymuWXxAe151lTNnCv97MddSOVsjpPPkityA= github.com/felixge/httpsnoop v1.1.0 h1:3YtUj32ZZkqZtt3sZZsClsymw/QDuVfpNhoA31zeORc= github.com/felixge/httpsnoop v1.1.0/go.mod h1:Zqxgdd+1Rkcz8euOqdr7lqgCRJztwr5hp9vDSi5UZCE= -github.com/git-pkgs/archives v0.8.2-0.20261018121304-37b9a6e2b526 h1:tOFUEAD+Dn1HxAKoIC4br3bAvMzskpO60KiTe04i7X4= -github.com/git-pkgs/archives v0.8.2-0.20261018121304-37b9a6e2b526/go.mod h1:GJ/h7NPTUYiPinOUxpYNsHFCUikYZcJXvugBvP1VGNc= +github.com/git-pkgs/archives v0.9.0 h1:j5+GtZLrcsefIG5QxAQ2dcdCFf57uizcn03PyGRFA3U= +github.com/git-pkgs/archives v0.9.0/go.mod h1:GJ/h7NPTUYiPinOUxpYNsHFCUikYZcJXvugBvP1VGNc= github.com/git-pkgs/artifacts v0.2.1 h1:VwdxR4yTDaqBZ34h0slxQBVyr2Xfa+QGOKCKviMx2xk= github.com/git-pkgs/artifacts v0.2.1/go.mod h1:Otosgq52pXT5UNN7lh6s/lszpWKVDO8XrOjN1M70/IA= github.com/git-pkgs/cooldown v0.3.0 h1:eAl2CQFwqXxluoVP9hHmropAf1VoV3gSxLMkN+NEu8A= From ac8ebb8a12554a701faecb4756b2e5836caa0307 Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Tue, 20 Oct 2026 11:09:07 +0100 Subject: [PATCH 4/4] Return 404 when browsing archive directories as files --- internal/server/browse.go | 6 +---- internal/server/browse_archive_test.go | 34 ++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/internal/server/browse.go b/internal/server/browse.go index cb6e5e8d..d172aceb 100644 --- a/internal/server/browse.go +++ b/internal/server/browse.go @@ -393,13 +393,9 @@ func (s *Server) browseFile(w http.ResponseWriter, r *http.Request, ecosystem, n // Find the file and stream it straight from the archive fileReader, err := archive.Extract(filePath) switch { - case errors.Is(err, fs.ErrNotExist): + case errors.Is(err, fs.ErrNotExist), errors.Is(err, archives.ErrIsDir): notFound(w, "file not found") return - case errors.Is(err, archives.ErrIsDir): - s.logger.Error("failed to extract file", "error", err, "path", filePath) - internalError(w, "failed to extract file") - return case err != nil: s.browseArchiveError(w, err, cachedArtifact.Filename) return diff --git a/internal/server/browse_archive_test.go b/internal/server/browse_archive_test.go index a1c80cd8..838802e2 100644 --- a/internal/server/browse_archive_test.go +++ b/internal/server/browse_archive_test.go @@ -9,6 +9,7 @@ import ( "encoding/json" "fmt" "io" + "log/slog" "net/http" "net/http/httptest" "net/url" @@ -332,6 +333,39 @@ func TestBrowseFileContents(t *testing.T) { } } +func TestBrowseFileDirectoryNotFound(t *testing.T) { + for _, format := range []string{"tar.gz", "zip"} { + for _, explicit := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/explicit=%t", format, explicit), func(t *testing.T) { + ts := newTestServer(t) + defer ts.close() + var logs bytes.Buffer + ts.server.logger = slog.New(slog.NewTextHandler(&logs, &slog.HandlerOptions{Level: slog.LevelError})) + + entries := []archiveEntry{{name: "pkg/src/main.go", body: "package main\n"}} + if explicit { + entries = append([]archiveEntry{{name: "pkg/src/", dir: true}}, entries...) + } + data := buildTarGz(t, entries) + if format == "zip" { + data = buildZip(t, entries) + } + cacheBrowseArtifact(t, ts, "cargo", "demo", "demo."+format, data) + + for _, dir := range []string{"src", "src/"} { + response := browseGet(ts, browseFileURL("cargo", "demo", dir)) + if response.Code != http.StatusNotFound { + t.Errorf("directory %q returned %d, want 404: %s", dir, response.Code, response.Body.String()) + } + } + if logs.Len() != 0 { + t.Errorf("directory request logged an error: %s", logs.String()) + } + }) + } + } +} + func TestBrowseFileFirstDuplicateWins(t *testing.T) { ts := newTestServer(t) defer ts.close()