Skip to content

Commit

Permalink
Return ENOTSUP 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 ENOTSUP for now (instead of doing the wrong thing and following
the symlink).
  • Loading branch information
@jmmv
jmmv committed Nov 5, 2018
1 parent 6613a3c commit d8955b2
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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.ENOTSUP {
t.Fatalf("Expected ENOTSUP changing the times of a symlink; got %v", err)
}
})
}
Expand Down
7 changes: 7 additions & 0 deletions internal/sandbox/node.go
View file
Original file line number Diff line number Diff line change
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.ENOTSUP
}

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
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,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::ENOTSUP));
}

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 d8955b2

Please sign in to comment.