fs: handles windows edge cases on rename (#1086)

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.

Signed-off-by: Adrian Cole <adrian@tetrate.io>
Co-authored-by: Takeshi Yoneda <takeshi@tetrate.io>

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

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

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

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

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

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

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)

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
 }

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
 	//