Skip to content

Commit 87e4dcd

Browse files
ianlancetaylorianlancetaylor
committed
archive/zip: only return directory once via io/fs.FS
While we're here fix the ModTime value for directories. Fixes #43872 Fixes #45345 Change-Id: I155e6517713ef6a9482b9431f1167a44337c6ad2 Reviewed-on: https://go-review.googlesource.com/c/go/+/311530 Trust: Ian Lance Taylor <[email protected]> Run-TryBot: Ian Lance Taylor <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Jeremy Faller <[email protected]>
1 parent 6d95e5a commit 87e4dcd

3 files changed

Lines changed: 92 additions & 18 deletions

File tree

‎src/archive/zip/reader.go‎

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -628,10 +628,11 @@ func (b *readBuf) sub(n int) readBuf {
628628
}
629629

630630
// A fileListEntry is a File and its ename.
631-
// If file == nil, the fileListEntry describes a directory, without metadata.
631+
// If file == nil, the fileListEntry describes a directory without metadata.
632632
type fileListEntry struct {
633-
name string
634-
file *File // nil for directories
633+
name string
634+
file *File
635+
isDir bool
635636
}
636637

637638
type fileInfoDirEntry interface {
@@ -640,20 +641,26 @@ type fileInfoDirEntry interface {
640641
}
641642

642643
func (e *fileListEntry) stat() fileInfoDirEntry {
643-
if e.file != nil {
644+
if !e.isDir {
644645
return headerFileInfo{&e.file.FileHeader}
645646
}
646647
return e
647648
}
648649

649650
// Only used for directories.
650-
func (f *fileListEntry) Name() string { _, elem, _ := split(f.name); return elem }
651-
func (f *fileListEntry) Size() int64 { return 0 }
652-
func (f *fileListEntry) ModTime() time.Time { return time.Time{} }
653-
func (f *fileListEntry) Mode() fs.FileMode { return fs.ModeDir | 0555 }
654-
func (f *fileListEntry) Type() fs.FileMode { return fs.ModeDir }
655-
func (f *fileListEntry) IsDir() bool { return true }
656-
func (f *fileListEntry) Sys() interface{} { return nil }
651+
func (f *fileListEntry) Name() string { _, elem, _ := split(f.name); return elem }
652+
func (f *fileListEntry) Size() int64 { return 0 }
653+
func (f *fileListEntry) Mode() fs.FileMode { return fs.ModeDir | 0555 }
654+
func (f *fileListEntry) Type() fs.FileMode { return fs.ModeDir }
655+
func (f *fileListEntry) IsDir() bool { return true }
656+
func (f *fileListEntry) Sys() interface{} { return nil }
657+
658+
func (f *fileListEntry) ModTime() time.Time {
659+
if f.file == nil {
660+
return time.Time{}
661+
}
662+
return f.file.FileHeader.Modified.UTC()
663+
}
657664

658665
func (f *fileListEntry) Info() (fs.FileInfo, error) { return f, nil }
659666

@@ -673,15 +680,32 @@ func toValidName(name string) string {
673680
func (r *Reader) initFileList() {
674681
r.fileListOnce.Do(func() {
675682
dirs := make(map[string]bool)
683+
knownDirs := make(map[string]bool)
676684
for _, file := range r.File {
685+
isDir := len(file.Name) > 0 && file.Name[len(file.Name)-1] == '/'
677686
name := toValidName(file.Name)
678687
for dir := path.Dir(name); dir != "."; dir = path.Dir(dir) {
679688
dirs[dir] = true
680689
}
681-
r.fileList = append(r.fileList, fileListEntry{name, file})
690+
entry := fileListEntry{
691+
name: name,
692+
file: file,
693+
isDir: isDir,
694+
}
695+
r.fileList = append(r.fileList, entry)
696+
if isDir {
697+
knownDirs[name] = true
698+
}
682699
}
683700
for dir := range dirs {
684-
r.fileList = append(r.fileList, fileListEntry{dir + "/", nil})
701+
if !knownDirs[dir] {
702+
entry := fileListEntry{
703+
name: dir,
704+
file: nil,
705+
isDir: true,
706+
}
707+
r.fileList = append(r.fileList, entry)
708+
}
685709
}
686710

687711
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) {
705729
if e == nil || !fs.ValidPath(name) {
706730
return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist}
707731
}
708-
if e.file == nil || strings.HasSuffix(e.file.Name, "/") {
732+
if e.isDir {
709733
return &openDir{e, r.openReadDir(name), 0}, nil
710734
}
711735
rc, err := e.file.Open()
@@ -730,7 +754,7 @@ func split(name string) (dir, elem string, isDir bool) {
730754
return name[:i], name[i+1:], isDir
731755
}
732756

733-
var dotFile = &fileListEntry{name: "./"}
757+
var dotFile = &fileListEntry{name: "./", isDir: true}
734758

735759
func (r *Reader) openLookup(name string) *fileListEntry {
736760
if name == "." {

‎src/archive/zip/reader_test.go‎

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1073,12 +1073,62 @@ func TestIssue12449(t *testing.T) {
10731073
}
10741074

10751075
func TestFS(t *testing.T) {
1076-
z, err := OpenReader("testdata/unix.zip")
1076+
for _, test := range []struct {
1077+
file string
1078+
want []string
1079+
}{
1080+
{
1081+
"testdata/unix.zip",
1082+
[]string{"hello", "dir/bar", "readonly"},
1083+
},
1084+
{
1085+
"testdata/subdir.zip",
1086+
[]string{"a/b/c"},
1087+
},
1088+
} {
1089+
t.Run(test.file, func(t *testing.T) {
1090+
t.Parallel()
1091+
z, err := OpenReader(test.file)
1092+
if err != nil {
1093+
t.Fatal(err)
1094+
}
1095+
defer z.Close()
1096+
if err := fstest.TestFS(z, test.want...); err != nil {
1097+
t.Error(err)
1098+
}
1099+
})
1100+
}
1101+
}
1102+
1103+
func TestFSModTime(t *testing.T) {
1104+
t.Parallel()
1105+
z, err := OpenReader("testdata/subdir.zip")
10771106
if err != nil {
10781107
t.Fatal(err)
10791108
}
1080-
if err := fstest.TestFS(z, "hello", "dir/bar", "dir/empty", "readonly"); err != nil {
1081-
t.Fatal(err)
1109+
defer z.Close()
1110+
1111+
for _, test := range []struct {
1112+
name string
1113+
want time.Time
1114+
}{
1115+
{
1116+
"a",
1117+
time.Date(2021, 4, 19, 12, 29, 56, 0, timeZone(-7*time.Hour)).UTC(),
1118+
},
1119+
{
1120+
"a/b/c",
1121+
time.Date(2021, 4, 19, 12, 29, 59, 0, timeZone(-7*time.Hour)).UTC(),
1122+
},
1123+
} {
1124+
fi, err := fs.Stat(z, test.name)
1125+
if err != nil {
1126+
t.Errorf("%s: %v", test.name, err)
1127+
continue
1128+
}
1129+
if got := fi.ModTime(); !got.Equal(test.want) {
1130+
t.Errorf("%s: got modtime %v, want %v", test.name, got, test.want)
1131+
}
10821132
}
10831133
}
10841134

‎src/archive/zip/testdata/subdir.zip‎

428 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)