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.

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

Emulates AT_SYMLINK_NOFOLLOW instead of sometimes implementing it #1588

Merged
merged 3 commits into from
Jul 22, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions imports/wasi_snapshot_preview1/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ func fdFilestatSetTimesFn(_ context.Context, mod api.Module, params []uint64) ex
// Fall back to path based, despite it being less precise.
switch errno {
case experimentalsys.EPERM, experimentalsys.ENOSYS:
errno = f.FS.Utimens(f.Name, &times, true)
errno = f.FS.Utimens(f.Name, &times)
}

return errno
Expand Down Expand Up @@ -1456,7 +1456,16 @@ func pathFilestatSetTimesFn(_ context.Context, mod api.Module, params []uint64)
}

symlinkFollow := flags&wasip1.LOOKUP_SYMLINK_FOLLOW != 0
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the flag is the opposite of the POSIX one in wasip1, presumably to avoid accidentally setting it by default.

return preopen.Utimens(pathName, &times, symlinkFollow)
if symlinkFollow {
return preopen.Utimens(pathName, &times)
}
// Otherwise, we need to emulate don't follow by opening the file by path.
if f, errno := preopen.OpenFile(pathName, syscall.O_WRONLY, 0); errno != 0 {
return errno
} else {
defer f.Close()
return f.Utimens(&times)
}
}

// pathLink is the WASI function named PathLinkName which adjusts the
Expand Down
11 changes: 5 additions & 6 deletions imports/wasi_snapshot_preview1/fs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"os"
"path"
"runtime"
"strings"
"testing"
gofstest "testing/fstest"
"time"
Expand Down Expand Up @@ -3495,17 +3494,17 @@ func Test_pathFilestatSetTimes(t *testing.T) {
},
}

if runtime.GOOS == "windows" && !platform.IsAtLeastGo120 {
// Windows 1.18 (possibly 1.19) returns ENOSYS on no_symlink_follow
tests = tests[:len(tests)-1]
}

