Skip to content

Commit

Permalink
Return EOPNOTSUPP when trying to setattr the times of a symlink
Browse files Browse the repository at this point in the history 
The setattr hook should be using futimensat to update the timestamps
of the node so that we could support the case of modifying those of
a symlink (without following it).

Unfortunately, we currently cannot do so because our testing environment
does not have this system call available in all platforms... so just
return EOPNOTSUPP for now (instead of doing the wrong thing and following
the symlink).
  • Loading branch information
@jmmv
jmmv committed Nov 9, 2018
1 parent af0fc43 commit 8c2bdc3
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 46 deletions.
2 changes: 1 addition & 1 deletion WORKSPACE
View file
Expand Up @@ -57,7 +57,7 @@ go_repository(
go_repository(
name = "org_golang_x_sys",
importpath = "golang.org/x/sys",
commit = "4b45465282a4624cf39876842a017334f13b8aff",
commit = "9b800f95dbbc54abff0acf7ee32d88ba4e328c89",
)

gazelle_dependencies()
Expand Down
58 changes: 18 additions & 40 deletions integration/read_write_test.go
View file
Expand Up @@ -24,6 +24,8 @@ import (
"testing"
"time"

"golang.org/x/sys/unix"

"github.com/bazelbuild/sandboxfs/integration/utils"
"github.com/bazelbuild/sandboxfs/internal/sandbox"
)
Expand Down Expand Up @@ -565,20 +567,11 @@ func TestReadWrite_Chmod(t *testing.T) {
}
})

t.Run("DanglingSymlink", func(t *testing.T) {
utils.MustSymlink(t, "missing", state.RootPath("dangling-symlink"))

path := state.MountPath("dangling-symlink")
if err := os.Chmod(path, 0555); err == nil {
t.Errorf("Want chmod to fail on dangling link, got success")
}
})

