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..4a7ef71 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 { @@ -720,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 9ef79b0..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[path] = true + name := strings.TrimSuffix(strings.TrimPrefix(path, dirPath), "/") + if seenDirs[name] { + continue + } + seenDirs[name] = true } files = append(files, f.info) continue @@ -178,15 +182,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] + "/" - if !seenDirs[subdir] { - seenDirs[subdir] = true + rel := strings.TrimSuffix(strings.TrimPrefix(path, dirPath), "/") + if i := strings.IndexByte(rel, '/'); i >= 0 { + name := rel[:i] + if !seenDirs[name] { + seenDirs[name] = true files = append(files, FileInfo{ - Path: subdir, - Name: parts[0], + Path: dirPath + name + "/", + Name: name, 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..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[path] = true + name := strings.TrimSuffix(strings.TrimPrefix(path, dirPath), "/") + if seenDirs[name] { + continue + } + seenDirs[name] = true } files = append(files, fileInfoFromZip(f)) continue @@ -179,16 +183,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] + "/" - if !seenDirs[subdir] { - seenDirs[subdir] = true + rel := strings.TrimSuffix(strings.TrimPrefix(path, dirPath), "/") + if i := strings.IndexByte(rel, '/'); i >= 0 { + name := rel[:i] + if !seenDirs[name] { + seenDirs[name] = true files = append(files, FileInfo{ - Path: subdir, - Name: parts[0], + Path: dirPath + name + "/", + Name: name, IsDir: true, }) }