From d4da735091986868015369e01c63794af9cc9b84 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Mon, 6 Jul 2020 09:49:20 -0400 Subject: io/fs: move FileInfo, FileMode, PathError, ErrInvalid, ... from os to io/fs First step of creating the new io/fs package. For #41190. Change-Id: I1339b1abdd533b0f1deab283628088b2f706fb5b Reviewed-on: https://go-review.googlesource.com/c/go/+/243906 Trust: Russ Cox Run-TryBot: Russ Cox Run-TryBot: Emmanuel Odeke TryBot-Result: Go Bot Reviewed-by: Rob Pike Reviewed-by: Emmanuel Odeke --- src/io/fs/fs.go | 140 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 src/io/fs/fs.go (limited to 'src/io/fs') diff --git a/src/io/fs/fs.go b/src/io/fs/fs.go new file mode 100644 index 0000000000..de5c465d9d --- /dev/null +++ b/src/io/fs/fs.go @@ -0,0 +1,140 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package fs defines basic interfaces to a file system. +// A file system can be provided by the host operating system +// but also by other packages. +package fs + +import ( + "internal/oserror" + "time" +) + +// Generic file system errors. +// Errors returned by file systems can be tested against these errors +// using errors.Is. +var ( + ErrInvalid = errInvalid() // "invalid argument" + ErrPermission = errPermission() // "permission denied" + ErrExist = errExist() // "file already exists" + ErrNotExist = errNotExist() // "file does not exist" + ErrClosed = errClosed() // "file already closed" +) + +func errInvalid() error { return oserror.ErrInvalid } +func errPermission() error { return oserror.ErrPermission } +func errExist() error { return oserror.ErrExist } +func errNotExist() error { return oserror.ErrNotExist } +func errClosed() error { return oserror.ErrClosed } + +// A FileInfo describes a file and is returned by Stat. +type FileInfo interface { + Name() string // base name of the file + Size() int64 // length in bytes for regular files; system-dependent for others + Mode() FileMode // file mode bits + ModTime() time.Time // modification time + IsDir() bool // abbreviation for Mode().IsDir() + Sys() interface{} // underlying data source (can return nil) +} + +// A FileMode represents a file's mode and permission bits. +// The bits have the same definition on all systems, so that +// information about files can be moved from one system +// to another portably. Not all bits apply to all systems. +// The only required bit is ModeDir for directories. +type FileMode uint32 + +// The defined file mode bits are the most significant bits of the FileMode. +// The nine least-significant bits are the standard Unix rwxrwxrwx permissions. +// The values of these bits should be considered part of the public API and +// may be used in wire protocols or disk representations: they must not be +// changed, although new bits might be added. +const ( + // The single letters are the abbreviations + // used by the String method's formatting. + ModeDir FileMode = 1 << (32 - 1 - iota) // d: is a directory + ModeAppend // a: append-only + ModeExclusive // l: exclusive use + ModeTemporary // T: temporary file; Plan 9 only + ModeSymlink // L: symbolic link + ModeDevice // D: device file + ModeNamedPipe // p: named pipe (FIFO) + ModeSocket // S: Unix domain socket + ModeSetuid // u: setuid + ModeSetgid // g: setgid + ModeCharDevice // c: Unix character device, when ModeDevice is set + ModeSticky // t: sticky + ModeIrregular // ?: non-regular file; nothing else is known about this file + + // Mask for the type bits. For regular files, none will be set. + ModeType = ModeDir | ModeSymlink | ModeNamedPipe | ModeSocket | ModeDevice | ModeCharDevice | ModeIrregular + + ModePerm FileMode = 0777 // Unix permission bits +) + +func (m FileMode) String() string { + const str = "dalTLDpSugct?" + var buf [32]byte // Mode is uint32. + w := 0 + for i, c := range str { + if m&(1< Date: Mon, 6 Jul 2020 09:56:43 -0400 Subject: io/fs: add FS, File, ReadDirFile; move DirEntry from os These are the core interfaces for the io/fs design. See #41190 and https://golang.org/s/draft-iofs-design for details. DirEntry was left behind in the previous move from os but is needed for ReadDirFile, so it moves in this commit. Also apply a couple comment changes suggested in the review of CL 261540. For #41190. Change-Id: I087741545139ed30b9ba5db728a0bad71129500b Reviewed-on: https://go-review.googlesource.com/c/go/+/243908 Trust: Russ Cox Run-TryBot: Russ Cox Reviewed-by: Emmanuel Odeke Reviewed-by: Rob Pike TryBot-Result: Go Bot --- src/io/fs/fs.go | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/io/fs/fs_test.go | 48 ++++++++++++++++++++++ src/os/dir.go | 24 ++--------- 3 files changed, 163 insertions(+), 21 deletions(-) create mode 100644 src/io/fs/fs_test.go (limited to 'src/io/fs') diff --git a/src/io/fs/fs.go b/src/io/fs/fs.go index de5c465d9d..d9f89fc6ee 100644 --- a/src/io/fs/fs.go +++ b/src/io/fs/fs.go @@ -12,6 +12,118 @@ import ( "time" ) +// An FS provides access to a hierarchical file system. +// +// The FS interface is the minimum implementation required of the file system. +// A file system may implement additional interfaces, +// such as fsutil.ReadFileFS, to provide additional or optimized functionality. +// See io/fsutil for details. +type FS interface { + // Open opens the named file. + // + // When Open returns an error, it should be of type *PathError + // with the Op field set to "open", the Path field set to name, + // and the Err field describing the problem. + // + // Open should reject attempts to open names that do not satisfy + // ValidPath(name), returning a *PathError with Err set to + // ErrInvalid or ErrNotExist. + Open(name string) (File, error) +} + +// ValidPath reports whether the given path name +// is valid for use in a call to Open. +// Path names passed to open are unrooted, slash-separated +// sequences of path elements, like “x/y/z”. +// Path names must not contain a “.” or “..” or empty element, +// except for the special case that the root directory is named “.”. +// +// Paths are slash-separated on all systems, even Windows. +// Backslashes must not appear in path names. +func ValidPath(name string) bool { + if name == "." { + // special case + return true + } + + // Iterate over elements in name, checking each. + for { + i := 0 + for i < len(name) && name[i] != '/' { + if name[i] == '\\' { + return false + } + i++ + } + elem := name[:i] + if elem == "" || elem == "." || elem == ".." { + return false + } + if i == len(name) { + return true // reached clean ending + } + name = name[i+1:] + } +} + +// A File provides access to a single file. +// The File interface is the minimum implementation required of the file. +// A file may implement additional interfaces, such as +// ReadDirFile, ReaderAt, or Seeker, to provide additional or optimized functionality. +type File interface { + Stat() (FileInfo, error) + Read([]byte) (int, error) + Close() error +} + +// A DirEntry is an entry read from a directory +// (using the ReadDir function or a ReadDirFile's ReadDir method). +type DirEntry interface { + // Name returns the name of the file (or subdirectory) described by the entry. + // This name is only the final element of the path (the base name), not the entire path. + // For example, Name would return "hello.go" not "/home/gopher/hello.go". + Name() string + + // IsDir reports whether the entry describes a directory. + IsDir() bool + + // Type returns the type bits for the entry. + // The type bits are a subset of the usual FileMode bits, those returned by the FileMode.Type method. + Type() FileMode + + // Info returns the FileInfo for the file or subdirectory described by the entry. + // The returned FileInfo may be from the time of the original directory read + // or from the time of the call to Info. If the file has been removed or renamed + // since the directory read, Info may return an error satisfying errors.Is(err, ErrNotExist). + // If the entry denotes a symbolic link, Info reports the information about the link itself, + // not the link's target. + Info() (FileInfo, error) +} + +// A ReadDirFile is a directory file whose entries can be read with the ReadDir method. +// Every directory file should implement this interface. +// (It is permissible for any file to implement this interface, +// but if so ReadDir should return an error for non-directories.) +type ReadDirFile interface { + File + + // ReadDir reads the contents of the directory and returns + // a slice of up to n DirEntry values in directory order. + // Subsequent calls on the same file will yield further DirEntry values. + // + // If n > 0, ReadDir returns at most n DirEntry structures. + // In this case, if ReadDir returns an empty slice, it will return + // a non-nil error explaining why. + // At the end of a directory, the error is io.EOF. + // + // If n <= 0, ReadDir returns all the DirEntry values from the directory + // in a single slice. In this case, if ReadDir succeeds (reads all the way + // to the end of the directory), it returns the slice and a nil error. + // If it encounters an error before the end of the directory, + // ReadDir returns the DirEntry list read until that point and a non-nil error. + ReadDir(n int) ([]DirEntry, error) +} + // Generic file system errors. // Errors returned by file systems can be tested against these errors // using errors.Is. diff --git a/src/io/fs/fs_test.go b/src/io/fs/fs_test.go new file mode 100644 index 0000000000..8d395fc0db --- /dev/null +++ b/src/io/fs/fs_test.go @@ -0,0 +1,48 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package fs_test + +import ( + . "io/fs" + "testing" +) + +var isValidPathTests = []struct { + name string + ok bool +}{ + {".", true}, + {"x", true}, + {"x/y", true}, + + {"", false}, + {"..", false}, + {"/", false}, + {"x/", false}, + {"/x", false}, + {"x/y/", false}, + {"/x/y", false}, + {"./", false}, + {"./x", false}, + {"x/.", false}, + {"x/./y", false}, + {"../", false}, + {"../x", false}, + {"x/..", false}, + {"x/../y", false}, + {"x//y", false}, + {`x\`, false}, + {`x\y`, false}, + {`\x`, false}, +} + +func TestValidPath(t *testing.T) { + for _, tt := range isValidPathTests { + ok := ValidPath(tt.name) + if ok != tt.ok { + t.Errorf("ValidPath(%q) = %v, want %v", tt.name, ok, tt.ok) + } + } +} diff --git a/src/os/dir.go b/src/os/dir.go index a312001704..b56d998459 100644 --- a/src/os/dir.go +++ b/src/os/dir.go @@ -4,6 +4,8 @@ package os +import "io/fs" + type readdirMode int const ( @@ -62,27 +64,7 @@ func (f *File) Readdirnames(n int) (names []string, err error) { // A DirEntry is an entry read from a directory // (using the ReadDir function or a File's ReadDir method). -type DirEntry interface { - // Name returns the name of the file (or subdirectory) described by the entry. - // This name is only the final element of the path, not the entire path. - // For example, Name would return "hello.go" not "/home/gopher/hello.go". - Name() string - - // IsDir reports whether the entry describes a subdirectory. - IsDir() bool - - // Type returns the type bits for the entry. - // The type bits are a subset of the usual FileMode bits, those returned by the FileMode.Type method. - Type() FileMode - - // Info returns the FileInfo for the file or subdirectory described by the entry. - // The returned FileInfo may be from the time of the original directory read - // or from the time of the call to Info. If the file has been removed or renamed - // since the directory read, Info may return an error satisfying errors.Is(err, ErrNotExist). - // If the entry denotes a symbolic link, Info reports the information about the link itself, - // not the link's target. - Info() (FileInfo, error) -} +type DirEntry = fs.DirEntry // ReadDir reads the contents of the directory associated with the file f // and returns a slice of DirEntry values in directory order. -- cgit v1.3 From f098ccf04a33e2e4d2dffa2e90fe77ca8a0fcbb4 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Mon, 6 Jul 2020 11:26:26 -0400 Subject: io/fs: add ReadFile and ReadFileFS Add ReadFile helper function, ReadFileFS interface, and test. Add ReadFile method to fstest.MapFS. Add testing of ReadFile method to fstest.TestFS. For #41190. Change-Id: I5b6a41e2e582824e570463b698b635abaa436c32 Reviewed-on: https://go-review.googlesource.com/c/go/+/243912 Trust: Russ Cox Reviewed-by: Rob Pike --- src/go/build/deps_test.go | 4 ++- src/io/fs/readfile.go | 63 ++++++++++++++++++++++++++++++++++++++++++++ src/io/fs/readfile_test.go | 43 ++++++++++++++++++++++++++++++ src/testing/fstest/mapfs.go | 12 +++++++++ src/testing/fstest/testfs.go | 39 ++++++++++++++++++++++++--- 5 files changed, 156 insertions(+), 5 deletions(-) create mode 100644 src/io/fs/readfile.go create mode 100644 src/io/fs/readfile_test.go (limited to 'src/io/fs') diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go index 4867a5031a..ccee539086 100644 --- a/src/go/build/deps_test.go +++ b/src/go/build/deps_test.go @@ -165,7 +165,9 @@ var depsRules = ` os/signal, STR < path/filepath - < io/ioutil, os/exec + < io/ioutil, os/exec; + + io/ioutil, os/exec, os/signal < OS; reflect !< OS; diff --git a/src/io/fs/readfile.go b/src/io/fs/readfile.go new file mode 100644 index 0000000000..7ee9eadac4 --- /dev/null +++ b/src/io/fs/readfile.go @@ -0,0 +1,63 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package fs + +import "io" + +// ReadFileFS is the interface implemented by a file system +// that provides an optimized implementation of ReadFile. +type ReadFileFS interface { + FS + + // ReadFile reads the named file and returns its contents. + // A successful call returns a nil error, not io.EOF. + // (Because ReadFile reads the whole file, the expected EOF + // from the final Read is not treated as an error to be reported.) + ReadFile(name string) ([]byte, error) +} + +// ReadFile reads the named file from the file system fs and returns its contents. +// A successful call returns a nil error, not io.EOF. +// (Because ReadFile reads the whole file, the expected EOF +// from the final Read is not treated as an error to be reported.) +// +// If fs implements ReadFileFS, ReadFile calls fs.ReadFile. +// Otherwise ReadFile calls fs.Open and uses Read and Close +// on the returned file. +func ReadFile(fsys FS, name string) ([]byte, error) { + if fsys, ok := fsys.(ReadFileFS); ok { + return fsys.ReadFile(name) + } + + file, err := fsys.Open(name) + if err != nil { + return nil, err + } + defer file.Close() + + var size int + if info, err := file.Stat(); err == nil { + size64 := info.Size() + if int64(int(size64)) == size64 { + size = int(size64) + } + } + + data := make([]byte, 0, size+1) + for { + if len(data) >= cap(data) { + d := append(data[:cap(data)], 0) + data = d[:len(data)] + } + n, err := file.Read(data[len(data):cap(data)]) + data = data[:len(data)+n] + if err != nil { + if err == io.EOF { + err = nil + } + return data, err + } + } +} diff --git a/src/io/fs/readfile_test.go b/src/io/fs/readfile_test.go new file mode 100644 index 0000000000..0afa334ace --- /dev/null +++ b/src/io/fs/readfile_test.go @@ -0,0 +1,43 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package fs_test + +import ( + . "io/fs" + "testing" + "testing/fstest" + "time" +) + +var testFsys = fstest.MapFS{ + "hello.txt": { + Data: []byte("hello, world"), + Mode: 0456, + ModTime: time.Now(), + Sys: &sysValue, + }, +} + +var sysValue int + +type readFileOnly struct{ ReadFileFS } + +func (readFileOnly) Open(name string) (File, error) { return nil, ErrNotExist } + +type openOnly struct{ FS } + +func TestReadFile(t *testing.T) { + // Test that ReadFile uses the method when present. + data, err := ReadFile(readFileOnly{testFsys}, "hello.txt") + if string(data) != "hello, world" || err != nil { + t.Fatalf(`ReadFile(readFileOnly, "hello.txt") = %q, %v, want %q, nil`, data, err, "hello, world") + } + + // Test that ReadFile uses Open when the method is not present. + data, err = ReadFile(openOnly{testFsys}, "hello.txt") + if string(data) != "hello, world" || err != nil { + t.Fatalf(`ReadFile(openOnly, "hello.txt") = %q, %v, want %q, nil`, data, err, "hello, world") + } +} diff --git a/src/testing/fstest/mapfs.go b/src/testing/fstest/mapfs.go index 84a943f409..e969ac2bd1 100644 --- a/src/testing/fstest/mapfs.go +++ b/src/testing/fstest/mapfs.go @@ -108,6 +108,18 @@ func (fsys MapFS) Open(name string) (fs.File, error) { return &mapDir{name, mapFileInfo{elem, file}, list, 0}, nil } +// fsOnly is a wrapper that hides all but the fs.FS methods, +// to avoid an infinite recursion when implementing special +// methods in terms of helpers that would use them. +// (In general, implementing these methods using the package fs helpers +// is redundant and unnecessary, but having the methods may make +// MapFS exercise more code paths when used in tests.) +type fsOnly struct{ fs.FS } + +func (fsys MapFS) ReadFile(name string) ([]byte, error) { + return fs.ReadFile(fsOnly{fsys}, name) +} + // A mapFileInfo implements fs.FileInfo and fs.DirEntry for a given map file. type mapFileInfo struct { name string diff --git a/src/testing/fstest/testfs.go b/src/testing/fstest/testfs.go index 2bb2120c19..66725ca2a4 100644 --- a/src/testing/fstest/testfs.go +++ b/src/testing/fstest/testfs.go @@ -310,6 +310,27 @@ func (t *fsTester) checkFile(file string) { // The return value doesn't matter. f.Close() + // Check that ReadFile works if present. + if fsys, ok := t.fsys.(fs.ReadFileFS); ok { + data2, err := fsys.ReadFile(file) + if err != nil { + t.errorf("%s: fsys.ReadFile: %v", file, err) + return + } + t.checkFileRead(file, "ReadAll vs fsys.ReadFile", data, data2) + + t.checkBadPath(file, "ReadFile", + func(name string) error { _, err := fsys.ReadFile(name); return err }) + } + + // Check that fs.ReadFile works with t.fsys. + data2, err := fs.ReadFile(t.fsys, file) + if err != nil { + t.errorf("%s: fs.ReadFile: %v", file, err) + return + } + t.checkFileRead(file, "ReadAll vs fs.ReadFile", data, data2) + // Use iotest.TestReader to check small reads, Seek, ReadAt. f, err = t.fsys.Open(file) if err != nil { @@ -329,8 +350,19 @@ func (t *fsTester) checkFileRead(file, desc string, data1, data2 []byte) { } } -// checkOpen checks that various invalid forms of file's name cannot be opened. +// checkBadPath checks that various invalid forms of file's name cannot be opened using t.fsys.Open. func (t *fsTester) checkOpen(file string) { + t.checkBadPath(file, "Open", func(file string) error { + f, err := t.fsys.Open(file) + if err == nil { + f.Close() + } + return err + }) +} + +// checkBadPath checks that various invalid forms of file's name cannot be opened using open. +func (t *fsTester) checkBadPath(file string, desc string, open func(string) error) { bad := []string{ "/" + file, file + "/.", @@ -356,9 +388,8 @@ func (t *fsTester) checkOpen(file string) { } for _, b := range bad { - if f, err := t.fsys.Open(b); err == nil { - f.Close() - t.errorf("%s: Open(%s) succeeded, want error", file, b) + if err := open(b); err == nil { + t.errorf("%s: %s(%s) succeeded, want error", file, desc, b) } } } -- cgit v1.3 From 10a1a1a37c007adef8425d273e6b276547982889 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Mon, 6 Jul 2020 11:26:45 -0400 Subject: io/fs: add Stat and StatFS Add Stat helper function, StatFS interface, and test. Add Stat method to fstest.MapFS. Add testing of Stat method to fstest.TestFS. For #41190. Change-Id: Icf8b6eb1c3fa6f93a9be8405ec5a9468fb1da97b Reviewed-on: https://go-review.googlesource.com/c/go/+/243913 Trust: Russ Cox Reviewed-by: Rob Pike --- src/io/fs/stat.go | 31 +++++++++++++++++++++++++++++++ src/io/fs/stat_test.go | 36 ++++++++++++++++++++++++++++++++++++ src/testing/fstest/mapfs.go | 4 ++++ src/testing/fstest/testfs.go | 25 ++++++++++++++++++++++++- 4 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 src/io/fs/stat.go create mode 100644 src/io/fs/stat_test.go (limited to 'src/io/fs') diff --git a/src/io/fs/stat.go b/src/io/fs/stat.go new file mode 100644 index 0000000000..735a6e3281 --- /dev/null +++ b/src/io/fs/stat.go @@ -0,0 +1,31 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package fs + +// A StatFS is a file system with a Stat method. +type StatFS interface { + FS + + // Stat returns a FileInfo describing the file. + // If there is an error, it should be of type *PathError. + Stat(name string) (FileInfo, error) +} + +// Stat returns a FileInfo describing the named file from the file system. +// +// If fs implements StatFS, Stat calls fs.Stat. +// Otherwise, Stat opens the file to stat it. +func Stat(fsys FS, name string) (FileInfo, error) { + if fsys, ok := fsys.(StatFS); ok { + return fsys.Stat(name) + } + + file, err := fsys.Open(name) + if err != nil { + return nil, err + } + defer file.Close() + return file.Stat() +} diff --git a/src/io/fs/stat_test.go b/src/io/fs/stat_test.go new file mode 100644 index 0000000000..e312b6fbd9 --- /dev/null +++ b/src/io/fs/stat_test.go @@ -0,0 +1,36 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package fs_test + +import ( + "fmt" + . "io/fs" + "testing" +) + +type statOnly struct{ StatFS } + +func (statOnly) Open(name string) (File, error) { return nil, ErrNotExist } + +func TestStat(t *testing.T) { + check := func(desc string, info FileInfo, err error) { + t.Helper() + if err != nil || info == nil || info.Mode() != 0456 { + infoStr := "" + if info != nil { + infoStr = fmt.Sprintf("FileInfo(Mode: %#o)", info.Mode()) + } + t.Fatalf("Stat(%s) = %v, %v, want Mode:0456, nil", desc, infoStr, err) + } + } + + // Test that Stat uses the method when present. + info, err := Stat(statOnly{testFsys}, "hello.txt") + check("statOnly", info, err) + + // Test that Stat uses Open when the method is not present. + info, err = Stat(openOnly{testFsys}, "hello.txt") + check("openOnly", info, err) +} diff --git a/src/testing/fstest/mapfs.go b/src/testing/fstest/mapfs.go index e969ac2bd1..b01911e589 100644 --- a/src/testing/fstest/mapfs.go +++ b/src/testing/fstest/mapfs.go @@ -120,6 +120,10 @@ func (fsys MapFS) ReadFile(name string) ([]byte, error) { return fs.ReadFile(fsOnly{fsys}, name) } +func (fsys MapFS) Stat(name string) (fs.FileInfo, error) { + return fs.Stat(fsOnly{fsys}, name) +} + // A mapFileInfo implements fs.FileInfo and fs.DirEntry for a given map file. type mapFileInfo struct { name string diff --git a/src/testing/fstest/testfs.go b/src/testing/fstest/testfs.go index 66725ca2a4..290d2596cc 100644 --- a/src/testing/fstest/testfs.go +++ b/src/testing/fstest/testfs.go @@ -230,7 +230,30 @@ func (t *fsTester) checkStat(path string, entry fs.DirEntry) { fentry := formatEntry(entry) finfo := formatInfoEntry(info) if fentry != finfo { - t.errorf("%s: mismatch:\n\tentry = %v\n\tfile.Stat() = %v", path, fentry, finfo) + t.errorf("%s: mismatch:\n\tentry = %s\n\tfile.Stat() = %s", path, fentry, finfo) + } + + info2, err := fs.Stat(t.fsys, path) + if err != nil { + t.errorf("%s: fs.Stat: %v", path, err) + return + } + finfo = formatInfo(info) + finfo2 := formatInfo(info2) + if finfo2 != finfo { + t.errorf("%s: fs.Stat(...) = %s\n\twant %s", path, finfo2, finfo) + } + + if fsys, ok := t.fsys.(fs.StatFS); ok { + info2, err := fsys.Stat(path) + if err != nil { + t.errorf("%s: fsys.Stat: %v", path, err) + return + } + finfo2 := formatInfo(info2) + if finfo2 != finfo { + t.errorf("%s: fsys.Stat(...) = %s\n\twant %s", path, finfo2, finfo) + } } } -- cgit v1.3 From 7a131acfd142f0fc7612365078b9f00e371fc0e2 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Mon, 6 Jul 2020 11:26:56 -0400 Subject: io/fs: add ReadDir and ReadDirFS Add ReadDir helper function, ReadDirFS interface, and test. Add ReadDir method to fstest.MapFS. Add testing of ReadDir method to fstest.TestFS. For #41190. Change-Id: Ib860770ec7433ba77b29e626682b238f1b3bf54f Reviewed-on: https://go-review.googlesource.com/c/go/+/243914 Trust: Russ Cox Reviewed-by: Rob Pike --- src/io/fs/readdir.go | 47 ++++++++++++++++++++++++++++++++++++++++++++ src/io/fs/readdir_test.go | 35 +++++++++++++++++++++++++++++++++ src/testing/fstest/mapfs.go | 4 ++++ src/testing/fstest/testfs.go | 42 ++++++++++++++++++++++++++++++++++++++- 4 files changed, 127 insertions(+), 1 deletion(-) create mode 100644 src/io/fs/readdir.go create mode 100644 src/io/fs/readdir_test.go (limited to 'src/io/fs') diff --git a/src/io/fs/readdir.go b/src/io/fs/readdir.go new file mode 100644 index 0000000000..3a5aa6d86a --- /dev/null +++ b/src/io/fs/readdir.go @@ -0,0 +1,47 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package fs + +import ( + "errors" + "sort" +) + +// ReadDirFS is the interface implemented by a file system +// that provides an optimized implementation of ReadDir. +type ReadDirFS interface { + FS + + // ReadDir reads the named directory + // and returns a list of directory entries sorted by filename. + ReadDir(name string) ([]DirEntry, error) +} + +// ReadDir reads the named directory +// and returns a list of directory entries sorted by filename. +// +// If fs implements ReadDirFS, ReadDir calls fs.ReadDir. +// Otherwise ReadDir calls fs.Open and uses ReadDir and Close +// on the returned file. +func ReadDir(fsys FS, name string) ([]DirEntry, error) { + if fsys, ok := fsys.(ReadDirFS); ok { + return fsys.ReadDir(name) + } + + file, err := fsys.Open(name) + if err != nil { + return nil, err + } + defer file.Close() + + dir, ok := file.(ReadDirFile) + if !ok { + return nil, &PathError{Op: "readdir", Path: name, Err: errors.New("not implemented")} + } + + list, err := dir.ReadDir(-1) + sort.Slice(list, func(i, j int) bool { return list[i].Name() < list[j].Name() }) + return list, err +} diff --git a/src/io/fs/readdir_test.go b/src/io/fs/readdir_test.go new file mode 100644 index 0000000000..46a4bc2788 --- /dev/null +++ b/src/io/fs/readdir_test.go @@ -0,0 +1,35 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package fs_test + +import ( + . "io/fs" + "testing" +) + +type readDirOnly struct{ ReadDirFS } + +func (readDirOnly) Open(name string) (File, error) { return nil, ErrNotExist } + +func TestReadDir(t *testing.T) { + check := func(desc string, dirs []DirEntry, err error) { + t.Helper() + if err != nil || len(dirs) != 1 || dirs[0].Name() != "hello.txt" { + var names []string + for _, d := range dirs { + names = append(names, d.Name()) + } + t.Errorf("ReadDir(%s) = %v, %v, want %v, nil", desc, names, err, []string{"hello.txt"}) + } + } + + // Test that ReadDir uses the method when present. + dirs, err := ReadDir(readDirOnly{testFsys}, ".") + check("readDirOnly", dirs, err) + + // Test that ReadDir uses Open when the method is not present. + dirs, err = ReadDir(openOnly{testFsys}, ".") + check("openOnly", dirs, err) +} diff --git a/src/testing/fstest/mapfs.go b/src/testing/fstest/mapfs.go index b01911e589..1eaf8f0040 100644 --- a/src/testing/fstest/mapfs.go +++ b/src/testing/fstest/mapfs.go @@ -124,6 +124,10 @@ func (fsys MapFS) Stat(name string) (fs.FileInfo, error) { return fs.Stat(fsOnly{fsys}, name) } +func (fsys MapFS) ReadDir(name string) ([]fs.DirEntry, error) { + return fs.ReadDir(fsOnly{fsys}, name) +} + // A mapFileInfo implements fs.FileInfo and fs.DirEntry for a given map file. type mapFileInfo struct { name string diff --git a/src/testing/fstest/testfs.go b/src/testing/fstest/testfs.go index 290d2596cc..4ea6ed6095 100644 --- a/src/testing/fstest/testfs.go +++ b/src/testing/fstest/testfs.go @@ -196,6 +196,36 @@ func (t *fsTester) checkDir(dir string) { } } t.checkDirList(dir, "first Open+ReadDir(-1) vs third Open+ReadDir(1,2) loop", list, list2) + + // If fsys has ReadDir, check that it matches and is sorted. + if fsys, ok := t.fsys.(fs.ReadDirFS); ok { + list2, err := fsys.ReadDir(dir) + if err != nil { + t.errorf("%s: fsys.ReadDir: %v", dir, err) + return + } + t.checkDirList(dir, "first Open+ReadDir(-1) vs fsys.ReadDir", list, list2) + + for i := 0; i+1 < len(list2); i++ { + if list2[i].Name() >= list2[i+1].Name() { + t.errorf("%s: fsys.ReadDir: list not sorted: %s before %s", dir, list2[i].Name(), list2[i+1].Name()) + } + } + } + + // Check fs.ReadDir as well. + list2, err = fs.ReadDir(t.fsys, dir) + if err != nil { + t.errorf("%s: fs.ReadDir: %v", dir, err) + return + } + t.checkDirList(dir, "first Open+ReadDir(-1) vs fs.ReadDir", list, list2) + + for i := 0; i+1 < len(list2); i++ { + if list2[i].Name() >= list2[i+1].Name() { + t.errorf("%s: fs.ReadDir: list not sorted: %s before %s", dir, list2[i].Name(), list2[i+1].Name()) + } + } } // formatEntry formats an fs.DirEntry into a string for error messages and comparison. @@ -233,12 +263,22 @@ func (t *fsTester) checkStat(path string, entry fs.DirEntry) { t.errorf("%s: mismatch:\n\tentry = %s\n\tfile.Stat() = %s", path, fentry, finfo) } + einfo, err := entry.Info() + if err != nil { + t.errorf("%s: entry.Info: %v", path, err) + return + } + fentry = formatInfo(einfo) + finfo = formatInfo(info) + if fentry != finfo { + t.errorf("%s: mismatch:\n\tentry.Info() = %s\n\tfile.Stat() = %s\n", path, fentry, finfo) + } + info2, err := fs.Stat(t.fsys, path) if err != nil { t.errorf("%s: fs.Stat: %v", path, err) return } - finfo = formatInfo(info) finfo2 := formatInfo(info2) if finfo2 != finfo { t.errorf("%s: fs.Stat(...) = %s\n\twant %s", path, finfo2, finfo) -- cgit v1.3 From b64202bc29b9c1cf0118878d1c0acc9cdb2308f6 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Mon, 6 Jul 2020 11:27:12 -0400 Subject: io/fs: add Glob and GlobFS Add Glob helper function, GlobFS interface, and test. Add Glob method to fstest.MapFS. Add testing of Glob method to fstest.TestFS. For #41190. Change-Id: If89dd7f63e310ba5ca2651340267a9ff39fcc0c7 Reviewed-on: https://go-review.googlesource.com/c/go/+/243915 Trust: Russ Cox Reviewed-by: Rob Pike --- src/go/build/deps_test.go | 2 +- src/io/fs/glob.go | 116 +++++++++++++++++++++++++++++++++++++++++++ src/io/fs/glob_test.go | 82 ++++++++++++++++++++++++++++++ src/testing/fstest/mapfs.go | 4 ++ src/testing/fstest/testfs.go | 96 +++++++++++++++++++++++++++++++++++ 5 files changed, 299 insertions(+), 1 deletion(-) create mode 100644 src/io/fs/glob.go create mode 100644 src/io/fs/glob_test.go (limited to 'src/io/fs') diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go index ccee539086..16a67791cf 100644 --- a/src/go/build/deps_test.go +++ b/src/go/build/deps_test.go @@ -123,7 +123,7 @@ var depsRules = ` < context < TIME; - TIME, io, sort + TIME, io, path, sort < io/fs; # MATH is RUNTIME plus the basic math packages. diff --git a/src/io/fs/glob.go b/src/io/fs/glob.go new file mode 100644 index 0000000000..77f6ebbaaf --- /dev/null +++ b/src/io/fs/glob.go @@ -0,0 +1,116 @@ +// Copyright 2010 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package fs + +import ( + "path" + "runtime" +) + +// A GlobFS is a file system with a Glob method. +type GlobFS interface { + FS + + // Glob returns the names of all files matching pattern, + // providing an implementation of the top-level + // Glob function. + Glob(pattern string) ([]string, error) +} + +// Glob returns the names of all files matching pattern or nil +// if there is no matching file. The syntax of patterns is the same +// as in path.Match. The pattern may describe hierarchical names such as +// /usr/*/bin/ed (assuming the Separator is '/'). +// +// Glob ignores file system errors such as I/O errors reading directories. +// The only possible returned error is path.ErrBadPattern, reporting that +// the pattern is malformed. +// +// If fs implements GlobFS, Glob calls fs.Glob. +// Otherwise, Glob uses ReadDir to traverse the directory tree +// and look for matches for the pattern. +func Glob(fsys FS, pattern string) (matches []string, err error) { + if fsys, ok := fsys.(GlobFS); ok { + return fsys.Glob(pattern) + } + + if !hasMeta(pattern) { + if _, err = Stat(fsys, pattern); err != nil { + return nil, nil + } + return []string{pattern}, nil + } + + dir, file := path.Split(pattern) + dir = cleanGlobPath(dir) + + if !hasMeta(dir) { + return glob(fsys, dir, file, nil) + } + + // Prevent infinite recursion. See issue 15879. + if dir == pattern { + return nil, path.ErrBadPattern + } + + var m []string + m, err = Glob(fsys, dir) + if err != nil { + return + } + for _, d := range m { + matches, err = glob(fsys, d, file, matches) + if err != nil { + return + } + } + return +} + +// cleanGlobPath prepares path for glob matching. +func cleanGlobPath(path string) string { + switch path { + case "": + return "." + default: + return path[0 : len(path)-1] // chop off trailing separator + } +} + +// glob searches for files matching pattern in the directory dir +// and appends them to matches, returning the updated slice. +// If the directory cannot be opened, glob returns the existing matches. +// New matches are added in lexicographical order. +func glob(fs FS, dir, pattern string, matches []string) (m []string, e error) { + m = matches + infos, err := ReadDir(fs, dir) + if err != nil { + return // ignore I/O error + } + + for _, info := range infos { + n := info.Name() + matched, err := path.Match(pattern, n) + if err != nil { + return m, err + } + if matched { + m = append(m, path.Join(dir, n)) + } + } + return +} + +// hasMeta reports whether path contains any of the magic characters +// recognized by path.Match. +func hasMeta(path string) bool { + for i := 0; i < len(path); i++ { + c := path[i] + if c == '*' || c == '?' || c == '[' || runtime.GOOS == "windows" && c == '\\' { + return true + } + } + return false +} diff --git a/src/io/fs/glob_test.go b/src/io/fs/glob_test.go new file mode 100644 index 0000000000..0183a49b6c --- /dev/null +++ b/src/io/fs/glob_test.go @@ -0,0 +1,82 @@ +// Copyright 2009 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package fs_test + +import ( + . "io/fs" + "os" + "testing" +) + +var globTests = []struct { + fs FS + pattern, result string +}{ + {os.DirFS("."), "glob.go", "glob.go"}, + {os.DirFS("."), "gl?b.go", "glob.go"}, + {os.DirFS("."), "*", "glob.go"}, + {os.DirFS(".."), "*/glob.go", "fs/glob.go"}, +} + +func TestGlob(t *testing.T) { + for _, tt := range globTests { + matches, err := Glob(tt.fs, tt.pattern) + if err != nil { + t.Errorf("Glob error for %q: %s", tt.pattern, err) + continue + } + if !contains(matches, tt.result) { + t.Errorf("Glob(%#q) = %#v want %v", tt.pattern, matches, tt.result) + } + } + for _, pattern := range []string{"no_match", "../*/no_match"} { + matches, err := Glob(os.DirFS("."), pattern) + if err != nil { + t.Errorf("Glob error for %q: %s", pattern, err) + continue + } + if len(matches) != 0 { + t.Errorf("Glob(%#q) = %#v want []", pattern, matches) + } + } +} + +func TestGlobError(t *testing.T) { + _, err := Glob(os.DirFS("."), "[]") + if err == nil { + t.Error("expected error for bad pattern; got none") + } +} + +// contains reports whether vector contains the string s. +func contains(vector []string, s string) bool { + for _, elem := range vector { + if elem == s { + return true + } + } + return false +} + +type globOnly struct{ GlobFS } + +func (globOnly) Open(name string) (File, error) { return nil, ErrNotExist } + +func TestGlobMethod(t *testing.T) { + check := func(desc string, names []string, err error) { + t.Helper() + if err != nil || len(names) != 1 || names[0] != "hello.txt" { + t.Errorf("Glob(%s) = %v, %v, want %v, nil", desc, names, err, []string{"hello.txt"}) + } + } + + // Test that ReadDir uses the method when present. + names, err := Glob(globOnly{testFsys}, "*.txt") + check("readDirOnly", names, err) + + // Test that ReadDir uses Open when the method is not present. + names, err = Glob(openOnly{testFsys}, "*.txt") + check("openOnly", names, err) +} diff --git a/src/testing/fstest/mapfs.go b/src/testing/fstest/mapfs.go index 1eaf8f0040..10e56f5b3c 100644 --- a/src/testing/fstest/mapfs.go +++ b/src/testing/fstest/mapfs.go @@ -128,6 +128,10 @@ func (fsys MapFS) ReadDir(name string) ([]fs.DirEntry, error) { return fs.ReadDir(fsOnly{fsys}, name) } +func (fsys MapFS) Glob(pattern string) ([]string, error) { + return fs.Glob(fsOnly{fsys}, pattern) +} + // A mapFileInfo implements fs.FileInfo and fs.DirEntry for a given map file. type mapFileInfo struct { name string diff --git a/src/testing/fstest/testfs.go b/src/testing/fstest/testfs.go index 4ea6ed6095..21cd00e5b6 100644 --- a/src/testing/fstest/testfs.go +++ b/src/testing/fstest/testfs.go @@ -11,6 +11,8 @@ import ( "io" "io/fs" "io/ioutil" + "path" + "reflect" "sort" "strings" "testing/iotest" @@ -226,6 +228,8 @@ func (t *fsTester) checkDir(dir string) { t.errorf("%s: fs.ReadDir: list not sorted: %s before %s", dir, list2[i].Name(), list2[i+1].Name()) } } + + t.checkGlob(dir, list) } // formatEntry formats an fs.DirEntry into a string for error messages and comparison. @@ -243,6 +247,98 @@ func formatInfo(info fs.FileInfo) string { return fmt.Sprintf("%s IsDir=%v Mode=%v Size=%d ModTime=%v", info.Name(), info.IsDir(), info.Mode(), info.Size(), info.ModTime()) } +// checkGlob checks that various glob patterns work if the file system implements GlobFS. +func (t *fsTester) checkGlob(dir string, list []fs.DirEntry) { + if _, ok := t.fsys.(fs.GlobFS); !ok { + return + } + + // Make a complex glob pattern prefix that only matches dir. + var glob string + if dir != "." { + elem := strings.Split(dir, "/") + for i, e := range elem { + var pattern []rune + for j, r := range e { + if r == '*' || r == '?' || r == '\\' || r == '[' { + pattern = append(pattern, '\\', r) + continue + } + switch (i + j) % 5 { + case 0: + pattern = append(pattern, r) + case 1: + pattern = append(pattern, '[', r, ']') + case 2: + pattern = append(pattern, '[', r, '-', r, ']') + case 3: + pattern = append(pattern, '[', '\\', r, ']') + case 4: + pattern = append(pattern, '[', '\\', r, '-', '\\', r, ']') + } + } + elem[i] = string(pattern) + } + glob = strings.Join(elem, "/") + "/" + } + + // Try to find a letter that appears in only some of the final names. + c := rune('a') + for ; c <= 'z'; c++ { + have, haveNot := false, false + for _, d := range list { + if strings.ContainsRune(d.Name(), c) { + have = true + } else { + haveNot = true + } + } + if have && haveNot { + break + } + } + if c > 'z' { + c = 'a' + } + glob += "*" + string(c) + "*" + + var want []string + for _, d := range list { + if strings.ContainsRune(d.Name(), c) { + want = append(want, path.Join(dir, d.Name())) + } + } + + names, err := t.fsys.(fs.GlobFS).Glob(glob) + if err != nil { + t.errorf("%s: Glob(%#q): %v", dir, glob, err) + return + } + if reflect.DeepEqual(want, names) { + return + } + + if !sort.StringsAreSorted(names) { + t.errorf("%s: Glob(%#q): unsorted output:\n%s", dir, glob, strings.Join(names, "\n")) + sort.Strings(names) + } + + var problems []string + for len(want) > 0 || len(names) > 0 { + switch { + case len(want) > 0 && len(names) > 0 && want[0] == names[0]: + want, names = want[1:], names[1:] + case len(want) > 0 && (len(names) == 0 || want[0] < names[0]): + problems = append(problems, "missing: "+want[0]) + want = want[1:] + default: + problems = append(problems, "extra: "+names[0]) + names = names[1:] + } + } + t.errorf("%s: Glob(%#q): wrong output:\n%s", dir, glob, strings.Join(problems, "\n")) +} + // checkStat checks that a direct stat of path matches entry, // which was found in the parent's directory listing. func (t *fsTester) checkStat(path string, entry fs.DirEntry) { -- cgit v1.3 From b5ddc42b465dd5b9532ee336d98343d81a6d35b2 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Thu, 22 Oct 2020 12:11:29 -0400 Subject: io/fs, path, path/filepath, testing/fstest: validate patterns in Match, Glob According to #28614, proposal review agreed in December 2018 that Match should return an error for failed matches where the unmatched part of the pattern has a syntax error. (The failed match has to date caused the scan of the pattern to stop early.) This change implements that behavior: the match loop continues scanning to the end of the pattern, even after a confirmed mismatch, to check whether the pattern is even well-formed. The change applies to both path.Match and filepath.Match. Then filepath.Glob and fs.Glob make a single validity-checking call to Match before beginning their usual processing. Also update fstest.TestFS to check for correct validation in custom Glob implementations. Fixes #28614. Change-Id: Ic1d35a4bb9c3565184ae83dbefc425c5c96318e7 Reviewed-on: https://go-review.googlesource.com/c/go/+/264397 Trust: Russ Cox Reviewed-by: Rob Pike --- src/io/fs/glob.go | 4 +++ src/io/fs/glob_test.go | 10 +++++-- src/path/filepath/match.go | 61 +++++++++++++++++++++++++---------------- src/path/filepath/match_test.go | 12 +++++--- src/path/match.go | 60 +++++++++++++++++++++++++++------------- src/path/match_test.go | 4 ++- src/testing/fstest/testfs.go | 6 ++++ 7 files changed, 106 insertions(+), 51 deletions(-) (limited to 'src/io/fs') diff --git a/src/io/fs/glob.go b/src/io/fs/glob.go index 77f6ebbaaf..cde6c49f3d 100644 --- a/src/io/fs/glob.go +++ b/src/io/fs/glob.go @@ -36,6 +36,10 @@ func Glob(fsys FS, pattern string) (matches []string, err error) { return fsys.Glob(pattern) } + // Check pattern is well-formed. + if _, err := path.Match(pattern, ""); err != nil { + return nil, err + } if !hasMeta(pattern) { if _, err = Stat(fsys, pattern); err != nil { return nil, nil diff --git a/src/io/fs/glob_test.go b/src/io/fs/glob_test.go index 0183a49b6c..5c8ac3fbf3 100644 --- a/src/io/fs/glob_test.go +++ b/src/io/fs/glob_test.go @@ -7,6 +7,7 @@ package fs_test import ( . "io/fs" "os" + "path" "testing" ) @@ -44,9 +45,12 @@ func TestGlob(t *testing.T) { } func TestGlobError(t *testing.T) { - _, err := Glob(os.DirFS("."), "[]") - if err == nil { - t.Error("expected error for bad pattern; got none") + bad := []string{`[]`, `nonexist/[]`} + for _, pattern := range bad { + _, err := Glob(os.DirFS("."), pattern) + if err != path.ErrBadPattern { + t.Errorf("Glob(fs, %#q) returned err=%v, want path.ErrBadPattern", pattern, err) + } } } diff --git a/src/path/filepath/match.go b/src/path/filepath/match.go index 20a334805b..c77a26952a 100644 --- a/src/path/filepath/match.go +++ b/src/path/filepath/match.go @@ -122,25 +122,28 @@ Scan: // If so, it returns the remainder of s (after the match). // Chunk is all single-character operators: literals, char classes, and ?. func matchChunk(chunk, s string) (rest string, ok bool, err error) { + // failed records whether the match has failed. + // After the match fails, the loop continues on processing chunk, + // checking that the pattern is well-formed but no longer reading s. + failed := false for len(chunk) > 0 { - if len(s) == 0 { - return + if !failed && len(s) == 0 { + failed = true } switch chunk[0] { case '[': // character class - r, n := utf8.DecodeRuneInString(s) - s = s[n:] - chunk = chunk[1:] - // We can't end right after '[', we're expecting at least - // a closing bracket and possibly a caret. - if len(chunk) == 0 { - err = ErrBadPattern - return + var r rune + if !failed { + var n int + r, n = utf8.DecodeRuneInString(s) + s = s[n:] } + chunk = chunk[1:] // possibly negated - negated := chunk[0] == '^' - if negated { + negated := false + if len(chunk) > 0 && chunk[0] == '^' { + negated = true chunk = chunk[1:] } // parse all ranges @@ -153,12 +156,12 @@ func matchChunk(chunk, s string) (rest string, ok bool, err error) { } var lo, hi rune if lo, chunk, err = getEsc(chunk); err != nil { - return + return "", false, err } hi = lo if chunk[0] == '-' { if hi, chunk, err = getEsc(chunk[1:]); err != nil { - return + return "", false, err } } if lo <= r && r <= hi { @@ -167,35 +170,41 @@ func matchChunk(chunk, s string) (rest string, ok bool, err error) { nrange++ } if match == negated { - return + failed = true } case '?': - if s[0] == Separator { - return + if !failed { + if s[0] == Separator { + failed = true + } + _, n := utf8.DecodeRuneInString(s) + s = s[n:] } - _, n := utf8.DecodeRuneInString(s) - s = s[n:] chunk = chunk[1:] case '\\': if runtime.GOOS != "windows" { chunk = chunk[1:] if len(chunk) == 0 { - err = ErrBadPattern - return + return "", false, ErrBadPattern } } fallthrough default: - if chunk[0] != s[0] { - return + if !failed { + if chunk[0] != s[0] { + failed = true + } + s = s[1:] } - s = s[1:] chunk = chunk[1:] } } + if failed { + return "", false, nil + } return s, true, nil } @@ -232,6 +241,10 @@ func getEsc(chunk string) (r rune, nchunk string, err error) { // The only possible returned error is ErrBadPattern, when pattern // is malformed. func Glob(pattern string) (matches []string, err error) { + // Check pattern is well-formed. + if _, err := Match(pattern, ""); err != nil { + return nil, err + } if !hasMeta(pattern) { if _, err = os.Lstat(pattern); err != nil { return nil, nil diff --git a/src/path/filepath/match_test.go b/src/path/filepath/match_test.go index b8657626bc..1c3b567fa3 100644 --- a/src/path/filepath/match_test.go +++ b/src/path/filepath/match_test.go @@ -75,8 +75,10 @@ var matchTests = []MatchTest{ {"[", "a", false, ErrBadPattern}, {"[^", "a", false, ErrBadPattern}, {"[^bc", "a", false, ErrBadPattern}, - {"a[", "a", false, nil}, + {"a[", "a", false, ErrBadPattern}, {"a[", "ab", false, ErrBadPattern}, + {"a[", "x", false, ErrBadPattern}, + {"a/b[", "x", false, ErrBadPattern}, {"*x", "xxx", true, nil}, } @@ -155,9 +157,11 @@ func TestGlob(t *testing.T) { } func TestGlobError(t *testing.T) { - _, err := Glob("[]") - if err == nil { - t.Error("expected error for bad pattern; got none") + bad := []string{`[]`, `nonexist/[]`} + for _, pattern := range bad { + if _, err := Glob(pattern); err != ErrBadPattern { + t.Errorf("Glob(%#q) returned err=%v, want ErrBadPattern", pattern, err) + } } } diff --git a/src/path/match.go b/src/path/match.go index 837eb8bb8b..918624c60e 100644 --- a/src/path/match.go +++ b/src/path/match.go @@ -75,6 +75,14 @@ Pattern: } } } + // Before returning false with no error, + // check that the remainder of the pattern is syntactically valid. + for len(pattern) > 0 { + _, chunk, pattern = scanChunk(pattern) + if _, _, err := matchChunk(chunk, ""); err != nil { + return false, err + } + } return false, nil } return len(name) == 0, nil @@ -114,20 +122,28 @@ Scan: // If so, it returns the remainder of s (after the match). // Chunk is all single-character operators: literals, char classes, and ?. func matchChunk(chunk, s string) (rest string, ok bool, err error) { + // failed records whether the match has failed. + // After the match fails, the loop continues on processing chunk, + // checking that the pattern is well-formed but no longer reading s. + failed := false for len(chunk) > 0 { - if len(s) == 0 { - return + if !failed && len(s) == 0 { + failed = true } switch chunk[0] { case '[': // character class - r, n := utf8.DecodeRuneInString(s) - s = s[n:] + var r rune + if !failed { + var n int + r, n = utf8.DecodeRuneInString(s) + s = s[n:] + } chunk = chunk[1:] // possibly negated - notNegated := true + negated := false if len(chunk) > 0 && chunk[0] == '^' { - notNegated = false + negated = true chunk = chunk[1:] } // parse all ranges @@ -140,12 +156,12 @@ func matchChunk(chunk, s string) (rest string, ok bool, err error) { } var lo, hi rune if lo, chunk, err = getEsc(chunk); err != nil { - return + return "", false, err } hi = lo if chunk[0] == '-' { if hi, chunk, err = getEsc(chunk[1:]); err != nil { - return + return "", false, err } } if lo <= r && r <= hi { @@ -153,34 +169,40 @@ func matchChunk(chunk, s string) (rest string, ok bool, err error) { } nrange++ } - if match != notNegated { - return + if match == negated { + failed = true } case '?': - if s[0] == '/' { - return + if !failed { + if s[0] == '/' { + failed = true + } + _, n := utf8.DecodeRuneInString(s) + s = s[n:] } - _, n := utf8.DecodeRuneInString(s) - s = s[n:] chunk = chunk[1:] case '\\': chunk = chunk[1:] if len(chunk) == 0 { - err = ErrBadPattern - return + return "", false, ErrBadPattern } fallthrough default: - if chunk[0] != s[0] { - return + if !failed { + if chunk[0] != s[0] { + failed = true + } + s = s[1:] } - s = s[1:] chunk = chunk[1:] } } + if failed { + return "", false, nil + } return s, true, nil } diff --git a/src/path/match_test.go b/src/path/match_test.go index 3e027e1f68..996bd691eb 100644 --- a/src/path/match_test.go +++ b/src/path/match_test.go @@ -67,8 +67,10 @@ var matchTests = []MatchTest{ {"[", "a", false, ErrBadPattern}, {"[^", "a", false, ErrBadPattern}, {"[^bc", "a", false, ErrBadPattern}, - {"a[", "a", false, nil}, + {"a[", "a", false, ErrBadPattern}, {"a[", "ab", false, ErrBadPattern}, + {"a[", "x", false, ErrBadPattern}, + {"a/b[", "x", false, ErrBadPattern}, {"*x", "xxx", true, nil}, } diff --git a/src/testing/fstest/testfs.go b/src/testing/fstest/testfs.go index 21cd00e5b6..4912a271b2 100644 --- a/src/testing/fstest/testfs.go +++ b/src/testing/fstest/testfs.go @@ -282,6 +282,12 @@ func (t *fsTester) checkGlob(dir string, list []fs.DirEntry) { glob = strings.Join(elem, "/") + "/" } + // Test that malformed patterns are detected. + // The error is likely path.ErrBadPattern but need not be. + if _, err := t.fsys.(fs.GlobFS).Glob(glob + "nonexist/[]"); err == nil { + t.errorf("%s: Glob(%#q): bad pattern not detected", dir, glob+"nonexist/[]") + } + // Try to find a letter that appears in only some of the final names. c := rune('a') for ; c <= 'z'; c++ { -- cgit v1.3 From 362d25f2c82980860cb4eb5bfd0648116504788d Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Sun, 19 Jul 2020 01:31:05 -0400 Subject: io/fs: add WalkDir This commit is a copy of filepath.WalkDir adapted to use fs.FS instead of the native OS file system. It is the last implementation piece of the io/fs proposal. The original io/fs proposal was to adopt filepath.Walk, but we have since introduced the more efficient filepath.WalkDir (#42027), so this CL adopts that more efficient option instead. (The changes in path/filepath bring the two copies more in line with each other. The main change is unembedding the field in statDirEntry, so that the fs.DirEntry passed to the WalkDirFunc for the root of the tree does not have any extra methods.) For #41190. Change-Id: I9359dfcc110338c0ec64535f22cafb38d0b613a6 Reviewed-on: https://go-review.googlesource.com/c/go/+/243916 Trust: Russ Cox Run-TryBot: Russ Cox TryBot-Result: Go Bot Reviewed-by: Rob Pike --- src/io/fs/walk.go | 132 +++++++++++++++++++++++++++++++++ src/io/fs/walk_test.go | 155 +++++++++++++++++++++++++++++++++++++++ src/path/filepath/export_test.go | 2 - src/path/filepath/path.go | 77 +++---------------- src/path/filepath/path_test.go | 15 +++- 5 files changed, 310 insertions(+), 71 deletions(-) create mode 100644 src/io/fs/walk.go create mode 100644 src/io/fs/walk_test.go (limited to 'src/io/fs') diff --git a/src/io/fs/walk.go b/src/io/fs/walk.go new file mode 100644 index 0000000000..e50c1bb15c --- /dev/null +++ b/src/io/fs/walk.go @@ -0,0 +1,132 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package fs + +import ( + "errors" + "path" +) + +// SkipDir is used as a return value from WalkFuncs to indicate that +// the directory named in the call is to be skipped. It is not returned +// as an error by any function. +var SkipDir = errors.New("skip this directory") + +// WalkDirFunc is the type of the function called by WalkDir to visit +// each each file or directory. +// +// The path argument contains the argument to Walk as a prefix. +// That is, if Walk is called with root argument "dir" and finds a file +// named "a" in that directory, the walk function will be called with +// argument "dir/a". +// +// The directory and file are joined with Join, which may clean the +// directory name: if Walk is called with the root argument "x/../dir" +// and finds a file named "a" in that directory, the walk function will +// be called with argument "dir/a", not "x/../dir/a". +// +// The d argument is the fs.DirEntry for the named path. +// +// The error result returned by the function controls how WalkDir +// continues. If the function returns the special value SkipDir, WalkDir +// skips the current directory (path if d.IsDir() is true, otherwise +// path's parent directory). Otherwise, if the function returns a non-nil +// error, WalkDir stops entirely and returns that error. +// +// The err argument reports an error related to path, signaling that +// WalkDir will not walk into that directory. The function can decide how +// to handle that error; as described earlier, returning the error will +// cause WalkDir to stop walking the entire tree. +// +// WalkDir calls the function with a non-nil err argument in two cases. +// +// First, if the initial os.Lstat on the root directory fails, WalkDir +// calls the function with path set to root, d set to nil, and err set to +// the error from os.Lstat. +// +// Second, if a directory's ReadDir method fails, WalkDir calls the +// function with path set to the directory's path, d set to an +// fs.DirEntry describing the directory, and err set to the error from +// ReadDir. In this second case, the function is called twice with the +// path of the directory: the first call is before the directory read is +// attempted and has err set to nil, giving the function a chance to +// return SkipDir and avoid the ReadDir entirely. The second call is +// after a failed ReadDir and reports the error from ReadDir. +// (If ReadDir succeeds, there is no second call.) +// +// The differences between WalkDirFunc compared to WalkFunc are: +// +// - The second argument has type fs.DirEntry instead of fs.FileInfo. +// - The function is called before reading a directory, to allow SkipDir +// to bypass the directory read entirely. +// - If a directory read fails, the function is called a second time +// for that directory to report the error. +// +type WalkDirFunc func(path string, entry DirEntry, err error) error + +// walkDir recursively descends path, calling walkDirFn. +func walkDir(fsys FS, name string, d DirEntry, walkDirFn WalkDirFunc) error { + if err := walkDirFn(name, d, nil); err != nil || !d.IsDir() { + if err == SkipDir && d.IsDir() { + // Successfully skipped directory. + err = nil + } + return err + } + + dirs, err := ReadDir(fsys, name) + if err != nil { + // Second call, to report ReadDir error. + err = walkDirFn(name, d, err) + if err != nil { + return err + } + } + + for _, d1 := range dirs { + name1 := path.Join(name, d1.Name()) + if err := walkDir(fsys, name1, d1, walkDirFn); err != nil { + if err == SkipDir { + break + } + return err + } + } + return nil +} + +// WalkDir walks the file tree rooted at root, calling fn for each file or +// directory in the tree, including root. +// +// All errors that arise visiting files and directories are filtered by fn: +// see the fs.WalkDirFunc documentation for details. +// +// The files are walked in lexical order, which makes the output deterministic +// but requires WalkDir to read an entire directory into memory before proceeding +// to walk that directory. +// +// WalkDir does not follow symbolic links found in directories, +// but if root itself is a symbolic link, its target will be walked. +func WalkDir(fsys FS, root string, fn WalkDirFunc) error { + info, err := Stat(fsys, root) + if err != nil { + err = fn(root, nil, err) + } else { + err = walkDir(fsys, root, &statDirEntry{info}, fn) + } + if err == SkipDir { + return nil + } + return err +} + +type statDirEntry struct { + info FileInfo +} + +func (d *statDirEntry) Name() string { return d.info.Name() } +func (d *statDirEntry) IsDir() bool { return d.info.IsDir() } +func (d *statDirEntry) Type() FileMode { return d.info.Mode().Type() } +func (d *statDirEntry) Info() (FileInfo, error) { return d.info, nil } diff --git a/src/io/fs/walk_test.go b/src/io/fs/walk_test.go new file mode 100644 index 0000000000..395471e2e8 --- /dev/null +++ b/src/io/fs/walk_test.go @@ -0,0 +1,155 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package fs_test + +import ( + . "io/fs" + "io/ioutil" + "os" + pathpkg "path" + "runtime" + "testing" + "testing/fstest" +) + +type Node struct { + name string + entries []*Node // nil if the entry is a file + mark int +} + +var tree = &Node{ + "testdata", + []*Node{ + {"a", nil, 0}, + {"b", []*Node{}, 0}, + {"c", nil, 0}, + { + "d", + []*Node{ + {"x", nil, 0}, + {"y", []*Node{}, 0}, + { + "z", + []*Node{ + {"u", nil, 0}, + {"v", nil, 0}, + }, + 0, + }, + }, + 0, + }, + }, + 0, +} + +func walkTree(n *Node, path string, f func(path string, n *Node)) { + f(path, n) + for _, e := range n.entries { + walkTree(e, pathpkg.Join(path, e.name), f) + } +} + +func makeTree(t *testing.T) FS { + fsys := fstest.MapFS{} + walkTree(tree, tree.name, func(path string, n *Node) { + if n.entries == nil { + fsys[path] = &fstest.MapFile{} + } else { + fsys[path] = &fstest.MapFile{Mode: ModeDir} + } + }) + return fsys +} + +func markTree(n *Node) { walkTree(n, "", func(path string, n *Node) { n.mark++ }) } + +func checkMarks(t *testing.T, report bool) { + walkTree(tree, tree.name, func(path string, n *Node) { + if n.mark != 1 && report { + t.Errorf("node %s mark = %d; expected 1", path, n.mark) + } + n.mark = 0 + }) +} + +// Assumes that each node name is unique. Good enough for a test. +// If clear is true, any incoming error is cleared before return. The errors +// are always accumulated, though. +func mark(entry DirEntry, err error, errors *[]error, clear bool) error { + name := entry.Name() + walkTree(tree, tree.name, func(path string, n *Node) { + if n.name == name { + n.mark++ + } + }) + if err != nil { + *errors = append(*errors, err) + if clear { + return nil + } + return err + } + return nil +} + +func chtmpdir(t *testing.T) (restore func()) { + oldwd, err := os.Getwd() + if err != nil { + t.Fatalf("chtmpdir: %v", err) + } + d, err := ioutil.TempDir("", "test") + if err != nil { + t.Fatalf("chtmpdir: %v", err) + } + if err := os.Chdir(d); err != nil { + t.Fatalf("chtmpdir: %v", err) + } + return func() { + if err := os.Chdir(oldwd); err != nil { + t.Fatalf("chtmpdir: %v", err) + } + os.RemoveAll(d) + } +} + +func TestWalkDir(t *testing.T) { + if runtime.GOOS == "darwin" && runtime.GOARCH == "arm64" { + restore := chtmpdir(t) + defer restore() + } + + tmpDir, err := ioutil.TempDir("", "TestWalk") + if err != nil { + t.Fatal("creating temp dir:", err) + } + defer os.RemoveAll(tmpDir) + + origDir, err := os.Getwd() + if err != nil { + t.Fatal("finding working dir:", err) + } + if err = os.Chdir(tmpDir); err != nil { + t.Fatal("entering temp dir:", err) + } + defer os.Chdir(origDir) + + fsys := makeTree(t) + errors := make([]error, 0, 10) + clear := true + markFn := func(path string, entry DirEntry, err error) error { + return mark(entry, err, &errors, clear) + } + // Expect no errors. + err = WalkDir(fsys, ".", markFn) + if err != nil { + t.Fatalf("no error expected, found: %s", err) + } + if len(errors) != 0 { + t.Fatalf("unexpected errors: %s", errors) + } + checkMarks(t, true) +} diff --git a/src/path/filepath/export_test.go b/src/path/filepath/export_test.go index e7ad7dd01a..0cf9e3bca1 100644 --- a/src/path/filepath/export_test.go +++ b/src/path/filepath/export_test.go @@ -5,5 +5,3 @@ package filepath var LstatP = &lstat - -type DirEntryFromInfo = dirEntryFromInfo diff --git a/src/path/filepath/path.go b/src/path/filepath/path.go index 3f7e5c713d..2e7b439355 100644 --- a/src/path/filepath/path.go +++ b/src/path/filepath/path.go @@ -334,59 +334,7 @@ func Rel(basepath, targpath string) (string, error) { // SkipDir is used as a return value from WalkFuncs to indicate that // the directory named in the call is to be skipped. It is not returned // as an error by any function. -var SkipDir = errors.New("skip this directory") - -// WalkDirFunc is the type of the function called by WalkDir to visit -// each each file or directory. -// -// The path argument contains the argument to Walk as a prefix. -// That is, if Walk is called with root argument "dir" and finds a file -// named "a" in that directory, the walk function will be called with -// argument "dir/a". -// -// The directory and file are joined with Join, which may clean the -// directory name: if Walk is called with the root argument "x/../dir" -// and finds a file named "a" in that directory, the walk function will -// be called with argument "dir/a", not "x/../dir/a". -// -// The d argument is the fs.DirEntry for the named path. -// -// The error result returned by the function controls how WalkDir -// continues. If the function returns the special value SkipDir, WalkDir -// skips the current directory (path if d.IsDir() is true, otherwise -// path's parent directory). Otherwise, if the function returns a non-nil -// error, WalkDir stops entirely and returns that error. -// -// The err argument reports an error related to path, signaling that -// WalkDir will not walk into that directory. The function can decide how -// to handle that error; as described earlier, returning the error will -// cause WalkDir to stop walking the entire tree. -// -// WalkDir calls the function with a non-nil err argument in two cases. -// -// First, if the initial os.Lstat on the root directory fails, WalkDir -// calls the function with path set to root, d set to nil, and err set to -// the error from os.Lstat. -// -// Second, if a directory's ReadDir method fails, WalkDir calls the -// function with path set to the directory's path, d set to an -// fs.DirEntry describing the directory, and err set to the error from -// ReadDir. In this second case, the function is called twice with the -// path of the directory: the first call is before the directory read is -// attempted and has err set to nil, giving the function a chance to -// return SkipDir and avoid the ReadDir entirely. The second call is -// after a failed ReadDir and reports the error from ReadDir. -// (If ReadDir succeeds, there is no second call.) -// -// The differences between WalkDirFunc compared to WalkFunc are: -// -// - The second argument has type fs.DirEntry instead of fs.FileInfo. -// - The function is called before reading a directory, to allow SkipDir -// to bypass the directory read entirely. -// - If a directory read fails, the function is called a second time -// for that directory to report the error. -// -type WalkDirFunc func(path string, d fs.DirEntry, err error) error +var SkipDir error = fs.SkipDir // WalkFunc is the type of the function called by Walk to visit each each // file or directory. @@ -430,7 +378,7 @@ type WalkFunc func(path string, info fs.FileInfo, err error) error var lstat = os.Lstat // for testing // walkDir recursively descends path, calling walkDirFn. -func walkDir(path string, d fs.DirEntry, walkDirFn WalkDirFunc) error { +func walkDir(path string, d fs.DirEntry, walkDirFn fs.WalkDirFunc) error { if err := walkDirFn(path, d, nil); err != nil || !d.IsDir() { if err == SkipDir && d.IsDir() { // Successfully skipped directory. @@ -502,19 +450,19 @@ func walk(path string, info fs.FileInfo, walkFn WalkFunc) error { // directory in the tree, including root. // // All errors that arise visiting files and directories are filtered by fn: -// see the WalkDirFunc documentation for details. +// see the fs.WalkDirFunc documentation for details. // // The files are walked in lexical order, which makes the output deterministic // but requires WalkDir to read an entire directory into memory before proceeding // to walk that directory. // // WalkDir does not follow symbolic links. -func WalkDir(root string, fn WalkDirFunc) error { +func WalkDir(root string, fn fs.WalkDirFunc) error { info, err := os.Lstat(root) if err != nil { err = fn(root, nil, err) } else { - err = walkDir(root, &dirEntryFromInfo{info}, fn) + err = walkDir(root, &statDirEntry{info}, fn) } if err == SkipDir { return nil @@ -522,17 +470,14 @@ func WalkDir(root string, fn WalkDirFunc) error { return err } -type dirEntryFromInfo struct { - fs.FileInfo +type statDirEntry struct { + info fs.FileInfo } -func (e *dirEntryFromInfo) Type() fs.FileMode { - return e.Mode().Type() -} - -func (e *dirEntryFromInfo) Info() (fs.FileInfo, error) { - return e.FileInfo, nil -} +func (d *statDirEntry) Name() string { return d.info.Name() } +func (d *statDirEntry) IsDir() bool { return d.info.IsDir() } +func (d *statDirEntry) Type() fs.FileMode { return d.info.Mode().Type() } +func (d *statDirEntry) Info() (fs.FileInfo, error) { return d.info, nil } // Walk walks the file tree rooted at root, calling fn for each file or // directory in the tree, including root. diff --git a/src/path/filepath/path_test.go b/src/path/filepath/path_test.go index ec6f8f2de9..d760530e26 100644 --- a/src/path/filepath/path_test.go +++ b/src/path/filepath/path_test.go @@ -432,19 +432,28 @@ func chtmpdir(t *testing.T) (restore func()) { } func TestWalk(t *testing.T) { - walk := func(root string, fn filepath.WalkDirFunc) error { + walk := func(root string, fn fs.WalkDirFunc) error { return filepath.Walk(root, func(path string, info fs.FileInfo, err error) error { - return fn(path, &filepath.DirEntryFromInfo{info}, err) + return fn(path, &statDirEntry{info}, err) }) } testWalk(t, walk, 1) } +type statDirEntry struct { + info fs.FileInfo +} + +func (d *statDirEntry) Name() string { return d.info.Name() } +func (d *statDirEntry) IsDir() bool { return d.info.IsDir() } +func (d *statDirEntry) Type() fs.FileMode { return d.info.Mode().Type() } +func (d *statDirEntry) Info() (fs.FileInfo, error) { return d.info, nil } + func TestWalkDir(t *testing.T) { testWalk(t, filepath.WalkDir, 2) } -func testWalk(t *testing.T, walk func(string, filepath.WalkDirFunc) error, errVisit int) { +func testWalk(t *testing.T, walk func(string, fs.WalkDirFunc) error, errVisit int) { if runtime.GOOS == "ios" { restore := chtmpdir(t) defer restore() -- cgit v1.3 From c906608406f22087e9bc3ee7616c3f1fbba2503b Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Mon, 9 Nov 2020 09:25:05 -0500 Subject: io/fs: fix reference to WalkFunc The comment explains differences between WalkDirFunc and WalkFunc, but when this code moved out of path/filepath, we forgot to change the reference to be filepath.WalkFunc. Fix that. (The text should not be deleted, because path/filepath does not contain this type - WalkDirFunc - nor this text anymore.) Pointed out by Carl Johnson on CL 243916 post-submit. For #41190. Change-Id: I44c64d0b7e60cd6d3694cfd6d0b95468ec4612fe Reviewed-on: https://go-review.googlesource.com/c/go/+/268417 Trust: Russ Cox Reviewed-by: Dmitri Shuralyov --- src/io/fs/walk.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/io/fs') diff --git a/src/io/fs/walk.go b/src/io/fs/walk.go index e50c1bb15c..dfc73767bc 100644 --- a/src/io/fs/walk.go +++ b/src/io/fs/walk.go @@ -9,7 +9,7 @@ import ( "path" ) -// SkipDir is used as a return value from WalkFuncs to indicate that +// SkipDir is used as a return value from WalkDirFuncs to indicate that // the directory named in the call is to be skipped. It is not returned // as an error by any function. var SkipDir = errors.New("skip this directory") @@ -56,7 +56,7 @@ var SkipDir = errors.New("skip this directory") // after a failed ReadDir and reports the error from ReadDir. // (If ReadDir succeeds, there is no second call.) // -// The differences between WalkDirFunc compared to WalkFunc are: +// The differences between WalkDirFunc compared to filepath.WalkFunc are: // // - The second argument has type fs.DirEntry instead of fs.FileInfo. // - The function is called before reading a directory, to allow SkipDir -- cgit v1.3 From f3ce010b331513b4dca26a12dc0fd12dc4385d9c Mon Sep 17 00:00:00 2001 From: fzipp Date: Sun, 8 Nov 2020 13:50:30 +0000 Subject: io/fs: make WalkDirFunc parameter name consistent with doc comment The the DirEntry parameter of WalkDirFunc is referred to as `d` in the doc comment. Change-Id: Ibfcf7908eaa0ef1309898150e8fd71101e7de09b GitHub-Last-Rev: e858c52d81b93d293621d7e744bdcb7d6cbd412c GitHub-Pull-Request: golang/go#42447 Reviewed-on: https://go-review.googlesource.com/c/go/+/268277 Trust: Emmanuel Odeke Reviewed-by: Russ Cox --- src/io/fs/walk.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/io/fs') diff --git a/src/io/fs/walk.go b/src/io/fs/walk.go index dfc73767bc..06d0b1769c 100644 --- a/src/io/fs/walk.go +++ b/src/io/fs/walk.go @@ -64,7 +64,7 @@ var SkipDir = errors.New("skip this directory") // - If a directory read fails, the function is called a second time // for that directory to report the error. // -type WalkDirFunc func(path string, entry DirEntry, err error) error +type WalkDirFunc func(path string, d DirEntry, err error) error // walkDir recursively descends path, calling walkDirFn. func walkDir(fsys FS, name string, d DirEntry, walkDirFn WalkDirFunc) error { -- cgit v1.3 From 478bde3a4388997924a02ee9296864866d8ba3ba Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 2 Dec 2020 12:49:20 -0500 Subject: io/fs: add Sub Sub provides a convenient way to refer to a subdirectory automatically in future operations, like Unix's chdir(2). The CL also includes updates to fstest to check Sub implementations. As part of updating fstest, I changed the meaning of TestFS's expected list to introduce a special case: if you list no expected files, that means the FS must be empty. In general it's OK not to list all the expected files, but if you list none, that's almost certainly a mistake - if your FS were broken and empty, you wouldn't find out. Making no expected files mean "must be empty" makes the mistake less likely - if your file system ever worked, then your test will keep it working. That change found a testing bug: embedtest was making exactly that mistake. Fixes #42322. Change-Id: I63fd4aa866b30061a0e51ca9a1927e576d6ec41e Reviewed-on: https://go-review.googlesource.com/c/go/+/274856 Trust: Russ Cox Run-TryBot: Russ Cox TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor --- doc/go1.16.html | 2 +- src/embed/internal/embedtest/embed_test.go | 2 +- src/io/fs/readdir_test.go | 12 ++- src/io/fs/readfile_test.go | 16 ++++ src/io/fs/sub.go | 127 +++++++++++++++++++++++++++++ src/io/fs/sub_test.go | 57 +++++++++++++ src/os/file.go | 7 ++ src/testing/fstest/mapfs.go | 10 +++ src/testing/fstest/testfs.go | 42 ++++++++++ 9 files changed, 271 insertions(+), 4 deletions(-) create mode 100644 src/io/fs/sub.go create mode 100644 src/io/fs/sub_test.go (limited to 'src/io/fs') diff --git a/doc/go1.16.html b/doc/go1.16.html index 4d4b459009..62d9b97db8 100644 --- a/doc/go1.16.html +++ b/doc/go1.16.html @@ -384,7 +384,7 @@ Do not send CLs removing the interior tags from such phrases. the new embed.FS type implements fs.FS, as does zip.Reader. - The new os.Dir function + The new os.DirFS function provides an implementation of fs.FS backed by a tree of operating system files.

