Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve handling of mode bits #250

Merged
merged 1 commit into from Aug 2, 2020
Merged

Improve handling of mode bits #250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 15 additions & 8 deletions memmap.go
View file
Open in desktop
Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand All @@ -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()
Expand Down
147 changes: 147 additions & 0 deletions memmap_test.go
View file
Open in desktop
Expand Up @@ -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)
Expand Down Expand Up @@ -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())
}
}