From fb6147ca946b2ec1ee562323bfd0d6577c27bf75 Mon Sep 17 00:00:00 2001 From: Crypt Keeper <64215+codefromthecrypt@users.noreply.github.com> Date: Sat, 22 Jul 2023 08:03:47 +0800 Subject: [PATCH] Emulates AT_SYMLINK_NOFOLLOW instead of sometimes implementing it (#1588) Signed-off-by: Adrian Cole --- imports/wasi_snapshot_preview1/fs.go | 13 +++++++++-- imports/wasi_snapshot_preview1/fs_test.go | 11 ++++----- internal/fsapi/fs.go | 8 +++---- internal/fsapi/unimplemented.go | 2 +- internal/gojs/fs.go | 2 +- internal/sysfs/adapter_test.go | 2 +- internal/sysfs/dirfs.go | 4 ++-- internal/sysfs/dirfs_test.go | 20 +++------------- internal/sysfs/futimens.go | 16 ++++--------- internal/sysfs/futimens_darwin.go | 14 ++++-------- internal/sysfs/futimens_linux.go | 14 ++++-------- internal/sysfs/futimens_test.go | 28 +++-------------------- internal/sysfs/futimens_unsupported.go | 9 ++++---- internal/sysfs/futimens_windows.go | 4 ++-- internal/sysfs/readfs.go | 2 +- internal/sysfs/readfs_test.go | 2 +- 16 files changed, 52 insertions(+), 99 deletions(-) diff --git a/imports/wasi_snapshot_preview1/fs.go b/imports/wasi_snapshot_preview1/fs.go index f0985a7961..033b70cb7d 100644 --- a/imports/wasi_snapshot_preview1/fs.go +++ b/imports/wasi_snapshot_preview1/fs.go @@ -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, ×, true) + errno = f.FS.Utimens(f.Name, ×) } return errno @@ -1456,7 +1456,16 @@ func pathFilestatSetTimesFn(_ context.Context, mod api.Module, params []uint64) } symlinkFollow := flags&wasip1.LOOKUP_SYMLINK_FOLLOW != 0 - return preopen.Utimens(pathName, ×, symlinkFollow) + if symlinkFollow { + return preopen.Utimens(pathName, ×) + } + // 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(×) + } } // pathLink is the WASI function named PathLinkName which adjusts the diff --git a/imports/wasi_snapshot_preview1/fs_test.go b/imports/wasi_snapshot_preview1/fs_test.go index 80d375df6e..fc8e504c48 100644 --- a/imports/wasi_snapshot_preview1/fs_test.go +++ b/imports/wasi_snapshot_preview1/fs_test.go @@ -10,7 +10,6 @@ import ( "os" "path" "runtime" - "strings" "testing" gofstest "testing/fstest" "time" @@ -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 diff --git a/internal/fsapi/fs.go b/internal/fsapi/fs.go index 25cc030273..50bf61687d 100644 --- a/internal/fsapi/fs.go +++ b/internal/fsapi/fs.go @@ -275,10 +275,8 @@ type FS interface { // 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. + // behaves the 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 // @@ -292,7 +290,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.. } diff --git a/internal/fsapi/unimplemented.go b/internal/fsapi/unimplemented.go index 6f2ab1f1c8..b31cbd13f5 100644 --- a/internal/fsapi/unimplemented.go +++ b/internal/fsapi/unimplemented.go @@ -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 } diff --git a/internal/gojs/fs.go b/internal/gojs/fs.go index 1422665355..ad223fd642 100644 --- a/internal/gojs/fs.go +++ b/internal/gojs/fs.go @@ -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, ×, true) + errno := fsc.RootFS().Utimens(path, ×) return jsfsInvoke(ctx, mod, callback, errno) } diff --git a/internal/sysfs/adapter_test.go b/internal/sysfs/adapter_test.go index 9cd77660d0..f79f4bdcaa 100644 --- a/internal/sysfs/adapter_test.go +++ b/internal/sysfs/adapter_test.go @@ -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) } diff --git a/internal/sysfs/dirfs.go b/internal/sysfs/dirfs.go index 5f3f01eb78..c908d6c550 100644 --- a/internal/sysfs/dirfs.go +++ b/internal/sysfs/dirfs.go @@ -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 { diff --git a/internal/sysfs/dirfs_test.go b/internal/sysfs/dirfs_test.go index 7f3de3baf8..078df6df3c 100644 --- a/internal/sysfs/dirfs_test.go +++ b/internal/sysfs/dirfs_test.go @@ -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 @@ -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() @@ -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: @@ -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) diff --git a/internal/sysfs/futimens.go b/internal/sysfs/futimens.go index e9a445d9a9..9144126b61 100644 --- a/internal/sysfs/futimens.go +++ b/internal/sysfs/futimens.go @@ -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 // @@ -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) } @@ -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() diff --git a/internal/sysfs/futimens_darwin.go b/internal/sysfs/futimens_darwin.go index f4ede33778..a5663ede3c 100644 --- a/internal/sysfs/futimens_darwin.go +++ b/internal/sysfs/futimens_darwin.go @@ -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 { var flags int - if !symlinkFollow { - flags = _AT_SYMLINK_NOFOLLOW - } return utimensat(_AT_FDCWD, path, times, flags) } diff --git a/internal/sysfs/futimens_linux.go b/internal/sysfs/futimens_linux.go index a7ae264d2b..5008ca8140 100644 --- a/internal/sysfs/futimens_linux.go +++ b/internal/sysfs/futimens_linux.go @@ -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 { diff --git a/internal/sysfs/futimens_test.go b/internal/sysfs/futimens_test.go index 3cd2210839..b87d46f1a0 100644 --- a/internal/sysfs/futimens_test.go +++ b/internal/sysfs/futimens_test.go @@ -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) } @@ -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() @@ -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: @@ -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 diff --git a/internal/sysfs/futimens_unsupported.go b/internal/sysfs/futimens_unsupported.go index f1602bdb01..2816cb84ee 100644 --- a/internal/sysfs/futimens_unsupported.go +++ b/internal/sysfs/futimens_unsupported.go @@ -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 { diff --git a/internal/sysfs/futimens_windows.go b/internal/sysfs/futimens_windows.go index 4d252d3cd5..26d9c2a425 100644 --- a/internal/sysfs/futimens_windows.go +++ b/internal/sysfs/futimens_windows.go @@ -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 { diff --git a/internal/sysfs/readfs.go b/internal/sysfs/readfs.go index 76d182c1b8..1e96e2b4dd 100644 --- a/internal/sysfs/readfs.go +++ b/internal/sysfs/readfs.go @@ -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 } diff --git a/internal/sysfs/readfs_test.go b/internal/sysfs/readfs_test.go index f1667fd498..b7b2c60951 100644 --- a/internal/sysfs/readfs_test.go +++ b/internal/sysfs/readfs_test.go @@ -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) }