Skip to content

Commit f72925f

Browse files
committed
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`.
1 parent 100c9a6 commit f72925f

File tree

13 files changed

+106
-52
lines changed

13 files changed

+106
-52
lines changed

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ io.Seeker
116116
io.Writer
117117
io.WriterAt
118118

119+
Chmod(mode os.FileMode) : error
119120
Name() : string
120121
Readdir(count int) : []os.FileInfo, error
121122
Readdirnames(n int) : []string, error

afero.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ type File interface {
4242
io.Writer
4343
io.WriterAt
4444

45+
Chmod(mode os.FileMode) error
4546
Name() string
4647
Readdir(count int) ([]os.FileInfo, error)
4748
Readdirnames(n int) ([]string, error)

gcsfs/file.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package gcsfs
1818

1919
import (
2020
"context"
21+
"errors"
2122
"fmt"
2223
"io"
2324
"log"
@@ -175,6 +176,10 @@ func (o *GcsFile) WriteAt(b []byte, off int64) (n int, err error) {
175176
return written, err
176177
}
177178

179+
func (o *GcsFile) Chmod(mode os.FileMode) error {
180+
return errors.New("method Chmod is not implemented in GCS")
181+
}
182+
178183
func (o *GcsFile) Name() string {
179184
return filepath.FromSlash(o.resource.name)
180185
}

iofs.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,10 @@ type fromIOFSFile struct {
203203
name string
204204
}
205205

206+
func (f fromIOFSFile) Chmod(mode os.FileMode) error {
207+
return notImplemented("chmod", f.name)
208+
}
209+
206210
func (f fromIOFSFile) ReadAt(p []byte, off int64) (n int, err error) {
207211
readerAt, ok := f.File.(io.ReaderAt)
208212
if !ok {

mem/file.go

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,11 @@ import (
2525
"time"
2626
)
2727

28-
const FilePathSeparator = string(filepath.Separator)
28+
const (
29+
FilePathSeparator = string(filepath.Separator)
30+
31+
chmodBits = os.ModePerm | os.ModeSetuid | os.ModeSetgid | os.ModeSticky // Only a subset of bits are allowed to be changed. Documented under os.Chmod()
32+
)
2933

3034
type File struct {
3135
// atomic requires 64-bit alignment for struct field access
@@ -67,11 +71,20 @@ func (d *FileData) Name() string {
6771
}
6872

6973
func CreateFile(name string) *FileData {
70-
return &FileData{name: name, mode: os.ModeTemporary, modtime: time.Now()}
74+
return &FileData{
75+
name: name,
76+
modtime: time.Now(),
77+
}
7178
}
7279

7380
func CreateDir(name string) *FileData {
74-
return &FileData{name: name, memDir: &DirMap{}, dir: true, modtime: time.Now()}
81+
return &FileData{
82+
name: name,
83+
mode: os.ModeDir,
84+
memDir: &DirMap{},
85+
dir: true,
86+
modtime: time.Now(),
87+
}
7588
}
7689

7790
func ChangeFileName(f *FileData, newname string) {
@@ -80,9 +93,9 @@ func ChangeFileName(f *FileData, newname string) {
8093
f.Unlock()
8194
}
8295

83-
func SetMode(f *FileData, mode os.FileMode) {
96+
func Chmod(f *FileData, mode os.FileMode) {
8497
f.Lock()
85-
f.mode = mode
98+
f.mode = (f.mode & ^chmodBits) | (mode & chmodBits)
8699
f.Unlock()
87100
}
88101

@@ -131,6 +144,11 @@ func (f *File) Close() error {
131144
return nil
132145
}
133146

147+
func (f *File) Chmod(mode os.FileMode) error {
148+
Chmod(f.fileData, mode)
149+
return nil
150+
}
151+
134152
func (f *File) Name() string {
135153
return f.fileData.Name()
136154
}

mem/file_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,13 @@ func TestFileDataModeRace(t *testing.T) {
8181
t.Errorf("Failed to read correct value, was %v", s.Mode())
8282
}
8383

84-
SetMode(&d, someOtherMode)
84+
Chmod(&d, someOtherMode)
8585
if s.Mode() != someOtherMode {
8686
t.Errorf("Failed to set Mode, was %v", s.Mode())
8787
}
8888

8989
go func() {
90-
SetMode(&d, someMode)
90+
Chmod(&d, someMode)
9191
}()
9292

9393
if s.Mode() != someMode && s.Mode() != someOtherMode {

memmap.go

Lines changed: 17 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,6 @@ import (
2525
"github.com/spf13/afero/mem"
2626
)
2727

28-
const chmodBits = os.ModePerm | os.ModeSetuid | os.ModeSetgid | os.ModeSticky // Only a subset of bits are allowed to be changed. Documented under os.Chmod()
29-
3028
type MemMapFs struct {
3129
mu sync.RWMutex
3230
data map[string]*mem.FileData
@@ -43,7 +41,7 @@ func (m *MemMapFs) getData() map[string]*mem.FileData {
4341
// Root should always exist, right?
4442
// TODO: what about windows?
4543
root := mem.CreateDir(FilePathSeparator)
46-
mem.SetMode(root, os.ModeDir|0755)
44+
mem.Chmod(root, 0755)
4745
m.data[FilePathSeparator] = root
4846
})
4947
return m.data
@@ -123,15 +121,14 @@ func (m *MemMapFs) lockfreeMkdir(name string, perm os.FileMode) error {
123121
}
124122
} else {
125123
item := mem.CreateDir(name)
126-
mem.SetMode(item, os.ModeDir|perm)
124+
mem.Chmod(item, perm)
127125
m.getData()[name] = item
128126
m.registerWithParent(item, perm)
129127
}
130128
return nil
131129
}
132130

133131
func (m *MemMapFs) Mkdir(name string, perm os.FileMode) error {
134-
perm &= chmodBits
135132
name = normalizePath(name)
136133

137134
m.mu.RLock()
@@ -143,12 +140,12 @@ func (m *MemMapFs) Mkdir(name string, perm os.FileMode) error {
143140

144141
m.mu.Lock()
145142
item := mem.CreateDir(name)
146-
mem.SetMode(item, os.ModeDir|perm)
143+
mem.Chmod(item, perm)
147144
m.getData()[name] = item
148145
m.registerWithParent(item, perm)
149146
m.mu.Unlock()
150147

151-
return m.setFileMode(name, perm|os.ModeDir)
148+
return nil
152149
}
153150

154151
func (m *MemMapFs) MkdirAll(path string, perm os.FileMode) error {
@@ -215,7 +212,6 @@ func (m *MemMapFs) lockfreeOpen(name string) (*mem.FileData, error) {
215212
}
216213

217214
func (m *MemMapFs) OpenFile(name string, flag int, perm os.FileMode) (File, error) {
218-
perm &= chmodBits
219215
chmod := false
220216
file, err := m.openWrite(name)
221217
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
246242
}
247243
}
248244
if chmod {
249-
return file, m.setFileMode(name, perm)
245+
m.Chmod(name, perm)
250246
}
251247
return file, nil
252248
}
@@ -331,45 +327,30 @@ func (m *MemMapFs) Stat(name string) (os.FileInfo, error) {
331327
return fi, nil
332328
}
333329

334-
func (m *MemMapFs) Chmod(name string, mode os.FileMode) error {
335-
mode &= chmodBits
330+
func (m *MemMapFs) getFileData(name string) *mem.FileData {
331+
name = normalizePath(name)
336332

337333
m.mu.RLock()
338-
f, ok := m.getData()[name]
334+
f := m.getData()[name]
339335
m.mu.RUnlock()
340-
if !ok {
341-
return &os.PathError{Op: "chmod", Path: name, Err: ErrFileNotFound}
342-
}
343-
prevOtherBits := mem.GetFileInfo(f).Mode() & ^chmodBits
344336

345-
mode = prevOtherBits | mode
346-
return m.setFileMode(name, mode)
337+
return f
347338
}
348339

349-
func (m *MemMapFs) setFileMode(name string, mode os.FileMode) error {
350-
name = normalizePath(name)
351-
352-
m.mu.RLock()
353-
f, ok := m.getData()[name]
354-
m.mu.RUnlock()
355-
if !ok {
340+
func (m *MemMapFs) Chmod(name string, mode os.FileMode) error {
341+
f := m.getFileData(name)
342+
if f == nil {
356343
return &os.PathError{Op: "chmod", Path: name, Err: ErrFileNotFound}
357344
}
358345

359-
m.mu.Lock()
360-
mem.SetMode(f, mode)
361-
m.mu.Unlock()
346+
mem.Chmod(f, mode)
362347

363348
return nil
364349
}
365350

366351
func (m *MemMapFs) Chown(name string, uid, gid int) error {
367-
name = normalizePath(name)
368-
369-
m.mu.RLock()
370-
f, ok := m.getData()[name]
371-
m.mu.RUnlock()
372-
if !ok {
352+
f := m.getFileData(name)
353+
if f == nil {
373354
return &os.PathError{Op: "chown", Path: name, Err: ErrFileNotFound}
374355
}
375356

@@ -380,18 +361,12 @@ func (m *MemMapFs) Chown(name string, uid, gid int) error {
380361
}
381362

382363
func (m *MemMapFs) Chtimes(name string, atime time.Time, mtime time.Time) error {
383-
name = normalizePath(name)
384-
385-
m.mu.RLock()
386-
f, ok := m.getData()[name]
387-
m.mu.RUnlock()
388-
if !ok {
364+
f := m.getFileData(name)
365+
if f == nil {
389366
return &os.PathError{Op: "chtimes", Path: name, Err: ErrFileNotFound}
390367
}
391368

392-
m.mu.Lock()
393369
mem.SetModTime(f, mtime)
394-
m.mu.Unlock()
395370

396371
return nil
397372
}

memmap_test.go

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,7 @@ func TestMemFsChmod(t *testing.T) {
614614
t.Fatal("mkdir failed to create a directory: mode =", info.Mode())
615615
}
616616

617-
err = fs.Chmod(file, 0)
617+
err = fs.Chmod(file, os.ModeTemporary|0355)
618618
if err != nil {
619619
t.Error("Failed to run chmod:", err)
620620
}
@@ -623,9 +623,29 @@ func TestMemFsChmod(t *testing.T) {
623623
if err != nil {
624624
t.Fatal(err)
625625
}
626-
if info.Mode().String() != "d---------" {
627-
t.Error("chmod should not change file type. New mode =", info.Mode())
626+
if info.Mode().String() != "d-wxr-xr-x" {
627+
t.Error("incorrect file mode after chmod:", info.Mode())
628628
}
629+
630+
f, err := fs.Open(file)
631+
if err != nil {
632+
t.Error("failed to open file:", err)
633+
}
634+
635+
err = f.Chmod(os.ModeNamedPipe | 0744)
636+
if err != nil {
637+
t.Error("Failed to run chmod:", err)
638+
}
639+
640+
info, err = fs.Stat(file)
641+
if err != nil {
642+
t.Fatal(err)
643+
}
644+
if info.Mode().String() != "drwxr--r--" {
645+
t.Error("incorrect file mode after chmod:", info.Mode())
646+
}
647+
648+
f.Close()
629649
}
630650

631651
// can't use Mkdir to get around which permissions we're allowed to set

regexpfs.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,10 @@ func (f *RegexpFile) WriteAt(s []byte, o int64) (int, error) {
178178
return f.f.WriteAt(s, o)
179179
}
180180

181+
func (f *RegexpFile) Chmod(mode os.FileMode) error {
182+
return f.f.Chmod(mode)
183+
}
184+
181185
func (f *RegexpFile) Name() string {
182186
return f.f.Name()
183187
}

sftpfs/file.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ func (f *File) Close() error {
4444
return f.fd.Close()
4545
}
4646

47+
func (f *File) Chmod(mode os.FileMode) error {
48+
return f.fd.Chmod(mode)
49+
}
50+
4751
func (f *File) Name() string {
4852
return f.fd.Name()
4953
}

tarfs/file.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,10 @@ func (f *File) Write(p []byte) (n int, err error) { return 0, syscall.EROFS }
7171

7272
func (f *File) WriteAt(p []byte, off int64) (n int, err error) { return 0, syscall.EROFS }
7373

74+
func (f *File) Chmod(mode os.FileMode) error {
75+
return syscall.EROFS
76+
}
77+
7478
func (f *File) Name() string {
7579
return filepath.Join(splitpath(f.h.Name))
7680
}

unionFile.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,20 @@ func (f *UnionFile) Close() error {
4141
return BADFD
4242
}
4343

44+
func (f *UnionFile) Chmod(mode os.FileMode) error {
45+
if f.Layer != nil {
46+
err := f.Layer.Chmod(mode)
47+
if err == nil && f.Base != nil {
48+
err = f.Base.Chmod(mode)
49+
}
50+
return err
51+
}
52+
if f.Base != nil {
53+
return f.Base.Chmod(mode)
54+
}
55+
return BADFD
56+
}
57+
4458
func (f *UnionFile) Read(s []byte) (int, error) {
4559
if f.Layer != nil {
4660
n, err := f.Layer.Read(s)

zipfs/file.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,10 @@ func (f *File) Write(p []byte) (n int, err error) { return 0, syscall.EPERM }
104104

105105
func (f *File) WriteAt(p []byte, off int64) (n int, err error) { return 0, syscall.EPERM }
106106

107+
func (f *File) Chmod(mode os.FileMode) error {
108+
return syscall.EPERM
109+
}
110+
107111
func (f *File) Name() string {
108112
if f.zipfile == nil {
109113
return string(filepath.Separator)

0 commit comments

Comments
 (0)