Repository: tetratelabs/wazero
Pull request #1093 (2023-02-02 06:41:21)

support O_DIRECTORY and O_NOFOLLOW
Follow up to our discussion on Slack, this PR adds support for setting the O_DIRECTORY and O_NOFOLLOW flags when the wasi flags O_DIRECTORY and LOOKUP_SYMLINK_FOLLOW are set.

Signed-off-by: Achille Roussel <achille.roussel@gmail.com>

mathetake: rebased && fixed GOOS=js

Patch:
diff --git a/imports/wasi_snapshot_preview1/fs.go b/imports/wasi_snapshot_preview1/fs.go
index 4625a8d7..e3ec00d1 100644
--- a/imports/wasi_snapshot_preview1/fs.go
+++ b/imports/wasi_snapshot_preview1/fs.go
@@ -1545,7 +1545,7 @@ func pathOpenFn(_ context.Context, mod api.Module, params []uint64) Errno {
 
 	// TODO: dirflags is a lookupflags, and it only has one bit: symlink_follow
 	// https://github.com/WebAssembly/WASI/blob/snapshot-01/phases/snapshot/docs.md#lookupflags
-	_ /* dirflags */ = uint32(params[1])
+	dirflags := uint16(params[1])
 
 	path := uint32(params[2])
 	pathLen := uint32(params[3])
@@ -1563,7 +1563,8 @@ func pathOpenFn(_ context.Context, mod api.Module, params []uint64) Errno {
 		return errno
 	}
 
-	fileOpenFlags, isDir := openFlags(oflags, fdflags)
+	fileOpenFlags := openFlags(dirflags, oflags, fdflags)
+	isDir := fileOpenFlags&platform.O_DIRECTORY != 0
 
 	if isDir && oflags&O_CREAT != 0 {
 		return ErrnoInval // use pathCreateDirectory!
@@ -1635,8 +1636,16 @@ func preopenPath(fsc *sys.FSContext, dirFD uint32) (string, Errno) {
 	}
 }
 
-func openFlags(oflags, fdflags uint16) (openFlags int, isDir bool) {
-	isDir = oflags&O_DIRECTORY != 0
+func openFlags(dirflags, oflags, fdflags uint16) (openFlags int) {
+	if dirflags&LOOKUP_SYMLINK_FOLLOW == 0 {
+		openFlags |= platform.O_NOFOLLOW
+	}
+	if oflags&O_DIRECTORY != 0 {
+		openFlags |= platform.O_DIRECTORY
+		return // Early return for directories as the rest of flags doesn't make sense for it.
+	} else if oflags&O_EXCL != 0 {
+		openFlags |= syscall.O_EXCL
+	}
 	if oflags&O_TRUNC != 0 {
 		openFlags |= syscall.O_RDWR | syscall.O_TRUNC
 	}
@@ -1649,12 +1658,6 @@ func openFlags(oflags, fdflags uint16) (openFlags int, isDir bool) {
 	if openFlags == 0 {
 		openFlags = syscall.O_RDONLY
 	}
-	if isDir {
-		return
-	}
-	if oflags&O_EXCL != 0 {
-		openFlags |= syscall.O_EXCL
-	}
 	return
 }
 
diff --git a/internal/platform/open_file.go b/internal/platform/open_file.go
index c22615df..bed40d37 100644
--- a/internal/platform/open_file.go
+++ b/internal/platform/open_file.go
@@ -1,10 +1,18 @@
-//go:build !windows
+//go:build !windows && !js
 
 package platform
 
 import (
 	"io/fs"
 	"os"
+	"syscall"
+)
+
+// Simple aliases to constants in the syscall package for portability with
+// platforms which do not have them (e.g. windows)
+const (
+	O_DIRECTORY = syscall.O_DIRECTORY
+	O_NOFOLLOW  = syscall.O_NOFOLLOW
 )
 
 func OpenFile(name string, flag int, perm fs.FileMode) (*os.File, error) {
diff --git a/internal/platform/open_file_js.go b/internal/platform/open_file_js.go
new file mode 100644
index 00000000..32cdc62d
--- /dev/null
+++ b/internal/platform/open_file_js.go
@@ -0,0 +1,19 @@
+//go:build js
+
+package platform
+
+import (
+	"io/fs"
+	"os"
+)
+
+// See the comments on the same constants in open_file_windows.go
+const (
+	O_DIRECTORY = 1 << 29
+	O_NOFOLLOW  = 1 << 30
+)
+
+func OpenFile(name string, flag int, perm fs.FileMode) (*os.File, error) {
+	flag &= ^(O_DIRECTORY | O_NOFOLLOW) // erase placeholders
+	return os.OpenFile(name, flag, perm)
+}
diff --git a/internal/platform/open_file_windows.go b/internal/platform/open_file_windows.go
index 43189d29..2c17e35d 100644
--- a/internal/platform/open_file_windows.go
+++ b/internal/platform/open_file_windows.go
@@ -7,6 +7,26 @@ import (
 	"unsafe"
 )
 
+// Windows does not have these constants, we declare placeholders which should
+// not conflict with other open flags. These placeholders are not declared as
+// value zero so code written in a way which expects them to be bit flags still
+// works as expected.
+//
+// Since those placeholder are not interpreted by the open function, the unix
+// features they represent are also not implemented on windows:
+//
+//   - O_DIRECTORY allows programs to ensure that the opened file is a directory.
+//     This could be emulated by doing a stat call on the file after opening it
+//     to verify that it is in fact a directory, then closing it and returning an
+//     error if it is not.
+//
+//   - O_NOFOLLOW allows programs to ensure that if the opened file is a symbolic
+//     link, the link itself is opened instead of its target.
+const (
+	O_DIRECTORY = 1 << 29
+	O_NOFOLLOW  = 1 << 30
+)
+
 func OpenFile(name string, flag int, perm fs.FileMode) (*os.File, error) {
 	fd, err := open(name, flag|syscall.O_CLOEXEC, uint32(perm))
 	if err == nil {
@@ -19,6 +39,7 @@ func OpenFile(name string, flag int, perm fs.FileMode) (*os.File, error) {
 // The following is lifted from syscall_windows.go to add support for setting FILE_SHARE_DELETE.
 // https://github.com/golang/go/blob/go1.19.5/src/syscall/syscall_windows.go#L307-L375
 func open(path string, mode int, perm uint32) (fd syscall.Handle, err error) {
+	mode &= ^(O_DIRECTORY | O_NOFOLLOW) // erase placeholders
 	if len(path) == 0 {
 		return syscall.InvalidHandle, syscall.ERROR_FILE_NOT_FOUND
 	}


Test patch:
diff --git a/imports/wasi_snapshot_preview1/fs_unit_test.go b/imports/wasi_snapshot_preview1/fs_unit_test.go
index 0c267dfb..49e01f82 100644
--- a/imports/wasi_snapshot_preview1/fs_unit_test.go
+++ b/imports/wasi_snapshot_preview1/fs_unit_test.go
@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/tetratelabs/wazero/internal/fstest"
+	"github.com/tetratelabs/wazero/internal/platform"
 	"github.com/tetratelabs/wazero/internal/sys"
 	"github.com/tetratelabs/wazero/internal/testing/require"
 	. "github.com/tetratelabs/wazero/internal/wasi_snapshot_preview1"
@@ -363,45 +364,48 @@ func Test_writeDirents(t *testing.T) {
 
 func Test_openFlags(t *testing.T) {
 	tests := []struct {
-		name              string
-		oflags, fdflags   uint16
-		expectedOpenFlags int
-		expectedIsDir     bool
+		name                      string
+		dirflags, oflags, fdflags uint16
+		expectedOpenFlags         int
 	}{
 		{
 			name:              "oflags=0",
-			expectedOpenFlags: syscall.O_RDONLY,
+			expectedOpenFlags: platform.O_NOFOLLOW | syscall.O_RDONLY,
 		},
 		{
 			name:              "oflags=O_CREAT",
 			oflags:            O_CREAT,
-			expectedOpenFlags: syscall.O_RDWR | syscall.O_CREAT,
+			expectedOpenFlags: platform.O_NOFOLLOW | syscall.O_RDWR | syscall.O_CREAT,
 		},
 		{
 			name:              "oflags=O_DIRECTORY",
 			oflags:            O_DIRECTORY,
-			expectedOpenFlags: syscall.O_RDONLY,
-			expectedIsDir:     true,
+			expectedOpenFlags: platform.O_NOFOLLOW | platform.O_DIRECTORY,
 		},
 		{
 			name:              "oflags=O_EXCL",
 			oflags:            O_EXCL,
-			expectedOpenFlags: syscall.O_RDONLY | syscall.O_EXCL,
+			expectedOpenFlags: platform.O_NOFOLLOW | syscall.O_RDONLY | syscall.O_EXCL,
 		},
 		{
 			name:              "oflags=O_TRUNC",
 			oflags:            O_TRUNC,
-			expectedOpenFlags: syscall.O_RDWR | syscall.O_TRUNC,
+			expectedOpenFlags: platform.O_NOFOLLOW | syscall.O_RDWR | syscall.O_TRUNC,
 		},
 		{
 			name:              "fdflags=FD_APPEND",
 			fdflags:           FD_APPEND,
-			expectedOpenFlags: syscall.O_RDWR | syscall.O_APPEND,
+			expectedOpenFlags: platform.O_NOFOLLOW | syscall.O_RDWR | syscall.O_APPEND,
 		},
 		{
 			name:              "oflags=O_TRUNC|O_CREAT",
 			oflags:            O_TRUNC | O_CREAT,
-			expectedOpenFlags: syscall.O_RDWR | syscall.O_TRUNC | syscall.O_CREAT,
+			expectedOpenFlags: platform.O_NOFOLLOW | syscall.O_RDWR | syscall.O_TRUNC | syscall.O_CREAT,
+		},
+		{
+			name:              "dirflags=LOOKUP_SYMLINK_FOLLOW",
+			dirflags:          LOOKUP_SYMLINK_FOLLOW,
+			expectedOpenFlags: syscall.O_RDONLY,
 		},
 	}
 
@@ -409,9 +413,8 @@ func Test_openFlags(t *testing.T) {
 		tc := tt
 
 		t.Run(tc.name, func(t *testing.T) {
-			openFlags, isDir := openFlags(tc.oflags, tc.fdflags)
+			openFlags := openFlags(tc.dirflags, tc.oflags, tc.fdflags)
 			require.Equal(t, tc.expectedOpenFlags, openFlags)
-			require.Equal(t, tc.expectedIsDir, isDir)
 		})
 	}
 }