fs: Fix nested dir listing for archives without explicit dirs (#339)

Author: jeremyje

.gitignore

@@ -1 +1,2 @@
 _gitignore
+__debug_bin

archiver.go

@@ -148,6 +148,9 @@ func nameOnDiskToNameInArchive(nameOnDisk, rootOnDisk, rootInArchive string) str
 //
 // For example, "a/b/c" => "b/c".
 func trimTopDir(dir string) string {
+	if len(dir) > 0 && dir[0] == '/' {
+		dir = dir[1:]
+	}
 	if pos := strings.Index(dir, "/"); pos >= 0 {
 		return dir[pos+1:]
 	}
@@ -159,6 +162,9 @@ func trimTopDir(dir string) string {
 //
 // For example, "a/b/c" => "a".
 func topDir(dir string) string {
+	if len(dir) > 0 && dir[0] == '/' {
+		dir = dir[1:]
+	}
 	if pos := strings.Index(dir, "/"); pos >= 0 {
 		return dir[:pos]
 	}

archiver_test.go

@@ -7,6 +7,46 @@ import (
 	"testing"
 )
 
+func TestTrimTopDir(t *testing.T) {
+	for _, tc := range []struct {
+		input string
+		want  string
+	}{
+		{input: "a/b/c", want: "b/c"},
+		{input: "a", want: "a"},
+		{input: "abc/def", want: "def"},
+		{input: "/abc/def", want: "def"},
+	} {
+		tc := tc
+		t.Run(tc.input, func(t *testing.T) {
+			got := trimTopDir(tc.input)
+			if got != tc.want {
+				t.Errorf("want: '%s', got: '%s')", tc.want, got)
+			}
+		})
+	}
+}
+
+func TestTopDir(t *testing.T) {
+	for _, tc := range []struct {
+		input string
+		want  string
+	}{
+		{input: "a/b/c", want: "a"},
+		{input: "a", want: "a"},
+		{input: "abc/def", want: "abc"},
+		{input: "/abc/def", want: "abc"},
+	} {
+		tc := tc
+		t.Run(tc.input, func(t *testing.T) {
+			got := topDir(tc.input)
+			if got != tc.want {
+				t.Errorf("want: '%s', got: '%s')", tc.want, got)
+			}
+		})
+	}
+}
+
 func TestFileIsIncluded(t *testing.T) {
 	for i, tc := range []struct {
 		included  []string

fs.go

@@ -458,13 +458,17 @@ func (f ArchiveFS) ReadDir(name string) ([]fs.DirEntry, error) {
 			// leaving them to be inferred from the names of files instead (issue #330)
 			// so as we traverse deeper, we need to implicitly find subfolders within
 			// this current directory and add fake entries to the output
-			remainingPath := strings.TrimPrefix(file.NameInArchive, name)
+			remainingPath := file.NameInArchive
+
+			if name != "." {
+				remainingPath = strings.TrimPrefix(file.NameInArchive, name)
+			}
 			nextDir := topDir(remainingPath)        // if name in archive is "a/b/c" and root is "a", this becomes "b" (the implied folder to add)
 			implicitDir := path.Join(name, nextDir) // the full path of the implied directory
 
 			// create fake entry only if no entry currently exists (don't overwrite a real entry)
 			if _, ok := entries[implicitDir]; !ok {
-				entries[implicitDir] = implicitDirEntry{implicitDir}
+				entries[implicitDir] = implicitDirEntry{nextDir}
 			}
 
 			return fs.SkipDir

fs_test.go

@@ -9,6 +9,8 @@ import (
 	"log"
 	"net/http"
 	"path"
+	"reflect"
+	"sort"
 	"testing"
 )
 
@@ -40,9 +42,14 @@ func TestPathWithoutTopDir(t *testing.T) {
 }
 
 //go:generate zip testdata/test.zip go.mod
+//go:generate zip -qr9 testdata/nodir.zip archiver.go go.mod cmd/arc/main.go .github/ISSUE_TEMPLATE/bug_report.md .github/FUNDING.yml README.md .github/workflows/ubuntu-latest.yml
 
-//go:embed testdata/test.zip
-var testZIP []byte
+var (
+	//go:embed testdata/test.zip
+	testZIP []byte
+	//go:embed testdata/nodir.zip
+	nodirZIP []byte
+)
 
 func ExampleArchiveFS_Stream() {
 	fsys := ArchiveFS{
@@ -70,3 +77,96 @@ func ExampleArchiveFS_Stream() {
 	// go.mod
 	// true
 }
+
+func TestArchiveFS_ReadDir(t *testing.T) {
+	for _, tc := range []struct {
+		name    string
+		archive ArchiveFS
+		want    map[string][]string
+	}{
+		{
+			name: "test.zip",
+			archive: ArchiveFS{
+				Stream: io.NewSectionReader(bytes.NewReader(testZIP), 0, int64(len(testZIP))),
+				Format: Zip{},
+			},
+			// unzip -l testdata/test.zip
+			want: map[string][]string{
+				".": {"go.mod"},
+			},
+		},
+		{
+			name: "nodir.zip",
+			archive: ArchiveFS{
+				Stream: io.NewSectionReader(bytes.NewReader(nodirZIP), 0, int64(len(nodirZIP))),
+				Format: Zip{},
+			},
+			// unzip -l testdata/nodir.zip
+			want: map[string][]string{
+				".":       {".github", "README.md", "archiver.go", "cmd", "go.mod"},
+				".github": {"FUNDING.yml", "ISSUE_TEMPLATE", "workflows"},
+				"cmd":     {"arc"},
+			},
+		},
+	} {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			fsys := tc.archive
+			for baseDir, wantLS := range tc.want {
+				baseDir := baseDir
+				wantLS := wantLS
+				t.Run(fmt.Sprintf("ReadDir(%s)", baseDir), func(t *testing.T) {
+					dis, err := fsys.ReadDir(baseDir)
+					if err != nil {
+						t.Error(err)
+					}
+
+					dirs := []string{}
+					for _, di := range dis {
+						dirs = append(dirs, di.Name())
+					}
+
+					// Stabilize the sort order
+					sort.Strings(dirs)
+
+					if !reflect.DeepEqual(wantLS, dirs) {
+						t.Errorf("ReadDir() got: %v, want: %v", dirs, wantLS)
+					}
+				})
+
+				// Uncomment to reproduce https://github.com/mholt/archiver/issues/340.
+				/*
+					t.Run(fmt.Sprintf("Open(%s)", baseDir), func(t *testing.T) {
+						f, err := fsys.Open(baseDir)
+						if err != nil {
+							t.Error(err)
+						}
+
+						rdf, ok := f.(fs.ReadDirFile)
+						if !ok {
+							t.Fatalf("'%s' did not return a fs.ReadDirFile, %+v", baseDir, rdf)
+						}
+
+						dis, err := rdf.ReadDir(-1)
+						if err != nil {
+							t.Fatal(err)
+						}
+
+						dirs := []string{}
+						for _, di := range dis {
+							dirs = append(dirs, di.Name())
+						}
+
+						// Stabilize the sort order
+						sort.Strings(dirs)
+
+						if !reflect.DeepEqual(wantLS, dirs) {
+							t.Errorf("Open().ReadDir(-1) got: %v, want: %v", dirs, wantLS)
+						}
+					})
+				*/
+			}
+		})
+	}
+}