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`.

Author: owbone

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

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)

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)
 }

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 {

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()
 }

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 {

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,15 +121,14 @@ 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)
 	}
 	return nil
 }
 
 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
 }

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

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()
 }

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()
 }