for _, tt := range tests {
tc := tt

t.Run(tc.name, func(t *testing.T) {
defer log.Reset()

if tc.flags == 0 && !sysfs.SupportsSymlinkNoFollow {
tc.expectedErrno = wasip1.ErrnoNosys
tc.expectedLog = strings.ReplaceAll(tc.expectedLog, "ESUCCESS", "ENOSYS")
}

pathName := tc.pathName
if pathName == "" {
pathName = file
Expand Down
5 changes: 1 addition & 4 deletions internal/fsapi/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,9 +277,6 @@ type FS interface {
// may be specified instead of real timestamps. A nil `times` parameter
// behaves the same as if both were set to UTIME_NOW.
//
// When the `symlinkFollow` parameter is true and the path is a symbolic link,
// the target of expanding that link is updated.
//
// # Errors
//
// A zero Errno is success. The below are expected otherwise:
Expand All @@ -292,7 +289,7 @@ type FS interface {
//
// - This is like syscall.UtimesNano and `utimensat` with `AT_FDCWD` in
// POSIX. See https://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html
Utimens(path string, times *[2]syscall.Timespec, symlinkFollow bool) experimentalsys.Errno
Utimens(path string, times *[2]syscall.Timespec) experimentalsys.Errno
// TODO: change impl to not use syscall package,
// possibly by being just a pair of int64s..
}
2 changes: 1 addition & 1 deletion internal/fsapi/unimplemented.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func (UnimplementedFS) Unlink(path string) experimentalsys.Errno {
}

// Utimens implements FS.Utimens
func (UnimplementedFS) Utimens(path string, times *[2]syscall.Timespec, symlinkFollow bool) experimentalsys.Errno {
func (UnimplementedFS) Utimens(path string, times *[2]syscall.Timespec) experimentalsys.Errno {
return experimentalsys.ENOSYS
}

Expand Down
2 changes: 1 addition & 1 deletion internal/gojs/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ func (u *jsfsUtimes) invoke(ctx context.Context, mod api.Module, args ...interfa
times := [2]syscall.Timespec{
syscall.NsecToTimespec(atimeSec * 1e9), syscall.NsecToTimespec(mtimeSec * 1e9),
}
errno := fsc.RootFS().Utimens(path, &times, true)
errno := fsc.RootFS().Utimens(path, &times)

return jsfsInvoke(ctx, mod, callback, errno)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/sysfs/adapter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func TestAdapt_UtimesNano(t *testing.T) {
realPath := joinPath(tmpDir, path)
require.NoError(t, os.WriteFile(realPath, []byte{}, 0o600))

err := testFS.Utimens(path, nil, true)
err := testFS.Utimens(path, nil)
require.EqualErrno(t, experimentalsys.ENOSYS, err)
}

Expand Down
4 changes: 2 additions & 2 deletions internal/sysfs/dirfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ func (d *dirFS) Symlink(oldName, link string) experimentalsys.Errno {
}

// Utimens implements the same method as documented on fsapi.FS
func (d *dirFS) Utimens(path string, times *[2]syscall.Timespec, symlinkFollow bool) experimentalsys.Errno {
return Utimens(d.join(path), times, symlinkFollow)
func (d *dirFS) Utimens(path string, times *[2]syscall.Timespec) experimentalsys.Errno {
return Utimens(d.join(path), times)
}

func (d *dirFS) join(path string) string {
Expand Down
20 changes: 3 additions & 17 deletions internal/sysfs/dirfs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -531,14 +531,8 @@ func TestDirFS_Utimesns(t *testing.T) {
require.NoError(t, err)

t.Run("doesn't exist", func(t *testing.T) {
err := testFS.Utimens("nope", nil, true)
err := testFS.Utimens("nope", nil)
require.EqualErrno(t, sys.ENOENT, err)
err = testFS.Utimens("nope", nil, false)
if SupportsSymlinkNoFollow {
require.EqualErrno(t, sys.ENOENT, err)
} else {
require.EqualErrno(t, sys.ENOSYS, err)
}
})

// Note: This sets microsecond granularity because Windows doesn't support
Expand Down Expand Up @@ -603,12 +597,11 @@ func TestDirFS_Utimesns(t *testing.T) {
},
}

for _, fileType := range []string{"dir", "file", "link", "link-follow"} {
for _, fileType := range []string{"dir", "file", "link"} {
for _, tt := range tests {
tc := tt
fileType := fileType
name := fileType + " " + tc.name
symlinkNoFollow := fileType == "link"

t.Run(name, func(t *testing.T) {
tmpDir := t.TempDir()
Expand All @@ -634,9 +627,6 @@ func TestDirFS_Utimesns(t *testing.T) {
path = "file"
statPath = "file"
case "link":
path = "file-link"
statPath = "file-link"
case "link-follow":
path = "file-link"
statPath = "file"
default:
Expand All @@ -646,11 +636,7 @@ func TestDirFS_Utimesns(t *testing.T) {
oldSt, errno := testFS.Lstat(statPath)
require.EqualErrno(t, 0, errno)

errno = testFS.Utimens(path, tc.times, !symlinkNoFollow)
if symlinkNoFollow && !SupportsSymlinkNoFollow {
require.EqualErrno(t, sys.ENOSYS, errno)
return
}
errno = testFS.Utimens(path, tc.times)
require.EqualErrno(t, 0, errno)

newSt, errno := testFS.Lstat(statPath)
Expand Down
16 changes: 5 additions & 11 deletions internal/sysfs/futimens.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,8 @@ const (
// The `times` parameter includes the access and modification timestamps to
// assign. Special syscall.Timespec NSec values UTIME_NOW and UTIME_OMIT may be
// specified instead of real timestamps. A nil `times` parameter behaves the
// same as if both were set to UTIME_NOW.
//
// When the `symlinkFollow` parameter is true and the path is a symbolic link,
// the target of expanding that link is updated.
// same as if both were set to UTIME_NOW. If the path is a symbolic link, the
// target of expanding that link is updated.
//
// # Errors
//
Expand All @@ -45,8 +43,8 @@ const (
//
// - This is like syscall.UtimesNano and `utimensat` with `AT_FDCWD` in
// POSIX. See https://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html
func Utimens(path string, times *[2]syscall.Timespec, symlinkFollow bool) experimentalsys.Errno {
err := utimens(path, times, symlinkFollow)
func Utimens(path string, times *[2]syscall.Timespec) experimentalsys.Errno {
err := utimens(path, times)
return experimentalsys.UnwrapOSError(err)
}

Expand All @@ -57,11 +55,7 @@ func timesToPtr(times *[2]syscall.Timespec) unsafe.Pointer { //nolint:unused
return unsafe.Pointer(nil)
}

func utimensPortable(path string, times *[2]syscall.Timespec, symlinkFollow bool) error { //nolint:unused
if !symlinkFollow {
return experimentalsys.ENOSYS
}

func utimensPortable(path string, times *[2]syscall.Timespec) error { //nolint:unused
// Handle when both inputs are current system time.
if times == nil || times[0].Nsec == UTIME_NOW && times[1].Nsec == UTIME_NOW {
ts := nowTimespec()
Expand Down
14 changes: 5 additions & 9 deletions internal/sysfs/futimens_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,18 @@ import (
)

const (
_AT_FDCWD = -0x2
_AT_SYMLINK_NOFOLLOW = 0x0020
_UTIME_NOW = -1
_UTIME_OMIT = -2
SupportsSymlinkNoFollow = true
_AT_FDCWD = -0x2
_AT_SYMLINK_NOFOLLOW = 0x0020
_UTIME_NOW = -1
_UTIME_OMIT = -2
)

//go:noescape
//go:linkname utimensat syscall.utimensat
func utimensat(dirfd int, path string, times *[2]syscall.Timespec, flags int) error

func utimens(path string, times *[2]syscall.Timespec, symlinkFollow bool) error {
func utimens(path string, times *[2]syscall.Timespec) error {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a later PR will switch this to os.Chtimes which is basically the same logic (once we remove the special OMIT and NOW constants)

var flags int
if !symlinkFollow {
flags = _AT_SYMLINK_NOFOLLOW
}
return utimensat(_AT_FDCWD, path, times, flags)
}

Expand Down
14 changes: 4 additions & 10 deletions internal/sysfs/futimens_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,13 @@ import (
)

const (
_AT_FDCWD = -0x64
_AT_SYMLINK_NOFOLLOW = 0x100
_UTIME_NOW = (1 << 30) - 1
_UTIME_OMIT = (1 << 30) - 2
SupportsSymlinkNoFollow = true
_AT_FDCWD = -0x64
_UTIME_NOW = (1 << 30) - 1
_UTIME_OMIT = (1 << 30) - 2
)

func utimens(path string, times *[2]syscall.Timespec, symlinkFollow bool) (err error) {
func utimens(path string, times *[2]syscall.Timespec) (err error) {
var flags int
if !symlinkFollow {
flags = _AT_SYMLINK_NOFOLLOW
}

var _p0 *byte
_p0, err = syscall.BytePtrFromString(path)
if err != nil {
Expand Down
28 changes: 3 additions & 25 deletions internal/sysfs/futimens_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,8 @@ import (

func TestUtimens(t *testing.T) {
t.Run("doesn't exist", func(t *testing.T) {
err := Utimens("nope", nil, true)
err := Utimens("nope", nil)
require.EqualErrno(t, sys.ENOENT, err)

err = Utimens("nope", nil, false)
if SupportsSymlinkNoFollow {
require.EqualErrno(t, sys.ENOENT, err)
} else {
require.EqualErrno(t, sys.ENOSYS, err)
}
})
testUtimens(t, false)
}
Expand Down Expand Up @@ -105,19 +98,11 @@ func testUtimens(t *testing.T, futimes bool) {
},
},
}
for _, fileType := range []string{"dir", "file", "link", "link-follow"} {
for _, fileType := range []string{"dir", "file", "link"} {
for _, tt := range tests {
tc := tt
fileType := fileType
name := fileType + " " + tc.name
symlinkNoFollow := fileType == "link"

// symlinkNoFollow is invalid for file descriptor based operations,
// because the default for open is to follow links. You can't avoid
// this. O_NOFOLLOW is used only to return ELOOP on a link.
if futimes && symlinkNoFollow {
continue
}

t.Run(name, func(t *testing.T) {
tmpDir := t.TempDir()
Expand All @@ -141,9 +126,6 @@ func testUtimens(t *testing.T, futimes bool) {
path = file
statPath = file
case "link":
path = link
statPath = link
case "link-follow":
path = link
statPath = file
default:
Expand All @@ -154,11 +136,7 @@ func testUtimens(t *testing.T, futimes bool) {
require.EqualErrno(t, 0, errno)

if !futimes {
err = Utimens(path, tc.times, !symlinkNoFollow)
if symlinkNoFollow && !SupportsSymlinkNoFollow {
require.EqualErrno(t, sys.ENOSYS, err)
return
}
errno = Utimens(path, tc.times)
require.EqualErrno(t, 0, errno)
} else {
flag := fsapi.O_RDWR
Expand Down
9 changes: 4 additions & 5 deletions internal/sysfs/futimens_unsupported.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,12 @@ import (

// Define values even if not used except as sentinels.
const (
_UTIME_NOW = -1
_UTIME_OMIT = -2
SupportsSymlinkNoFollow = false
_UTIME_NOW = -1
_UTIME_OMIT = -2
)

func utimens(path string, times *[2]syscall.Timespec, symlinkFollow bool) error {
return utimensPortable(path, times, symlinkFollow)
func utimens(path string, times *[2]syscall.Timespec) error {
return utimensPortable(path, times)
}

func futimens(fd uintptr, times *[2]syscall.Timespec) error {
Expand Down
4 changes: 2 additions & 2 deletions internal/sysfs/futimens_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ const (
SupportsSymlinkNoFollow = false
)

func utimens(path string, times *[2]syscall.Timespec, symlinkFollow bool) error {
return utimensPortable(path, times, symlinkFollow)
func utimens(path string, times *[2]syscall.Timespec) error {
return utimensPortable(path, times)
}

func futimens(fd uintptr, times *[2]syscall.Timespec) error {
Expand Down
2 changes: 1 addition & 1 deletion internal/sysfs/readfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func (r *readFS) Unlink(path string) experimentalsys.Errno {
}

// Utimens implements the same method as documented on fsapi.FS
func (r *readFS) Utimens(path string, times *[2]syscall.Timespec, symlinkFollow bool) experimentalsys.Errno {
func (r *readFS) Utimens(path string, times *[2]syscall.Timespec) experimentalsys.Errno {
return experimentalsys.EROFS
}

Expand Down
2 changes: 1 addition & 1 deletion internal/sysfs/readfs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func TestReadFS_UtimesNano(t *testing.T) {
realPath := joinPath(tmpDir, path)
require.NoError(t, os.WriteFile(realPath, []byte{}, 0o600))

err := testFS.Utimens(path, nil, true)
err := testFS.Utimens(path, nil)
require.EqualErrno(t, sys.EROFS, err)
}

Expand Down