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 <[email protected]> Co-authored-by: Takeshi Yoneda <[email protected]>
tetratelabs · Jan 31, 2023 · f040753 · f040753
1 parent 043da0d
commit f040753
Show file tree

Hide file tree

Showing 9 changed files with 381 additions and 26 deletions.
diff --git a/internal/platform/open_file.go 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
@@ -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
@@ -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_test.go 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/platform/rename_windows.go 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
@@ -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