From f05d66e17e75f319370f5e392c9744fe4230abf4 Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Sun, 13 Sep 2026 08:30:28 -0400 Subject: [PATCH 1/3] Drop per-entry allocations in isInDir and ListDir isInDir now requires a normalized dirPath (both callers already pass one) and uses IndexByte instead of Split; ListDir's subdir check does the same. No behaviour change. --- archives.go | 28 +++++++++++----------------- archives_test.go | 12 +++++++----- tar.go | 9 ++++----- tar_bench_test.go | 35 +++++++++++++++++++++++++++++++++++ zip.go | 10 ++++------ 5 files changed, 61 insertions(+), 33 deletions(-) diff --git a/archives.go b/archives.go index 6c41eeb..00f4550 100644 --- a/archives.go +++ b/archives.go @@ -252,29 +252,23 @@ func normalizeDir(dirPath string) string { return dirPath + "/" } -// isInDir checks if filePath is directly in dirPath (not in subdirectories). +// isInDir reports whether filePath is directly in dirPath (not in a +// subdirectory). dirPath must already be normalized via normalizeDir; +// callers do this once outside the per-entry loop. func isInDir(filePath, dirPath string) bool { - dirPath = normalizeDir(dirPath) - - // Normalize file path by trimming trailing slash filePath = strings.TrimSuffix(filePath, "/") - // Root directory if dirPath == "" { - // File is in root if it has no slashes - parts := strings.Split(filePath, "/") - return len(parts) == 1 + return strings.IndexByte(filePath, '/') < 0 } - // Check if file starts with directory path - if !strings.HasPrefix(filePath+"/", dirPath) { + // dirPath ends in "/"; an entry naming the directory itself counts as + // inside it so explicit dir entries in the archive are listed. + if filePath == dirPath[:len(dirPath)-1] { + return true + } + if !strings.HasPrefix(filePath, dirPath) { return false } - - // Get relative path - rel := strings.TrimPrefix(filePath, strings.TrimSuffix(dirPath, "/")) - rel = strings.TrimPrefix(rel, "/") - - // Should have no more slashes - return !strings.Contains(rel, "/") + return strings.IndexByte(filePath[len(dirPath):], '/') < 0 } diff --git a/archives_test.go b/archives_test.go index 8618263..9856d7d 100644 --- a/archives_test.go +++ b/archives_test.go @@ -85,11 +85,13 @@ func TestIsInDir(t *testing.T) { }{ {"file.txt", "", true}, {"dir/file.txt", "", false}, - {"dir/file.txt", "dir", true}, - {"dir/subdir/file.txt", "dir", false}, - {"dir/subdir/file.txt", "dir/subdir", true}, - {"other/file.txt", "dir", false}, - {"dir/", "", true}, // dir entry is in root + {"dir/file.txt", "dir/", true}, + {"dir/subdir/file.txt", "dir/", false}, + {"dir/subdir/file.txt", "dir/subdir/", true}, + {"other/file.txt", "dir/", false}, + {"dir/", "", true}, // dir entry is in root + {"dir/", "dir/", true}, // explicit entry for the directory itself + {"dirX/file", "dir/", false}, } for _, tt := range tests { diff --git a/tar.go b/tar.go index 9ef79b0..4a979f5 100644 --- a/tar.go +++ b/tar.go @@ -178,15 +178,14 @@ func (t *tarReader) ListDir(dirPath string) ([]FileInfo, error) { // Check if we should add a subdirectory entry if dirPath == "" || strings.HasPrefix(path, dirPath) { - rel := strings.TrimPrefix(path, dirPath) - parts := strings.Split(strings.TrimSuffix(rel, "/"), "/") - if len(parts) > 1 { - subdir := dirPath + parts[0] + "/" + rel := strings.TrimSuffix(strings.TrimPrefix(path, dirPath), "/") + if i := strings.IndexByte(rel, '/'); i >= 0 { + subdir := dirPath + rel[:i] + "/" if !seenDirs[subdir] { seenDirs[subdir] = true files = append(files, FileInfo{ Path: subdir, - Name: parts[0], + Name: rel[:i], IsDir: true, }) } diff --git a/tar_bench_test.go b/tar_bench_test.go index 40c7da9..b707ac6 100644 --- a/tar_bench_test.go +++ b/tar_bench_test.go @@ -1,6 +1,8 @@ package archives import ( + "archive/tar" + "bytes" "fmt" "io" "testing" @@ -20,6 +22,39 @@ func BenchmarkTarBrowse(b *testing.B) { } } +func BenchmarkListDir(b *testing.B) { + const ( + files = 2000 + subdirs = 20 + filePerm = 0o644 + ) + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + for i := range files { + _ = tw.WriteHeader(&tar.Header{ + Name: fmt.Sprintf("package/lib/sub%02d/file%04d.dat", i%subdirs, i), + Mode: filePerm, + }) + } + _ = tw.Close() + r, err := OpenBytesWithPrefix("test.tar", buf.Bytes(), "package/") + if err != nil { + b.Fatal(err) + } + b.Cleanup(func() { _ = r.Close() }) + + for _, dir := range []string{"", "lib", "lib/sub00"} { + b.Run(dir, func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + if _, err := r.ListDir(dir); err != nil { + b.Fatal(err) + } + } + }) + } +} + func benchmarkTarBrowse(b *testing.B, filename string, payload []byte) { b.Helper() raw := compressTar(b, filename, tarWithPayloads(b, payload, payload, payload, payload)) diff --git a/zip.go b/zip.go index 838fff9..e1d0434 100644 --- a/zip.go +++ b/zip.go @@ -179,16 +179,14 @@ func (z *zipReader) ListDir(dirPath string) ([]FileInfo, error) { // Check if we should add a subdirectory entry if dirPath == "" || strings.HasPrefix(path, dirPath) { - rel := strings.TrimPrefix(path, dirPath) - parts := strings.Split(strings.TrimSuffix(rel, "/"), "/") - if len(parts) > 1 { - // This file is in a subdirectory - subdir := dirPath + parts[0] + "/" + rel := strings.TrimSuffix(strings.TrimPrefix(path, dirPath), "/") + if i := strings.IndexByte(rel, '/'); i >= 0 { + subdir := dirPath + rel[:i] + "/" if !seenDirs[subdir] { seenDirs[subdir] = true files = append(files, FileInfo{ Path: subdir, - Name: parts[0], + Name: rel[:i], IsDir: true, }) } From 79985fd402f9a320dac76004ef0c570ab247ca0f Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Sun, 13 Sep 2026 08:34:40 -0400 Subject: [PATCH 2/3] Key seenDirs by child name instead of full path The full path was built for every entry to use as the dedup key; the child segment name is just as unique within a single ListDir call and is a substring of the entry name, so the concat now happens once per distinct child rather than once per entry. --- tar.go | 12 ++++++------ zip.go | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tar.go b/tar.go index 4a979f5..1ebf6b1 100644 --- a/tar.go +++ b/tar.go @@ -170,7 +170,7 @@ func (t *tarReader) ListDir(dirPath string) ([]FileInfo, error) { // Check if this file/dir is directly in the requested directory if isInDir(path, dirPath) { if f.info.IsDir { - seenDirs[path] = true + seenDirs[strings.TrimSuffix(strings.TrimPrefix(path, dirPath), "/")] = true } files = append(files, f.info) continue @@ -180,12 +180,12 @@ func (t *tarReader) ListDir(dirPath string) ([]FileInfo, error) { if dirPath == "" || strings.HasPrefix(path, dirPath) { rel := strings.TrimSuffix(strings.TrimPrefix(path, dirPath), "/") if i := strings.IndexByte(rel, '/'); i >= 0 { - subdir := dirPath + rel[:i] + "/" - if !seenDirs[subdir] { - seenDirs[subdir] = true + name := rel[:i] + if !seenDirs[name] { + seenDirs[name] = true files = append(files, FileInfo{ - Path: subdir, - Name: rel[:i], + Path: dirPath + name + "/", + Name: name, IsDir: true, }) } diff --git a/zip.go b/zip.go index e1d0434..3564fc0 100644 --- a/zip.go +++ b/zip.go @@ -171,7 +171,7 @@ func (z *zipReader) ListDir(dirPath string) ([]FileInfo, error) { // Check if this file/dir is directly in the requested directory if isInDir(path, dirPath) { if f.FileInfo().IsDir() { - seenDirs[path] = true + seenDirs[strings.TrimSuffix(strings.TrimPrefix(path, dirPath), "/")] = true } files = append(files, fileInfoFromZip(f)) continue @@ -181,12 +181,12 @@ func (z *zipReader) ListDir(dirPath string) ([]FileInfo, error) { if dirPath == "" || strings.HasPrefix(path, dirPath) { rel := strings.TrimSuffix(strings.TrimPrefix(path, dirPath), "/") if i := strings.IndexByte(rel, '/'); i >= 0 { - subdir := dirPath + rel[:i] + "/" - if !seenDirs[subdir] { - seenDirs[subdir] = true + name := rel[:i] + if !seenDirs[name] { + seenDirs[name] = true files = append(files, FileInfo{ - Path: subdir, - Name: rel[:i], + Path: dirPath + name + "/", + Name: name, IsDir: true, }) } From b36321c7e0fb8ee46fdc42e591f184ac7fc48358 Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Sun, 13 Sep 2026 08:39:17 -0400 Subject: [PATCH 3/3] Skip late explicit dir entries in ListDir An archive that lists files before their parent directory entry got a synthetic subdir entry from the first file and then the explicit entry appended as well. Check seenDirs before appending an explicit dir entry so the first one wins, matching the dir-entry-first case. --- archives_test.go | 28 ++++++++++++++++++++++++++++ tar.go | 6 +++++- zip.go | 6 +++++- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/archives_test.go b/archives_test.go index 9856d7d..4a7ef71 100644 --- a/archives_test.go +++ b/archives_test.go @@ -722,6 +722,34 @@ func TestTarListDirNoDuplicatesWithExplicitDirEntries(t *testing.T) { assertNoDuplicates(t, "ListDir project-abc123/", files) } +func TestListDirNoDuplicatesWithLateDirEntry(t *testing.T) { + // Some archives (hand-built or from tools that append a manifest + // pass) emit files before the explicit entry for their parent + // directory. ListDir synthesises a subdir entry on the first file + // and must then skip the explicit one. + buf := new(bytes.Buffer) + tw := tar.NewWriter(buf) + _ = tw.WriteHeader(&tar.Header{Name: "pkg/src/main.go", Mode: 0o644, Size: 1}) + _, _ = tw.Write([]byte("x")) + _ = tw.WriteHeader(&tar.Header{Name: "pkg/src/", Typeflag: tar.TypeDir, Mode: 0o755}) + _ = tw.Close() + + reader, err := openTar(buf.Bytes(), "") + if err != nil { + t.Fatalf("openTar failed: %v", err) + } + defer func() { _ = reader.Close() }() + + files, err := reader.ListDir("pkg/") + if err != nil { + t.Fatalf("ListDir failed: %v", err) + } + assertNoDuplicates(t, "ListDir pkg/", files) + if len(files) != 1 || files[0].Path != "pkg/src/" || !files[0].IsDir { + t.Errorf("ListDir pkg/ = %+v, want single pkg/src/ dir", files) + } +} + func TestGetStripPrefixNpm(t *testing.T) { // Create npm-style archive buf := new(bytes.Buffer) diff --git a/tar.go b/tar.go index 1ebf6b1..2d3c451 100644 --- a/tar.go +++ b/tar.go @@ -170,7 +170,11 @@ func (t *tarReader) ListDir(dirPath string) ([]FileInfo, error) { // Check if this file/dir is directly in the requested directory if isInDir(path, dirPath) { if f.info.IsDir { - seenDirs[strings.TrimSuffix(strings.TrimPrefix(path, dirPath), "/")] = true + name := strings.TrimSuffix(strings.TrimPrefix(path, dirPath), "/") + if seenDirs[name] { + continue + } + seenDirs[name] = true } files = append(files, f.info) continue diff --git a/zip.go b/zip.go index 3564fc0..01c1341 100644 --- a/zip.go +++ b/zip.go @@ -171,7 +171,11 @@ func (z *zipReader) ListDir(dirPath string) ([]FileInfo, error) { // Check if this file/dir is directly in the requested directory if isInDir(path, dirPath) { if f.FileInfo().IsDir() { - seenDirs[strings.TrimSuffix(strings.TrimPrefix(path, dirPath), "/")] = true + name := strings.TrimSuffix(strings.TrimPrefix(path, dirPath), "/") + if seenDirs[name] { + continue + } + seenDirs[name] = true } files = append(files, fileInfoFromZip(f)) continue