[release-branch.go1.16] archive/zip: only return directory once via io/fs.FS

While we're here fix the ModTime value for directories.

For #43872
For #45345
Fixes #45347

Change-Id: I155e6517713ef6a9482b9431f1167a44337c6ad2
Reviewed-on: https://go-review.googlesource.com/c/go/+/311530
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
(cherry picked from commit 87e4dcd446df2ab1985ef61ce15da329493248a1)
Reviewed-on: https://go-review.googlesource.com/c/go/+/315249
Trust: Jeremy Faller <jeremy@golang.org>

diff --git a/src/archive/zip/reader.go b/src/archive/zip/reader.go
index c288ad965bc92cd4477e8c9da1eb8033d84c3cbe..18f9833db0a08e3ad0040e8d69847e0060792e60 100644 (file)
--- a/src/archive/zip/reader.go
+++ b/src/archive/zip/reader.go
@@ -628,10 +628,11 @@ func (b *readBuf) sub(n int) readBuf {
 }
 
 // A fileListEntry is a File and its ename.
-// If file == nil, the fileListEntry describes a directory, without metadata.
+// If file == nil, the fileListEntry describes a directory without metadata.
 type fileListEntry struct {
-       name string
-       file *File // nil for directories
+       name  string
+       file  *File
+       isDir bool
 }
 
 type fileInfoDirEntry interface {
@@ -640,20 +641,26 @@ type fileInfoDirEntry interface {
 }
 
 func (e *fileListEntry) stat() fileInfoDirEntry {
-       if e.file != nil {
+       if !e.isDir {
                return headerFileInfo{&e.file.FileHeader}
        }
        return e
 }
 
 // Only used for directories.
-func (f *fileListEntry) Name() string       { _, elem, _ := split(f.name); return elem }
-func (f *fileListEntry) Size() int64        { return 0 }
-func (f *fileListEntry) ModTime() time.Time { return time.Time{} }
-func (f *fileListEntry) Mode() fs.FileMode  { return fs.ModeDir | 0555 }
-func (f *fileListEntry) Type() fs.FileMode  { return fs.ModeDir }
-func (f *fileListEntry) IsDir() bool        { return true }
-func (f *fileListEntry) Sys() interface{}   { return nil }
+func (f *fileListEntry) Name() string      { _, elem, _ := split(f.name); return elem }
+func (f *fileListEntry) Size() int64       { return 0 }
+func (f *fileListEntry) Mode() fs.FileMode { return fs.ModeDir | 0555 }
+func (f *fileListEntry) Type() fs.FileMode { return fs.ModeDir }
+func (f *fileListEntry) IsDir() bool       { return true }
+func (f *fileListEntry) Sys() interface{}  { return nil }
+
+func (f *fileListEntry) ModTime() time.Time {
+       if f.file == nil {
+               return time.Time{}
+       }
+       return f.file.FileHeader.Modified.UTC()
+}
 
 func (f *fileListEntry) Info() (fs.FileInfo, error) { return f, nil }
 
@@ -673,15 +680,32 @@ func toValidName(name string) string {
 func (r *Reader) initFileList() {
        r.fileListOnce.Do(func() {
                dirs := make(map[string]bool)
+               knownDirs := make(map[string]bool)
                for _, file := range r.File {
+                       isDir := len(file.Name) > 0 && file.Name[len(file.Name)-1] == '/'
                        name := toValidName(file.Name)
                        for dir := path.Dir(name); dir != "."; dir = path.Dir(dir) {
                                dirs[dir] = true
                        }
-                       r.fileList = append(r.fileList, fileListEntry{name, file})
+                       entry := fileListEntry{
+                               name:  name,
+                               file:  file,
+                               isDir: isDir,
+                       }
+                       r.fileList = append(r.fileList, entry)
+                       if isDir {
+                               knownDirs[name] = true
+                       }
                }
                for dir := range dirs {
-                       r.fileList = append(r.fileList, fileListEntry{dir + "/", nil})
+                       if !knownDirs[dir] {
+                               entry := fileListEntry{
+                                       name:  dir,
+                                       file:  nil,
+                                       isDir: true,
+                               }
+                               r.fileList = append(r.fileList, entry)
+                       }
                }
 
                sort.Slice(r.fileList, func(i, j int) bool { return fileEntryLess(r.fileList[i].name, r.fileList[j].name) })
@@ -705,7 +729,7 @@ func (r *Reader) Open(name string) (fs.File, error) {
        if e == nil || !fs.ValidPath(name) {
                return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist}
        }
-       if e.file == nil || strings.HasSuffix(e.file.Name, "/") {
+       if e.isDir {
                return &openDir{e, r.openReadDir(name), 0}, nil
        }
        rc, err := e.file.Open()
@@ -730,7 +754,7 @@ func split(name string) (dir, elem string, isDir bool) {
        return name[:i], name[i+1:], isDir
 }
 
-var dotFile = &fileListEntry{name: "./"}
+var dotFile = &fileListEntry{name: "./", isDir: true}
 
 func (r *Reader) openLookup(name string) *fileListEntry {
        if name == "." {

diff --git a/src/archive/zip/reader_test.go b/src/archive/zip/reader_test.go
index 5faf1f49b51e7b8e4cd25fbb34dfd32d46ed03bd..c6e8d215681604c4648370c017b5afa9d0e1d013 100644 (file)
--- a/src/archive/zip/reader_test.go
+++ b/src/archive/zip/reader_test.go
@@ -1073,12 +1073,62 @@ func TestIssue12449(t *testing.T) {
 }
 
 func TestFS(t *testing.T) {
-       z, err := OpenReader("testdata/unix.zip")
+       for _, test := range []struct {
+               file string
+               want []string
+       }{
+               {
+                       "testdata/unix.zip",
+                       []string{"hello", "dir/bar", "readonly"},
+               },
+               {
+                       "testdata/subdir.zip",
+                       []string{"a/b/c"},
+               },
+       } {
+               t.Run(test.file, func(t *testing.T) {
+                       t.Parallel()
+                       z, err := OpenReader(test.file)
+                       if err != nil {
+                               t.Fatal(err)
+                       }
+                       defer z.Close()
+                       if err := fstest.TestFS(z, test.want...); err != nil {
+                               t.Error(err)
+                       }
+               })
+       }
+}
+
+func TestFSModTime(t *testing.T) {
+       t.Parallel()
+       z, err := OpenReader("testdata/subdir.zip")
        if err != nil {
                t.Fatal(err)
        }
-       if err := fstest.TestFS(z, "hello", "dir/bar", "dir/empty", "readonly"); err != nil {
-               t.Fatal(err)
+       defer z.Close()
+
+       for _, test := range []struct {
+               name string
+               want time.Time
+       }{
+               {
+                       "a",
+                       time.Date(2021, 4, 19, 12, 29, 56, 0, timeZone(-7*time.Hour)).UTC(),
+               },
+               {
+                       "a/b/c",
+                       time.Date(2021, 4, 19, 12, 29, 59, 0, timeZone(-7*time.Hour)).UTC(),
+               },
+       } {
+               fi, err := fs.Stat(z, test.name)
+               if err != nil {
+                       t.Errorf("%s: %v", test.name, err)
+                       continue
+               }
+               if got := fi.ModTime(); !got.Equal(test.want) {
+                       t.Errorf("%s: got modtime %v, want %v", test.name, got, test.want)
+               }
        }
 }
 

diff --git a/src/archive/zip/testdata/subdir.zip b/src/archive/zip/testdata/subdir.zip
new file mode 100644 (file)
index 0000000..324d06b
Binary files /dev/null and b/src/archive/zip/testdata/subdir.zip differ