Emulates AT_SYMLINK_NOFOLLOW instead of sometimes implementing it (#1588

)

Signed-off-by: Adrian Cole <adrian@tetrate.io>

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, &times, true)
+		errno = f.FS.Utimens(f.Name, &times)
 	}
 
 	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, &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

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

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..
 }

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
 }
 

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, &times, true)
+	errno := fsc.RootFS().Utimens(path, &times)
 
 	return jsfsInvoke(ctx, mod, callback, errno)
 }

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)
 }
 

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 {

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)

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()

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)
 }
 

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 {

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

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 {

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 {

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
 }
 

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)
 }