diff --git a/src/embed/internal/embedtest/embed_test.go b/src/embed/internal/embedtest/embed_test.go index b1707a4c04..c6a7bea7a3 100644 --- a/src/embed/internal/embedtest/embed_test.go +++ b/src/embed/internal/embedtest/embed_test.go @@ -65,7 +65,7 @@ func TestGlobal(t *testing.T) { testFiles(t, global, "testdata/hello.txt", "hello, world\n") testFiles(t, global, "testdata/glass.txt", "I can eat glass and it doesn't hurt me.\n") - if err := fstest.TestFS(global); err != nil { + if err := fstest.TestFS(global, "concurrency.txt", "testdata/hello.txt"); err != nil { t.Fatal(err) } diff --git a/src/io/fs/readdir_test.go b/src/io/fs/readdir_test.go index 46a4bc2788..405bfa67ca 100644 --- a/src/io/fs/readdir_test.go +++ b/src/io/fs/readdir_test.go @@ -16,12 +16,12 @@ func (readDirOnly) Open(name string) (File, error) { return nil, ErrNotExist } func TestReadDir(t *testing.T) { check := func(desc string, dirs []DirEntry, err error) { t.Helper() - if err != nil || len(dirs) != 1 || dirs[0].Name() != "hello.txt" { + if err != nil || len(dirs) != 2 || dirs[0].Name() != "hello.txt" || dirs[1].Name() != "sub" { var names []string for _, d := range dirs { names = append(names, d.Name()) } - t.Errorf("ReadDir(%s) = %v, %v, want %v, nil", desc, names, err, []string{"hello.txt"}) + t.Errorf("ReadDir(%s) = %v, %v, want %v, nil", desc, names, err, []string{"hello.txt", "sub"}) } } @@ -32,4 +32,12 @@ func TestReadDir(t *testing.T) { // Test that ReadDir uses Open when the method is not present. dirs, err = ReadDir(openOnly{testFsys}, ".") check("openOnly", dirs, err) + + // Test that ReadDir on Sub of . works (sub_test checks non-trivial subs). + sub, err := Sub(testFsys, ".") + if err != nil { + t.Fatal(err) + } + dirs, err = ReadDir(sub, ".") + check("sub(.)", dirs, err) } diff --git a/src/io/fs/readfile_test.go b/src/io/fs/readfile_test.go index 0afa334ace..07219c1445 100644 --- a/src/io/fs/readfile_test.go +++ b/src/io/fs/readfile_test.go @@ -18,6 +18,12 @@ var testFsys = fstest.MapFS{ ModTime: time.Now(), Sys: &sysValue, }, + "sub/goodbye.txt": { + Data: []byte("goodbye, world"), + Mode: 0456, + ModTime: time.Now(), + Sys: &sysValue, + }, } var sysValue int @@ -40,4 +46,14 @@ func TestReadFile(t *testing.T) { if string(data) != "hello, world" || err != nil { t.Fatalf(`ReadFile(openOnly, "hello.txt") = %q, %v, want %q, nil`, data, err, "hello, world") } + + // Test that ReadFile on Sub of . works (sub_test checks non-trivial subs). + sub, err := Sub(testFsys, ".") + if err != nil { + t.Fatal(err) + } + data, err = ReadFile(sub, "hello.txt") + if string(data) != "hello, world" || err != nil { + t.Fatalf(`ReadFile(sub(.), "hello.txt") = %q, %v, want %q, nil`, data, err, "hello, world") + } } diff --git a/src/io/fs/sub.go b/src/io/fs/sub.go new file mode 100644 index 0000000000..381f409504 --- /dev/null +++ b/src/io/fs/sub.go @@ -0,0 +1,127 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package fs + +import ( + "errors" + "path" +) + +// A SubFS is a file system with a Sub method. +type SubFS interface { + FS + + // Sub returns an FS corresponding to the subtree rooted at dir. + Sub(dir string) (FS, error) +} + +// Sub returns an FS corresponding to the subtree rooted at fsys's dir. +// +// If fs implements SubFS, Sub calls returns fsys.Sub(dir). +// Otherwise, if dir is ".", Sub returns fsys unchanged. +// Otherwise, Sub returns a new FS implementation sub that, +// in effect, implements sub.Open(dir) as fsys.Open(path.Join(dir, name)). +// The implementation also translates calls to ReadDir, ReadFile, and Glob appropriately. +// +// Note that Sub(os.DirFS("/"), "prefix") is equivalent to os.DirFS("/prefix") +// and that neither of them guarantees to avoid operating system +// accesses outside "/prefix", because the implementation of os.DirFS +// does not check for symbolic links inside "/prefix" that point to +// other directories. That is, os.DirFS is not a general substitute for a +// chroot-style security mechanism, and Sub does not change that fact. +func Sub(fsys FS, dir string) (FS, error) { + if !ValidPath(dir) { + return nil, &PathError{Op: "sub", Path: dir, Err: errors.New("invalid name")} + } + if dir == "." { + return fsys, nil + } + if fsys, ok := fsys.(SubFS); ok { + return fsys.Sub(dir) + } + return &subFS{fsys, dir}, nil +} + +type subFS struct { + fsys FS + dir string +} + +// fullName maps name to the fully-qualified name dir/name. +func (f *subFS) fullName(op string, name string) (string, error) { + if !ValidPath(name) { + return "", &PathError{Op: op, Path: name, Err: errors.New("invalid name")} + } + return path.Join(f.dir, name), nil +} + +// shorten maps name, which should start with f.dir, back to the suffix after f.dir. +func (f *subFS) shorten(name string) (rel string, ok bool) { + if name == f.dir { + return ".", true + } + if len(name) >= len(f.dir)+2 && name[len(f.dir)] == '/' && name[:len(f.dir)] == f.dir { + return name[len(f.dir)+1:], true + } + return "", false +} + +// fixErr shortens any reported names in PathErrors by stripping dir. +func (f *subFS) fixErr(err error) error { + if e, ok := err.(*PathError); ok { + if short, ok := f.shorten(e.Path); ok { + e.Path = short + } + } + return err +} + +func (f *subFS) Open(name string) (File, error) { + full, err := f.fullName("open", name) + if err != nil { + return nil, err + } + file, err := f.fsys.Open(full) + return file, f.fixErr(err) +} + +func (f *subFS) ReadDir(name string) ([]DirEntry, error) { + full, err := f.fullName("open", name) + if err != nil { + return nil, err + } + dir, err := ReadDir(f.fsys, full) + return dir, f.fixErr(err) +} + +func (f *subFS) ReadFile(name string) ([]byte, error) { + full, err := f.fullName("open", name) + if err != nil { + return nil, err + } + data, err := ReadFile(f.fsys, full) + return data, f.fixErr(err) +} + +func (f *subFS) Glob(pattern string) ([]string, error) { + // Check pattern is well-formed. + if _, err := path.Match(pattern, ""); err != nil { + return nil, err + } + if pattern == "." { + return []string{"."}, nil + } + + full := f.dir + "/" + pattern + list, err := Glob(f.fsys, full) + for i, name := range list { + name, ok := f.shorten(name) + if !ok { + return nil, errors.New("invalid result from inner fsys Glob: " + name + " not in " + f.dir) // can't use fmt in this package + } + list[i] = name + } + return list, f.fixErr(err) +} diff --git a/src/io/fs/sub_test.go b/src/io/fs/sub_test.go new file mode 100644 index 0000000000..451b0efb02 --- /dev/null +++ b/src/io/fs/sub_test.go @@ -0,0 +1,57 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package fs_test + +import ( + . "io/fs" + "testing" +) + +type subOnly struct{ SubFS } + +func (subOnly) Open(name string) (File, error) { return nil, ErrNotExist } + +func TestSub(t *testing.T) { + check := func(desc string, sub FS, err error) { + t.Helper() + if err != nil { + t.Errorf("Sub(sub): %v", err) + return + } + data, err := ReadFile(sub, "goodbye.txt") + if string(data) != "goodbye, world" || err != nil { + t.Errorf(`ReadFile(%s, "goodbye.txt" = %q, %v, want %q, nil`, desc, string(data), err, "goodbye, world") + } + + dirs, err := ReadDir(sub, ".") + if err != nil || len(dirs) != 1 || dirs[0].Name() != "goodbye.txt" { + var names []string + for _, d := range dirs { + names = append(names, d.Name()) + } + t.Errorf(`ReadDir(%s, ".") = %v, %v, want %v, nil`, desc, names, err, []string{"goodbye.txt"}) + } + } + + // Test that Sub uses the method when present. + sub, err := Sub(subOnly{testFsys}, "sub") + check("subOnly", sub, err) + + // Test that Sub uses Open when the method is not present. + sub, err = Sub(openOnly{testFsys}, "sub") + check("openOnly", sub, err) + + _, err = sub.Open("nonexist") + if err == nil { + t.Fatal("Open(nonexist): succeeded") + } + pe, ok := err.(*PathError) + if !ok { + t.Fatalf("Open(nonexist): error is %T, want *PathError", err) + } + if pe.Path != "nonexist" { + t.Fatalf("Open(nonexist): err.Path = %q, want %q", pe.Path, "nonexist") + } +} diff --git a/src/os/file.go b/src/os/file.go index 304b055dbe..416bc0efa6 100644 --- a/src/os/file.go +++ b/src/os/file.go @@ -609,6 +609,13 @@ func isWindowsNulName(name string) bool { } // DirFS returns a file system (an fs.FS) for the tree of files rooted at the directory dir. +// +// Note that DirFS("/prefix") only guarantees that the Open calls it makes to the +// operating system will begin with "/prefix": DirFS("/prefix").Open("file") is the +// same as os.Open("/prefix/file"). So if /prefix/file is a symbolic link pointing outside +// the /prefix tree, then using DirFS does not stop the access any more than using +// os.Open does. DirFS is therefore not a general substitute for a chroot-style security +// mechanism when the directory tree contains arbitrary content. func DirFS(dir string) fs.FS { return dirFS(dir) } diff --git a/src/testing/fstest/mapfs.go b/src/testing/fstest/mapfs.go index 10e56f5b3c..a5d4a23fac 100644 --- a/src/testing/fstest/mapfs.go +++ b/src/testing/fstest/mapfs.go @@ -132,6 +132,16 @@ func (fsys MapFS) Glob(pattern string) ([]string, error) { return fs.Glob(fsOnly{fsys}, pattern) } +type noSub struct { + MapFS +} + +func (noSub) Sub() {} // not the fs.SubFS signature + +func (fsys MapFS) Sub(dir string) (fs.FS, error) { + return fs.Sub(noSub{fsys}, dir) +} + // A mapFileInfo implements fs.FileInfo and fs.DirEntry for a given map file. type mapFileInfo struct { name string diff --git a/src/testing/fstest/testfs.go b/src/testing/fstest/testfs.go index 4912a271b2..2602bdf0cc 100644 --- a/src/testing/fstest/testfs.go +++ b/src/testing/fstest/testfs.go @@ -22,6 +22,8 @@ import ( // It walks the entire tree of files in fsys, // opening and checking that each file behaves correctly. // It also checks that the file system contains at least the expected files. +// As a special case, if no expected files are listed, fsys must be empty. +// Otherwise, fsys must only contain at least the listed files: it can also contain others. // // If TestFS finds any misbehaviors, it returns an error reporting all of them. // The error text spans multiple lines, one per detected misbehavior. @@ -33,6 +35,32 @@ import ( // } // func TestFS(fsys fs.FS, expected ...string) error { + if err := testFS(fsys, expected...); err != nil { + return err + } + for _, name := range expected { + if i := strings.Index(name, "/"); i >= 0 { + dir, dirSlash := name[:i], name[:i+1] + var subExpected []string + for _, name := range expected { + if strings.HasPrefix(name, dirSlash) { + subExpected = append(subExpected, name[len(dirSlash):]) + } + } + sub, err := fs.Sub(fsys, dir) + if err != nil { + return err + } + if err := testFS(sub, subExpected...); err != nil { + return fmt.Errorf("testing fs.Sub(fsys, %s): %v", dir, err) + } + break // one sub-test is enough + } + } + return nil +} + +func testFS(fsys fs.FS, expected ...string) error { t := fsTester{fsys: fsys} t.checkDir(".") t.checkOpen(".") @@ -43,6 +71,20 @@ func TestFS(fsys fs.FS, expected ...string) error { for _, file := range t.files { found[file] = true } + delete(found, ".") + if len(expected) == 0 && len(found) > 0 { + var list []string + for k := range found { + if k != "." { + list = append(list, k) + } + } + sort.Strings(list) + if len(list) > 15 { + list = append(list[:10], "...") + } + t.errorf("expected empty file system but found files:\n%s", strings.Join(list, "\n")) + } for _, name := range expected { if !found[name] { t.errorf("expected but not found: %s", name) -- cgit v1.3 From 7ad6596c4711c48ef95334c9c6516a0c30979bd9 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Mon, 7 Dec 2020 15:20:37 -0500 Subject: io/fs: fix Sub method error text Noticed in (and alternative to) CL 275520. Change-Id: If6c107ee9928dd1910facd4dc66da7234cb91c39 Reviewed-on: https://go-review.googlesource.com/c/go/+/275879 Trust: Russ Cox Trust: Robert Griesemer Run-TryBot: Russ Cox Reviewed-by: Robert Griesemer TryBot-Result: Go Bot --- src/io/fs/sub.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/io/fs') diff --git a/src/io/fs/sub.go b/src/io/fs/sub.go index 381f409504..64cdffe6de 100644 --- a/src/io/fs/sub.go +++ b/src/io/fs/sub.go @@ -88,7 +88,7 @@ func (f *subFS) Open(name string) (File, error) { } func (f *subFS) ReadDir(name string) ([]DirEntry, error) { - full, err := f.fullName("open", name) + full, err := f.fullName("read", name) if err != nil { return nil, err } @@ -97,7 +97,7 @@ func (f *subFS) ReadDir(name string) ([]DirEntry, error) { } func (f *subFS) ReadFile(name string) ([]byte, error) { - full, err := f.fullName("open", name) + full, err := f.fullName("read", name) if err != nil { return nil, err } -- cgit v1.3