t.Run("GoodSymlink", func(t *testing.T) {
t.Run("Symlink", func(t *testing.T) {
utils.MustWriteFile(t, state.RootPath("target"), 0644, "")
utils.MustSymlink(t, "target", state.RootPath("good-symlink"))
utils.MustSymlink(t, "target", state.RootPath("symlink"))

path := state.MountPath("good-symlink")
path := state.MountPath("symlink")
linkFileInfo, err := os.Lstat(path)
if err != nil {
t.Fatalf("Failed to stat %s: %v", path, err)
Expand All @@ -588,11 +581,11 @@ func TestReadWrite_Chmod(t *testing.T) {
t.Fatalf("Failed to chmod %s: %v", path, err)
}

if err := checkPerm("good-symlink", linkFileInfo.Mode()&os.ModePerm); err != nil {
if err := checkPerm("symlink", linkFileInfo.Mode()&os.ModePerm); err != nil {
t.Error(err)
}
if err := checkPerm("target", 0200); err != nil {
t.Error(err)
t.Errorf("Mode of symlink target was modified but shouldn't have been: %v", err)
}
})
}
Expand Down Expand Up @@ -664,9 +657,8 @@ func TestReadWrite_Chown(t *testing.T) {

utils.MustMkdirAll(t, state.RootPath("dir"), 0755)
utils.MustWriteFile(t, state.RootPath("file"), 0644, "new content")
utils.MustSymlink(t, "missing", state.RootPath("dangling-symlink"))
utils.MustWriteFile(t, state.RootPath("target"), 0644, "")
utils.MustSymlink(t, "target", state.RootPath("good-symlink"))
utils.MustSymlink(t, "target", state.RootPath("symlink"))

targetFileInfo, err := os.Lstat(state.RootPath("target"))
if err != nil {
Expand All @@ -683,8 +675,7 @@ func TestReadWrite_Chown(t *testing.T) {
}{
{"Dir", "dir", 1, 2},
{"File", "file", 3, 4},
{"DanglingSymlink", "dangling-symlink", 5, 6},
{"GoodSymlink", "good-symlink", 7, 8},
{"Symlink", "symlink", 7, 8},
}
for _, d := range testData {
t.Run(d.name, func(t *testing.T) {
Expand Down Expand Up @@ -821,35 +812,22 @@ func TestReadWrite_Chtimes(t *testing.T) {
}
})

t.Run("DanglingSymlink", func(t *testing.T) {
utils.MustSymlink(t, "missing", state.RootPath("dangling-symlink"))

if _, err := chtimes("dangling-symlink", time.Unix(0, 0), time.Unix(0, 0)); err == nil {
t.Errorf("Want chtimes to fail on dangling link, got success")
}
})

t.Run("GoodSymlink", func(t *testing.T) {
t.Run("Symlink", func(t *testing.T) {
utils.MustWriteFile(t, state.RootPath("target"), 0644, "")
utils.MustSymlink(t, "target", state.RootPath("good-symlink"))
path := state.MountPath("good-symlink")
utils.MustSymlink(t, "target", state.RootPath("symlink"))
path := state.MountPath("symlink")

linkFileInfo, err := os.Lstat(path)
atimeTimespec, err := unix.TimeToTimespec(someAtime)
if err != nil {
t.Fatalf("Failed to stat %s: %v", path, err)
t.Fatalf("Failed to convert %v to a timespec: %v", someAtime, err)
}
linkStat := linkFileInfo.Sys().(*syscall.Stat_t)

wantMinCtime, err := chtimes(path, someAtime, someMtime)
mtimeTimespec, err := unix.TimeToTimespec(someMtime)
if err != nil {
t.Fatal(err)
t.Fatalf("Failed to convert %v to a timespec: %v", someMtime, err)
}

if err := checkTimes("good-symlink", time.Unix(0, 0), linkFileInfo.ModTime(), sandbox.Ctime(linkStat)); err != nil {
t.Error(err)
}
if err := checkTimes("target", someAtime, someMtime, wantMinCtime); err != nil {
t.Error(err)
if err = unix.UtimesNanoAt(unix.AT_FDCWD, path, []unix.Timespec{atimeTimespec, mtimeTimespec}, unix.AT_SYMLINK_NOFOLLOW); err == nil || err != syscall.EOPNOTSUPP {
t.Fatalf("Expected EOPNOTSUPP changing the times of a symlink; got %v", err)
}
})
}
Expand Down
7 changes: 7 additions & 0 deletions internal/sandbox/node.go
View file
Expand Up @@ -342,6 +342,13 @@ func (n *BaseNode) setattrTimes(req *fuse.SetattrRequest) error {
}

if underlyingPath, isMapped := n.UnderlyingPath(); isMapped && !n.deleted {
if n.attr.Mode&os.ModeType == os.ModeSymlink {
// TODO(https://github.com/bazelbuild/sandboxfs/issues/46): Should use
// futimensat to support changing the times of a symlink if requested to
// do so.
return fuse.Errno(syscall.EOPNOTSUPP)
}

if err := os.Chtimes(underlyingPath, atime, mtime); err != nil {
return err
}
Expand Down
11 changes: 6 additions & 5 deletions src/nodes/mod.rs
View file
Expand Up @@ -143,13 +143,14 @@ fn setattr_times(attr: &mut fuse::FileAttr, path: Option<&PathBuf>,
return Ok(());
}

let atime = atime.unwrap_or_else(|| conv::timespec_to_timeval(attr.atime));
let mtime = mtime.unwrap_or_else(|| conv::timespec_to_timeval(attr.mtime));
if attr.kind == fuse::FileType::Symlink {
// TODO(jmmv): Should use futimensat to avoid following symlinks but this function is
// not available on macOS El Capitan, which we currently require for CI builds.
warn!("Asked to update atime/mtime for symlink {:?} but following symlink instead", path);
// TODO(https://github.com/bazelbuild/sandboxfs/issues/46): Should use futimensat to support
// changing the times of a symlink if requested to do so.
return Err(nix::Error::from_errno(Errno::EOPNOTSUPP));
}

let atime = atime.unwrap_or_else(|| conv::timespec_to_timeval(attr.atime));
let mtime = mtime.unwrap_or_else(|| conv::timespec_to_timeval(attr.mtime));
let result = try_path(path, |p| sys::stat::utimes(p, &atime, &mtime));
if result.is_ok() {
attr.atime = conv::timeval_to_timespec(atime);
Expand Down

0 comments on commit 8c2bdc3

Please sign in to comment.