From 428eac8624eefa8fa01ed25ec60a3ec70e1fab90 Mon Sep 17 00:00:00 2001
From: Oliver Bone <owbone@gmail.com>
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        | 63 +++++++++++++++---------------------------------
 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, 108 insertions(+), 54 deletions(-)

diff --git a/README.md b/README.md
index cab257f..f928346 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 469ff7d..fdb2996 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 f916bd2..c549d23 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 c803455..b948308 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 5ef8b6a..ffc58c6 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 998a5d0..59408a4 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 ea0798d..60c5537 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
-
-	m.mu.RLock()
-	f, ok := 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)
-}
-
-func (m *MemMapFs) setFileMode(name string, mode os.FileMode) error {
+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 f
+}
+
+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 881a61b..613f402 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 ac359c6..d43bf93 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 eef14e3..979dce3 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 e1d63ed..6e09f06 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 34f99a4..8dbfb6d 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 355f5f4..e3eb90e 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)