mirror
/
afero
mirror of https://github.com/spf13/afero.git
1
0
Fork 1
Code Issues Packages Projects Releases Wiki Activity

Improve handling of mode bits 

- "/" should have mode `os.ModeDir|0755`, not `0000`. Among other
  things, this had resulted in `mode.IsDir()` returning false for root
  prior to this patch.
- `Mkdir`, `MkdirAll`, and `OpenFile` shouldn't be allowed to set
  permissions that are otherwise illegal through `Chmod`. This mirrors
  what Go's `os` package does: it calls `syscallMode(mode)`, which
  effectively clears out the same bits that are disallowed by `Chmod`.
- `MkdirAll` should use the given permissions for all intermediate
  directories that are created, not just for the final directory. Prior
  to this patch, intermediate directories were created with mode bits
  `0000`. Besides the permission bits being wrong, `mode.IsDir()` would
  return false for these directories prior to this patch.
This commit is contained in:
Anish Athalye 2020-07-15 13:48:32 -04:00
parent 5d9e780691
commit b598fbbf55
2 changed files with 162 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
memmap.go
View File

@ -25,6 +25,8 @@ 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
@ -40,7 +42,9 @@ func (m *MemMapFs) getData() map[string]*mem.FileData {
m.data = make(map[string]*mem.FileData)
// Root should always exist, right?
// TODO: what about windows?
m.data[FilePathSeparator] = mem.CreateDir(FilePathSeparator)
root := mem.CreateDir(FilePathSeparator)
mem.SetMode(root, os.ModeDir|0755)
m.data[FilePathSeparator] = root
})
return m.data
}
@ -52,7 +56,7 @@ func (m *MemMapFs) Create(name string) (File, error) {
m.mu.Lock()
file := mem.CreateFile(name)
m.getData()[name] = file
m.registerWithParent(file)
m.registerWithParent(file, 0)
m.mu.Unlock()
return mem.NewFileHandle(file), nil
}
@ -83,14 +87,14 @@ func (m *MemMapFs) findParent(f *mem.FileData) *mem.FileData {
return pfile
}
func (m *MemMapFs) registerWithParent(f *mem.FileData) {
func (m *MemMapFs) registerWithParent(f *mem.FileData, perm os.FileMode) {
if f == nil {
return
}
parent := m.findParent(f)
if parent == nil {
pdir := filepath.Dir(filepath.Clean(f.Name()))
err := m.lockfreeMkdir(pdir, 0777)
err := m.lockfreeMkdir(pdir, perm)
if err != nil {
//log.Println("Mkdir error:", err)
return
@ -119,13 +123,15 @@ func (m *MemMapFs) lockfreeMkdir(name string, perm os.FileMode) error {
}
} else {
item := mem.CreateDir(name)
mem.SetMode(item, os.ModeDir|perm)
m.getData()[name] = item
m.registerWithParent(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()
@ -137,8 +143,9 @@ func (m *MemMapFs) Mkdir(name string, perm os.FileMode) error {
m.mu.Lock()
item := mem.CreateDir(name)
mem.SetMode(item, os.ModeDir|perm)
m.getData()[name] = item
m.registerWithParent(item)
m.registerWithParent(item, perm)
m.mu.Unlock()
return m.setFileMode(name, perm|os.ModeDir)
@ -208,6 +215,7 @@ 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) {
@ -300,7 +308,7 @@ func (m *MemMapFs) Rename(oldname, newname string) error {
delete(m.getData(), oldname)
mem.ChangeFileName(fileData, newname)
m.getData()[newname] = fileData
m.registerWithParent(fileData)
m.registerWithParent(fileData, 0)
m.mu.Unlock()
m.mu.RLock()
} else {
@ -319,7 +327,6 @@ func (m *MemMapFs) Stat(name string) (os.FileInfo, error) {
}
func (m *MemMapFs) Chmod(name string, mode os.FileMode) error {
const chmodBits = os.ModePerm | os.ModeSetuid | os.ModeSetgid | os.ModeSticky // Only a subset of bits are allowed to be changed. Documented under os.Chmod()
mode &= chmodBits
m.mu.RLock()

147
memmap_test.go
View File

@ -412,6 +412,116 @@ loop:
}
}
// root is a directory
func TestMemFsRootDirMode(t *testing.T) {
t.Parallel()
fs := NewMemMapFs()
info, err := fs.Stat("/")
if err != nil {
t.Fatal(err)
}
if !info.IsDir() {
t.Error("should be a directory")
}
if !info.Mode().IsDir() {
t.Errorf("FileMode is not directory, is %s", info.Mode().String())
}
}
// MkdirAll creates intermediate directories with correct mode
func TestMemFsMkdirAllMode(t *testing.T) {
t.Parallel()
fs := NewMemMapFs()
err := fs.MkdirAll("/a/b/c", 0755)
if err != nil {
t.Fatal(err)
}
info, err := fs.Stat("/a")
if err != nil {
t.Fatal(err)
}
if !info.Mode().IsDir() {
t.Error("/a: mode is not directory")
}
if info.Mode() != os.FileMode(os.ModeDir|0755) {
t.Errorf("/a: wrong permissions, expected drwxr-xr-x, got %s", info.Mode())
}
info, err = fs.Stat("/a/b")
if err != nil {
t.Fatal(err)
}
if !info.Mode().IsDir() {
t.Error("/a/b: mode is not directory")
}
if info.Mode() != os.FileMode(os.ModeDir|0755) {
t.Errorf("/a/b: wrong permissions, expected drwxr-xr-x, got %s", info.Mode())
}
info, err = fs.Stat("/a/b/c")
if err != nil {
t.Fatal(err)
}
if !info.Mode().IsDir() {
t.Error("/a/b/c: mode is not directory")
}
if info.Mode() != os.FileMode(os.ModeDir|0755) {
t.Errorf("/a/b/c: wrong permissions, expected drwxr-xr-x, got %s", info.Mode())
}
}
// MkdirAll does not change permissions of already-existing directories
func TestMemFsMkdirAllNoClobber(t *testing.T) {
t.Parallel()
fs := NewMemMapFs()
err := fs.MkdirAll("/a/b/c", 0755)
if err != nil {
t.Fatal(err)
}
info, err := fs.Stat("/a/b")
if err != nil {
t.Fatal(err)
}
if info.Mode() != os.FileMode(os.ModeDir|0755) {
t.Errorf("/a/b: wrong permissions, expected drwxr-xr-x, got %s", info.Mode())
}
err = fs.MkdirAll("/a/b/c/d/e/f", 0710)
// '/a/b' is unchanged
if err != nil {
t.Fatal(err)
}
info, err = fs.Stat("/a/b")
if err != nil {
t.Fatal(err)
}
if info.Mode() != os.FileMode(os.ModeDir|0755) {
t.Errorf("/a/b: wrong permissions, expected drwxr-xr-x, got %s", info.Mode())
}
// new directories created with proper permissions
info, err = fs.Stat("/a/b/c/d")
if err != nil {
t.Fatal(err)
}
if info.Mode() != os.FileMode(os.ModeDir|0710) {
t.Errorf("/a/b/c/d: wrong permissions, expected drwx--x---, got %s", info.Mode())
}
info, err = fs.Stat("/a/b/c/d/e")
if err != nil {
t.Fatal(err)
}
if info.Mode() != os.FileMode(os.ModeDir|0710) {
t.Errorf("/a/b/c/d/e: wrong permissions, expected drwx--x---, got %s", info.Mode())
}
info, err = fs.Stat("/a/b/c/d/e/f")
if err != nil {
t.Fatal(err)
}
if info.Mode() != os.FileMode(os.ModeDir|0710) {
t.Errorf("/a/b/c/d/e/f: wrong permissions, expected drwx--x---, got %s", info.Mode())
}
}
func TestMemFsDirMode(t *testing.T) {
fs := NewMemMapFs()
err := fs.Mkdir("/testDir1", 0644)
@ -503,3 +613,40 @@ func TestMemFsChmod(t *testing.T) {
t.Error("chmod should not change file type. New mode =", info.Mode())
}
}
// can't use Mkdir to get around which permissions we're allowed to set
func TestMemFsMkdirModeIllegal(t *testing.T) {
t.Parallel()
fs := NewMemMapFs()
err := fs.Mkdir("/a", os.ModeSocket|0755)
if err != nil {
t.Fatal(err)
}
info, err := fs.Stat("/a")
if err != nil {
t.Fatal(err)
}
if info.Mode() != os.FileMode(os.ModeDir|0755) {
t.Fatalf("should not be able to use Mkdir to set illegal mode: %s", info.Mode().String())
}
}
// can't use OpenFile to get around which permissions we're allowed to set
func TestMemFsOpenFileModeIllegal(t *testing.T) {
t.Parallel()
fs := NewMemMapFs()
file, err := fs.OpenFile("/a", os.O_CREATE, os.ModeSymlink|0644)
if err != nil {
t.Fatal(err)
}
defer file.Close()
info, err := fs.Stat("/a")
if err != nil {
t.Fatal(err)
}
if info.Mode() != os.FileMode(0644) {
t.Fatalf("should not be able to use OpenFile to set illegal mode: %s", info.Mode().String())
}
}