Repository: tetratelabs/wazero
Pull request #1086 (2023-01-31 11:49:02)

fs: handles windows edge cases on rename
This handles edge cases on rename which were previously skipped in testing. Notably, there are delete flags not possible to propagate in current versions of go, which imply some copy/paste meanwhile.



Patch:
diff --git a/internal/platform/open_file.go b/internal/platform/open_file.go
new file mode 100644
index 00000000..c22615df
--- /dev/null
+++ b/internal/platform/open_file.go
@@ -0,0 +1,12 @@
+//go:build !windows
+
+package platform
+
+import (
+	"io/fs"
+	"os"
+)
+
+func OpenFile(name string, flag int, perm fs.FileMode) (*os.File, error) {
+	return os.OpenFile(name, flag, perm)
+}
diff --git a/internal/platform/open_file_windows.go b/internal/platform/open_file_windows.go
new file mode 100644
index 00000000..33763ae1
--- /dev/null
+++ b/internal/platform/open_file_windows.go
@@ -0,0 +1,70 @@
+package platform
+
+import (
+	"io/fs"
+	"os"
+	"syscall"
+	"unsafe"
+)
+
+func OpenFile(name string, flag int, perm fs.FileMode) (*os.File, error) {
+	fd, err := windowsOpenFile(name, flag|syscall.O_CLOEXEC, uint32(perm))
+	if err == nil {
+		return os.NewFile(uintptr(fd), name), nil
+	}
+
+	// TODO: Set FILE_SHARE_DELETE for directory as well.
+	return os.OpenFile(name, flag, perm)
+}
+
+// lifted from ./go/src/syscall/syscall_windows.go. Modified to add FILE_SHARE_DELETE
+func windowsOpenFile(path string, mode int, perm uint32) (fd syscall.Handle, err error) {
+	if len(path) == 0 {
+		return syscall.InvalidHandle, syscall.ERROR_FILE_NOT_FOUND
+	}
+	pathp, err := syscall.UTF16PtrFromString(path)
+	if err != nil {
+		return syscall.InvalidHandle, err
+	}
+	var access uint32
+	switch mode & (syscall.O_RDONLY | syscall.O_WRONLY | syscall.O_RDWR) {
+	case syscall.O_RDONLY:
+		access = syscall.GENERIC_READ
+	case syscall.O_WRONLY:
+		access = syscall.GENERIC_WRITE
+	case syscall.O_RDWR:
+		access = syscall.GENERIC_READ | syscall.GENERIC_WRITE
+	}
+	if mode&syscall.O_CREAT != 0 {
+		access |= syscall.GENERIC_WRITE
+	}
+	if mode&syscall.O_APPEND != 0 {
+		access &^= syscall.GENERIC_WRITE
+		access |= syscall.FILE_APPEND_DATA
+	}
+	sharemode := uint32(syscall.FILE_SHARE_READ | syscall.FILE_SHARE_WRITE |
+		syscall.FILE_SHARE_DELETE)
+	var sa *syscall.SecurityAttributes
+	if mode&syscall.O_CLOEXEC == 0 {
+		var _sa syscall.SecurityAttributes
+		_sa.Length = uint32(unsafe.Sizeof(sa))
+		_sa.InheritHandle = 1
+		sa = &_sa
+	}
+	var createmode uint32
+	switch {
+	case mode&(syscall.O_CREAT|syscall.O_EXCL) == (syscall.O_CREAT | syscall.O_EXCL):
+		createmode = syscall.CREATE_NEW
+	case mode&(syscall.O_CREAT|syscall.O_TRUNC) == (syscall.O_CREAT | syscall.O_TRUNC):
+		createmode = syscall.CREATE_ALWAYS
+	case mode&syscall.O_CREAT == syscall.O_CREAT:
+		createmode = syscall.OPEN_ALWAYS
+	case mode&syscall.O_TRUNC == syscall.O_TRUNC:
+		createmode = syscall.TRUNCATE_EXISTING
+	default:
+		createmode = syscall.OPEN_EXISTING
+	}
+	h, e := syscall.CreateFile(
+		pathp, access, sharemode, sa, createmode, syscall.FILE_ATTRIBUTE_NORMAL, 0)
+	return h, e
+}
diff --git a/internal/platform/rename.go b/internal/platform/rename.go
new file mode 100644
index 00000000..6c39a556
--- /dev/null
+++ b/internal/platform/rename.go
@@ -0,0 +1,12 @@
+//go:build !windows
+
+package platform
+
+import "syscall"
+
+func Rename(from, to string) error {
+	if from == to {
+		return nil
+	}
+	return syscall.Rename(from, to)
+}
diff --git a/internal/platform/rename_windows.go b/internal/platform/rename_windows.go
new file mode 100644
index 00000000..48a2d32e
--- /dev/null
+++ b/internal/platform/rename_windows.go
@@ -0,0 +1,45 @@
+package platform
+
+import (
+	"errors"
+	"os"
+	"syscall"
+)
+
+func Rename(from, to string) error {
+	if from == to {
+		return nil
+	}
+
+	fromStat, err := os.Stat(from)
+	if err != nil {
+		return syscall.ENOENT
+	}
+
+	if toStat, err := os.Stat(to); err == nil {
+		fromIsDir, toIsDir := fromStat.IsDir(), toStat.IsDir()
+		if fromIsDir && !toIsDir { // dir to file
+			return syscall.ENOTDIR
+		} else if !fromIsDir && toIsDir { // file to dir
+			return syscall.EISDIR
+		} else if !fromIsDir && !toIsDir { // file to file
+			// Use os.Rename instead of syscall.Rename in order to allow the overrides of the existing file.
+			// Underneath os.Rename, it uses MoveFileEx instead of MoveFile (used by syscall.Rename).
+			return os.Rename(from, to)
+		} else { // dir to dir
+			if dirs, _ := os.ReadDir(to); len(dirs) == 0 {
+				// On Windows, renaming to the empty dir will be rejected,
+				// so first we remove the empty dir, and then rename to it.
+				if err := os.Remove(to); err != nil {
+					return err
+				}
+				return syscall.Rename(from, to)
+			}
+			return syscall.ENOTEMPTY
+		}
+	} else if !errors.Is(err, syscall.ENOENT) { // Failed to stat the destination.
+		return err
+	} else { // Destination not-exist.
+		return syscall.Rename(from, to)
+	}
+}
diff --git a/internal/sysfs/dirfs.go b/internal/sysfs/dirfs.go
index f99a4c19..ebd2cfab 100644
--- a/internal/sysfs/dirfs.go
+++ b/internal/sysfs/dirfs.go
@@ -4,6 +4,8 @@ import (
 	"io/fs"
 	"os"
 	"syscall"
+
+	"github.com/tetratelabs/wazero/internal/platform"
 )
 
 func NewDirFS(dir string) FS {
@@ -40,7 +42,7 @@ func (d *dirFS) Open(name string) (fs.File, error) {
 
 // OpenFile implements FS.OpenFile
 func (d *dirFS) OpenFile(name string, flag int, perm fs.FileMode) (fs.File, error) {
-	f, err := os.OpenFile(d.join(name), flag, perm)
+	f, err := platform.OpenFile(d.join(name), flag, perm)
 	if err != nil {
 		return nil, unwrapOSError(err)
 	}
@@ -56,10 +58,8 @@ func (d *dirFS) Mkdir(name string, perm fs.FileMode) error {
 
 // Rename implements FS.Rename
 func (d *dirFS) Rename(from, to string) error {
-	if from == to {
-		return nil
-	}
-	return rename(d.join(from), d.join(to))
+	from, to = d.join(from), d.join(to)
+	return platform.Rename(from, to)
 }
 
 // Rmdir implements FS.Rmdir
diff --git a/internal/sysfs/syscall.go b/internal/sysfs/syscall.go
index 2776f787..63c18405 100644
--- a/internal/sysfs/syscall.go
+++ b/internal/sysfs/syscall.go
@@ -23,10 +23,6 @@ func adjustUnlinkError(err error) error {
 	return err
 }
 
-func rename(old, new string) error {
-	return syscall.Rename(old, new)
-}
-
 func maybeWrapFile(f file) file {
 	return f
 }
diff --git a/internal/sysfs/sysfs.go b/internal/sysfs/sysfs.go
index 8f3a3e5a..8231560b 100644
--- a/internal/sysfs/sysfs.go
+++ b/internal/sysfs/sysfs.go
@@ -79,6 +79,7 @@ type FS interface {
 	//   - syscall.ENOENT: `from` or `to` don't exist.
 	//   - syscall.ENOTDIR: `from` is a directory and `to` exists, but is a file.
 	//   - syscall.EISDIR: `from` is a file and `to` exists, but is a directory.
+	//   - syscall.ENOTEMPTY: `both from` and `to` are existing directory, but `to` is not empty.
 	//
 	// # Notes
 	//


Test patch:
diff --git a/internal/platform/rename_test.go b/internal/platform/rename_test.go
new file mode 100644
index 00000000..42755aa5
--- /dev/null
+++ b/internal/platform/rename_test.go
@@ -0,0 +1,201 @@
+package platform
+
+import (
+	"errors"
+	"os"
+	"path"
+	"syscall"
+	"testing"
+
+	"github.com/tetratelabs/wazero/internal/testing/require"
+)
+
+func TestRename(t *testing.T) {
+	t.Run("from doesn't exist", func(t *testing.T) {
+		tmpDir := t.TempDir()
+
+		file1Path := path.Join(tmpDir, "file1")
+		err := os.WriteFile(file1Path, []byte{1}, 0o600)
+		require.NoError(t, err)
+
+		err = Rename(path.Join(tmpDir, "non-exist"), file1Path)
+		require.Equal(t, syscall.ENOENT, err)
+	})
+	t.Run("file to non-exist", func(t *testing.T) {
+		tmpDir := t.TempDir()
+
+		file1Path := path.Join(tmpDir, "file1")
+		file1Contents := []byte{1}
+		err := os.WriteFile(file1Path, file1Contents, 0o600)
+		require.NoError(t, err)
+
+		file2Path := path.Join(tmpDir, "file2")
+		err = Rename(file1Path, file2Path)
+		require.NoError(t, err)
+
+		// Show the prior path no longer exists
+		_, err = os.Stat(file1Path)
+		require.Equal(t, syscall.ENOENT, errors.Unwrap(err))
+
+		s, err := os.Stat(file2Path)
+		require.NoError(t, err)
+		require.False(t, s.IsDir())
+	})
+	t.Run("dir to non-exist", func(t *testing.T) {
+		tmpDir := t.TempDir()
+
+		dir1Path := path.Join(tmpDir, "dir1")
+		require.NoError(t, os.Mkdir(dir1Path, 0o700))
+
+		dir2Path := path.Join(tmpDir, "dir2")
+		err := Rename(dir1Path, dir2Path)
+		require.NoError(t, err)
+
+		// Show the prior path no longer exists
+		_, err = os.Stat(dir1Path)
+		require.Equal(t, syscall.ENOENT, errors.Unwrap(err))
+
+		s, err := os.Stat(dir2Path)
+		require.NoError(t, err)
+		require.True(t, s.IsDir())
+	})
+	t.Run("dir to file", func(t *testing.T) {
+		tmpDir := t.TempDir()
+
+		dir1Path := path.Join(tmpDir, "dir1")
+		require.NoError(t, os.Mkdir(dir1Path, 0o700))
+
+		dir2Path := path.Join(tmpDir, "dir2")
+
+		// write a file to that path
+		err := os.WriteFile(dir2Path, []byte{2}, 0o600)
+		require.NoError(t, err)
+
+		err = Rename(dir1Path, dir2Path)
+		require.Equal(t, syscall.ENOTDIR, err)
+	})
+	t.Run("file to dir", func(t *testing.T) {
+		tmpDir := t.TempDir()
+
+		file1Path := path.Join(tmpDir, "file1")
+		file1Contents := []byte{1}
+		err := os.WriteFile(file1Path, file1Contents, 0o600)
+		require.NoError(t, err)
+
+		dir1Path := path.Join(tmpDir, "dir1")
+		require.NoError(t, os.Mkdir(dir1Path, 0o700))
+
+		err = Rename(file1Path, dir1Path)
+		require.Equal(t, syscall.EISDIR, err)
+	})
+
+	// Similar to https://github.com/ziglang/zig/blob/0.10.1/lib/std/fs/test.zig#L567-L582
+	t.Run("dir to empty dir should be fine", func(t *testing.T) {
+		tmpDir := t.TempDir()
+
+		dir1 := "dir1"
+		dir1Path := path.Join(tmpDir, dir1)
+		require.NoError(t, os.Mkdir(dir1Path, 0o700))
+
+		// add a file to that directory
+		file1 := "file1"
+		file1Path := path.Join(dir1Path, file1)
+		file1Contents := []byte{1}
+		err := os.WriteFile(file1Path, file1Contents, 0o600)
+		require.NoError(t, err)
+
+		dir2Path := path.Join(tmpDir, "dir2")
+		require.NoError(t, os.Mkdir(dir2Path, 0o700))
+
+		err = Rename(dir1Path, dir2Path)
+		require.NoError(t, err)
+
+		// Show the prior path no longer exists
+		_, err = os.Stat(dir1Path)
+		require.Equal(t, syscall.ENOENT, errors.Unwrap(err))
+
+		// Show the file inside that directory moved
+		s, err := os.Stat(path.Join(dir2Path, file1))
+		require.NoError(t, err)
+		require.False(t, s.IsDir())
+	})
+
+	// Similar to https://github.com/ziglang/zig/blob/0.10.1/lib/std/fs/test.zig#L584-L604
+	t.Run("dir to non empty dir should be EXIST", func(t *testing.T) {
+		tmpDir := t.TempDir()
+
+		dir1 := "dir1"
+		dir1Path := path.Join(tmpDir, dir1)
+		require.NoError(t, os.Mkdir(dir1Path, 0o700))
+
+		// add a file to that directory
+		file1Path := path.Join(dir1Path, "file1")
+		file1Contents := []byte{1}
+		err := os.WriteFile(file1Path, file1Contents, 0o600)
+		require.NoError(t, err)
+
+		dir2Path := path.Join(tmpDir, "dir2")
+		require.NoError(t, os.Mkdir(dir2Path, 0o700))
+
+		// Make the destination non-empty.
+		err = os.WriteFile(path.Join(dir2Path, "existing.txt"), []byte("any thing"), 0o600)
+		require.NoError(t, err)
+
+		err = Rename(dir1Path, dir2Path)
+		require.ErrorIs(t, syscall.ENOTEMPTY, err)
+	})
+
+	t.Run("file to file", func(t *testing.T) {
+		tmpDir := t.TempDir()
+
+		file1Path := path.Join(tmpDir, "file1")
+		file1Contents := []byte{1}
+		err := os.WriteFile(file1Path, file1Contents, 0o600)
+		require.NoError(t, err)
+
+		file2Path := path.Join(tmpDir, "file2")
+		file2Contents := []byte{2}
+		err = os.WriteFile(file2Path, file2Contents, 0o600)
+		require.NoError(t, err)
+
+		err = Rename(file1Path, file2Path)
+		require.NoError(t, err)
+
+		// Show the prior path no longer exists
+		_, err = os.Stat(file1Path)
+		require.Equal(t, syscall.ENOENT, errors.Unwrap(err))
+
+		// Show the file1 overwrote file2
+		b, err := os.ReadFile(file2Path)
+		require.NoError(t, err)
+		require.Equal(t, file1Contents, b)
+	})
+	t.Run("dir to itself", func(t *testing.T) {
+		tmpDir := t.TempDir()
+
+		dir1Path := path.Join(tmpDir, "dir1")
+		require.NoError(t, os.Mkdir(dir1Path, 0o700))
+
+		err := Rename(dir1Path, dir1Path)
+		require.NoError(t, err)
+
+		s, err := os.Stat(dir1Path)
+		require.NoError(t, err)
+		require.True(t, s.IsDir())
+	})
+	t.Run("file to itself", func(t *testing.T) {
+		tmpDir := t.TempDir()
+
+		file1Path := path.Join(tmpDir, "file1")
+		file1Contents := []byte{1}
+		err := os.WriteFile(file1Path, file1Contents, 0o600)
+		require.NoError(t, err)
+
+		err = Rename(file1Path, file1Path)
+		require.NoError(t, err)
+
+		b, err := os.ReadFile(file1Path)
+		require.NoError(t, err)
+		require.Equal(t, file1Contents, b)
+	})
+}
diff --git a/internal/sysfs/dirfs_test.go b/internal/sysfs/dirfs_test.go
index 00af3179..b61353a1 100644
--- a/internal/sysfs/dirfs_test.go
+++ b/internal/sysfs/dirfs_test.go
@@ -142,20 +142,12 @@ func TestDirFS_Rename(t *testing.T) {
 		dir2Path := pathutil.Join(tmpDir, dir2)
 
 		// write a file to that path
-		err := os.WriteFile(dir2Path, []byte{2}, 0o600)
+		f, err := os.OpenFile(dir2Path, os.O_RDWR|os.O_CREATE, 0o600)
 		require.NoError(t, err)
+		require.NoError(t, f.Close())
 
 		err = testFS.Rename(dir1, dir2)
-		if runtime.GOOS == "windows" {
-			require.NoError(t, err)
-
-			// Show the directory moved
-			s, err := os.Stat(dir2Path)
-			require.NoError(t, err)
-			require.True(t, s.IsDir())
-		} else {
-			require.Equal(t, syscall.ENOTDIR, err)
-		}
+		require.Equal(t, syscall.ENOTDIR, err)
 	})
 	t.Run("file to dir", func(t *testing.T) {
 		tmpDir := t.TempDir()
@@ -174,7 +166,9 @@ func TestDirFS_Rename(t *testing.T) {
 		err = testFS.Rename(file1, dir1)
 		require.Equal(t, syscall.EISDIR, err)
 	})
-	t.Run("dir to dir", func(t *testing.T) {
+
+	// Similar to https://github.com/ziglang/zig/blob/0.10.1/lib/std/fs/test.zig#L567-L582
+	t.Run("dir to empty dir should be fine", func(t *testing.T) {
 		tmpDir := t.TempDir()
 		testFS := NewDirFS(tmpDir)
 
@@ -194,11 +188,6 @@ func TestDirFS_Rename(t *testing.T) {
 		require.NoError(t, os.Mkdir(dir2Path, 0o700))
 
 		err = testFS.Rename(dir1, dir2)
-		if runtime.GOOS == "windows" {
-			// Windows doesn't let you overwrite an existing directory.
-			require.Equal(t, syscall.EINVAL, err)
-			return
-		}
 		require.NoError(t, err)
 
 		// Show the prior path no longer exists
@@ -210,6 +199,35 @@ func TestDirFS_Rename(t *testing.T) {
 		require.NoError(t, err)
 		require.False(t, s.IsDir())
 	})
+
+	// Similar to https://github.com/ziglang/zig/blob/0.10.1/lib/std/fs/test.zig#L584-L604
+	t.Run("dir to non empty dir should be EXIST", func(t *testing.T) {
+		tmpDir := t.TempDir()
+		testFS := NewDirFS(tmpDir)
+
+		dir1 := "dir1"
+		dir1Path := pathutil.Join(tmpDir, dir1)
+		require.NoError(t, os.Mkdir(dir1Path, 0o700))
+
+		// add a file to that directory
+		file1 := "file1"
+		file1Path := pathutil.Join(dir1Path, file1)
+		file1Contents := []byte{1}
+		err := os.WriteFile(file1Path, file1Contents, 0o600)
+		require.NoError(t, err)
+
+		dir2 := "dir2"
+		dir2Path := pathutil.Join(tmpDir, dir2)
+		require.NoError(t, os.Mkdir(dir2Path, 0o700))
+
+		// Make the destination non-empty.
+		err = os.WriteFile(pathutil.Join(dir2Path, "existing.txt"), []byte("any thing"), 0o600)
+		require.NoError(t, err)
+
+		err = testFS.Rename(dir1, dir2)
+		require.ErrorIs(t, syscall.ENOTEMPTY, err)
+	})
+
 	t.Run("file to file", func(t *testing.T) {
 		tmpDir := t.TempDir()
 		testFS := NewDirFS(tmpDir)