From 428eac8624eefa8fa01ed25ec60a3ec70e1fab90 Mon Sep 17 00:00:00 2001 From: Oliver Bone Date: Fri, 20 May 2022 12:19:26 +0100 Subject: [PATCH] Add Chmod() method to File `os.File` offers a `Chmod()` method. This is often safer and more direct to use than `os.Chmod()` because it operates on an open file descriptor rather than having to lookup the file by name. Without this, it's possible for the target file to be renamed, in which case an `os.Chmod()` would either fail or apply to any file that's taken its place. Therefore, add the `Chmod()` method to the `File` interface, and implement it for all `File` implementations. The bulk of this change is in `MemMapFs`, which required moving the chmod functionality down into the `mem` package so that it can be shared between both `mem.File` and `MemMapFs`. --- README.md | 1 + afero.go | 1 + gcsfs/file.go | 5 ++++ iofs.go | 4 ++++ mem/file.go | 28 +++++++++++++++++++---- mem/file_test.go | 4 ++-- memmap.go | 59 ++++++++++++++---------------------------------- memmap_test.go | 26 ++++++++++++++++++--- regexpfs.go | 4 ++++ sftpfs/file.go | 4 ++++ tarfs/file.go | 4 ++++ unionFile.go | 14 ++++++++++++ zipfs/file.go | 4 ++++ 13 files changed, 106 insertions(+), 52 deletions(-) diff --git a/README.md b/README.md index cab257f5..f9283469 100644 --- a/README.md +++ b/README.md @@ -116,6 +116,7 @@ io.Seeker io.Writer io.WriterAt +Chmod(mode os.FileMode) : error Name() : string Readdir(count int) : []os.FileInfo, error Readdirnames(n int) : []string, error diff --git a/afero.go b/afero.go index 469ff7d2..fdb29961 100644 --- a/afero.go +++ b/afero.go @@ -42,6 +42,7 @@ type File interface { io.Writer io.WriterAt + Chmod(mode os.FileMode) error Name() string Readdir(count int) ([]os.FileInfo, error) Readdirnames(n int) ([]string, error) diff --git a/gcsfs/file.go b/gcsfs/file.go index f916bd22..c549d23e 100644 --- a/gcsfs/file.go +++ b/gcsfs/file.go @@ -18,6 +18,7 @@ package gcsfs import ( "context" + "errors" "fmt" "io" "log" @@ -175,6 +176,10 @@ func (o *GcsFile) WriteAt(b []byte, off int64) (n int, err error) { return written, err } +func (o *GcsFile) Chmod(mode os.FileMode) error { + return errors.New("method Chmod is not implemented in GCS") +} + func (o *GcsFile) Name() string { return filepath.FromSlash(o.resource.name) } diff --git a/iofs.go b/iofs.go index c8034553..b9483088 100644 --- a/iofs.go +++ b/iofs.go @@ -203,6 +203,10 @@ type fromIOFSFile struct { name string } +func (f fromIOFSFile) Chmod(mode os.FileMode) error { + return notImplemented("chmod", f.name) +} + func (f fromIOFSFile) ReadAt(p []byte, off int64) (n int, err error) { readerAt, ok := f.File.(io.ReaderAt) if !ok { diff --git a/mem/file.go b/mem/file.go index 5ef8b6a3..ffc58c68 100644 --- a/mem/file.go +++ b/mem/file.go @@ -25,7 +25,11 @@ import ( "time" ) -const FilePathSeparator = string(filepath.Separator) +const ( + FilePathSeparator = string(filepath.Separator) + + chmodBits = os.ModePerm | os.ModeSetuid | os.ModeSetgid | os.ModeSticky // Only a subset of bits are allowed to be changed. Documented under os.Chmod() +) type File struct { // atomic requires 64-bit alignment for struct field access @@ -67,11 +71,20 @@ func (d *FileData) Name() string { } func CreateFile(name string) *FileData { - return &FileData{name: name, mode: os.ModeTemporary, modtime: time.Now()} + return &FileData{ + name: name, + modtime: time.Now(), + } } func CreateDir(name string) *FileData { - return &FileData{name: name, memDir: &DirMap{}, dir: true, modtime: time.Now()} + return &FileData{ + name: name, + mode: os.ModeDir, + memDir: &DirMap{}, + dir: true, + modtime: time.Now(), + } } func ChangeFileName(f *FileData, newname string) { @@ -80,9 +93,9 @@ func ChangeFileName(f *FileData, newname string) { f.Unlock() } -func SetMode(f *FileData, mode os.FileMode) { +func Chmod(f *FileData, mode os.FileMode) { f.Lock() - f.mode = mode + f.mode = (f.mode & ^chmodBits) | (mode & chmodBits) f.Unlock() } @@ -131,6 +144,11 @@ func (f *File) Close() error { return nil } +func (f *File) Chmod(mode os.FileMode) error { + Chmod(f.fileData, mode) + return nil +} + func (f *File) Name() string { return f.fileData.Name() } diff --git a/mem/file_test.go b/mem/file_test.go index 998a5d0b..59408a43 100644 --- a/mem/file_test.go +++ b/mem/file_test.go @@ -81,13 +81,13 @@ func TestFileDataModeRace(t *testing.T) { t.Errorf("Failed to read correct value, was %v", s.Mode()) } - SetMode(&d, someOtherMode) + Chmod(&d, someOtherMode) if s.Mode() != someOtherMode { t.Errorf("Failed to set Mode, was %v", s.Mode()) } go func() { - SetMode(&d, someMode) + Chmod(&d, someMode) }() if s.Mode() != someMode && s.Mode() != someOtherMode { diff --git a/memmap.go b/memmap.go index ea0798d8..60c55371 100644 --- a/memmap.go +++ b/memmap.go @@ -25,8 +25,6 @@ import ( "github.com/spf13/afero/mem" ) -const chmodBits = os.ModePerm | os.ModeSetuid | os.ModeSetgid | os.ModeSticky // Only a subset of bits are allowed to be changed. Documented under os.Chmod() - type MemMapFs struct { mu sync.RWMutex data map[string]*mem.FileData @@ -43,7 +41,7 @@ func (m *MemMapFs) getData() map[string]*mem.FileData { // Root should always exist, right? // TODO: what about windows? root := mem.CreateDir(FilePathSeparator) - mem.SetMode(root, os.ModeDir|0755) + mem.Chmod(root, 0755) m.data[FilePathSeparator] = root }) return m.data @@ -123,7 +121,7 @@ func (m *MemMapFs) lockfreeMkdir(name string, perm os.FileMode) error { } } else { item := mem.CreateDir(name) - mem.SetMode(item, os.ModeDir|perm) + mem.Chmod(item, perm) m.getData()[name] = item m.registerWithParent(item, perm) } @@ -131,7 +129,6 @@ func (m *MemMapFs) lockfreeMkdir(name string, perm os.FileMode) error { } func (m *MemMapFs) Mkdir(name string, perm os.FileMode) error { - perm &= chmodBits name = normalizePath(name) m.mu.RLock() @@ -143,12 +140,12 @@ func (m *MemMapFs) Mkdir(name string, perm os.FileMode) error { m.mu.Lock() item := mem.CreateDir(name) - mem.SetMode(item, os.ModeDir|perm) + mem.Chmod(item, perm) m.getData()[name] = item m.registerWithParent(item, perm) m.mu.Unlock() - return m.setFileMode(name, perm|os.ModeDir) + return nil } func (m *MemMapFs) MkdirAll(path string, perm os.FileMode) error { @@ -215,7 +212,6 @@ func (m *MemMapFs) lockfreeOpen(name string) (*mem.FileData, error) { } func (m *MemMapFs) OpenFile(name string, flag int, perm os.FileMode) (File, error) { - perm &= chmodBits chmod := false file, err := m.openWrite(name) if err == nil && (flag&os.O_EXCL > 0) { @@ -246,7 +242,7 @@ func (m *MemMapFs) OpenFile(name string, flag int, perm os.FileMode) (File, erro } } if chmod { - return file, m.setFileMode(name, perm) + m.Chmod(name, perm) } return file, nil } @@ -331,45 +327,30 @@ func (m *MemMapFs) Stat(name string) (os.FileInfo, error) { return fi, nil } -func (m *MemMapFs) Chmod(name string, mode os.FileMode) error { - mode &= chmodBits +func (m *MemMapFs) getFileData(name string) *mem.FileData { + name = normalizePath(name) m.mu.RLock() - f, ok := m.getData()[name] + f := m.getData()[name] m.mu.RUnlock() - if !ok { - return &os.PathError{Op: "chmod", Path: name, Err: ErrFileNotFound} - } - prevOtherBits := mem.GetFileInfo(f).Mode() & ^chmodBits - mode = prevOtherBits | mode - return m.setFileMode(name, mode) + return f } -func (m *MemMapFs) setFileMode(name string, mode os.FileMode) error { - name = normalizePath(name) - - m.mu.RLock() - f, ok := m.getData()[name] - m.mu.RUnlock() - if !ok { +func (m *MemMapFs) Chmod(name string, mode os.FileMode) error { + f := m.getFileData(name) + if f == nil { return &os.PathError{Op: "chmod", Path: name, Err: ErrFileNotFound} } - m.mu.Lock() - mem.SetMode(f, mode) - m.mu.Unlock() + mem.Chmod(f, mode) return nil } func (m *MemMapFs) Chown(name string, uid, gid int) error { - name = normalizePath(name) - - m.mu.RLock() - f, ok := m.getData()[name] - m.mu.RUnlock() - if !ok { + f := m.getFileData(name) + if f == nil { return &os.PathError{Op: "chown", Path: name, Err: ErrFileNotFound} } @@ -380,18 +361,12 @@ func (m *MemMapFs) Chown(name string, uid, gid int) error { } func (m *MemMapFs) Chtimes(name string, atime time.Time, mtime time.Time) error { - name = normalizePath(name) - - m.mu.RLock() - f, ok := m.getData()[name] - m.mu.RUnlock() - if !ok { + f := m.getFileData(name) + if f == nil { return &os.PathError{Op: "chtimes", Path: name, Err: ErrFileNotFound} } - m.mu.Lock() mem.SetModTime(f, mtime) - m.mu.Unlock() return nil } diff --git a/memmap_test.go b/memmap_test.go index 881a61bc..613f4029 100644 --- a/memmap_test.go +++ b/memmap_test.go @@ -614,7 +614,7 @@ func TestMemFsChmod(t *testing.T) { t.Fatal("mkdir failed to create a directory: mode =", info.Mode()) } - err = fs.Chmod(file, 0) + err = fs.Chmod(file, os.ModeTemporary|0355) if err != nil { t.Error("Failed to run chmod:", err) } @@ -623,9 +623,29 @@ func TestMemFsChmod(t *testing.T) { if err != nil { t.Fatal(err) } - if info.Mode().String() != "d---------" { - t.Error("chmod should not change file type. New mode =", info.Mode()) + if info.Mode().String() != "d-wxr-xr-x" { + t.Error("incorrect file mode after chmod:", info.Mode()) } + + f, err := fs.Open(file) + if err != nil { + t.Error("failed to open file:", err) + } + + err = f.Chmod(os.ModeNamedPipe | 0744) + if err != nil { + t.Error("Failed to run chmod:", err) + } + + info, err = fs.Stat(file) + if err != nil { + t.Fatal(err) + } + if info.Mode().String() != "drwxr--r--" { + t.Error("incorrect file mode after chmod:", info.Mode()) + } + + f.Close() } // can't use Mkdir to get around which permissions we're allowed to set diff --git a/regexpfs.go b/regexpfs.go index ac359c62..d43bf931 100644 --- a/regexpfs.go +++ b/regexpfs.go @@ -178,6 +178,10 @@ func (f *RegexpFile) WriteAt(s []byte, o int64) (int, error) { return f.f.WriteAt(s, o) } +func (f *RegexpFile) Chmod(mode os.FileMode) error { + return f.f.Chmod(mode) +} + func (f *RegexpFile) Name() string { return f.f.Name() } diff --git a/sftpfs/file.go b/sftpfs/file.go index eef14e36..979dce33 100644 --- a/sftpfs/file.go +++ b/sftpfs/file.go @@ -44,6 +44,10 @@ func (f *File) Close() error { return f.fd.Close() } +func (f *File) Chmod(mode os.FileMode) error { + return f.fd.Chmod(mode) +} + func (f *File) Name() string { return f.fd.Name() } diff --git a/tarfs/file.go b/tarfs/file.go index e1d63edc..6e09f06e 100644 --- a/tarfs/file.go +++ b/tarfs/file.go @@ -71,6 +71,10 @@ func (f *File) Write(p []byte) (n int, err error) { return 0, syscall.EROFS } func (f *File) WriteAt(p []byte, off int64) (n int, err error) { return 0, syscall.EROFS } +func (f *File) Chmod(mode os.FileMode) error { + return syscall.EROFS +} + func (f *File) Name() string { return filepath.Join(splitpath(f.h.Name)) } diff --git a/unionFile.go b/unionFile.go index 34f99a40..8dbfb6de 100644 --- a/unionFile.go +++ b/unionFile.go @@ -41,6 +41,20 @@ func (f *UnionFile) Close() error { return BADFD } +func (f *UnionFile) Chmod(mode os.FileMode) error { + if f.Layer != nil { + err := f.Layer.Chmod(mode) + if err == nil && f.Base != nil { + err = f.Base.Chmod(mode) + } + return err + } + if f.Base != nil { + return f.Base.Chmod(mode) + } + return BADFD +} + func (f *UnionFile) Read(s []byte) (int, error) { if f.Layer != nil { n, err := f.Layer.Read(s) diff --git a/zipfs/file.go b/zipfs/file.go index 355f5f45..e3eb90e8 100644 --- a/zipfs/file.go +++ b/zipfs/file.go @@ -104,6 +104,10 @@ func (f *File) Write(p []byte) (n int, err error) { return 0, syscall.EPERM } func (f *File) WriteAt(p []byte, off int64) (n int, err error) { return 0, syscall.EPERM } +func (f *File) Chmod(mode os.FileMode) error { + return syscall.EPERM +} + func (f *File) Name() string { if f.zipfile == nil { return string(filepath.Separator)