windows: fixes unlinking symlink to dir, rmdir on opened empty dir. (#…

…1172)

Signed-off-by: Takeshi Yoneda <[email protected]>

internal/platform/open_file_windows.go

@@ -3,6 +3,8 @@ package platform
 import (
 	"io/fs"
 	"os"
+	"runtime"
+	"strings"
 	"syscall"
 	"unsafe"
 )
@@ -140,6 +142,18 @@ func open(path string, mode int, perm uint32) (fd syscall.Handle, err error) {
 			}
 		}
 	}
+
+	if isGo120 {
+		// This shouldn't be included before 1.20 to have consistent behavior.
+		// https://github.com/golang/go/commit/0f0aa5d8a6a0253627d58b3aa083b24a1091933f
+		if createmode == syscall.OPEN_EXISTING && access == syscall.GENERIC_READ {
+			// Necessary for opening directory handles.
+			attrs |= syscall.FILE_FLAG_BACKUP_SEMANTICS
+		}
+	}
+
 	h, e := syscall.CreateFile(pathp, access, sharemode, sa, createmode, attrs, 0)
 	return h, e
 }
+
+var isGo120 = strings.Contains(runtime.Version(), "go1.20")

internal/platform/unlink.go

@@ -0,0 +1,13 @@
+//go:build !windows
+
+package platform
+
+import "syscall"
+
+func Unlink(name string) error {
+	err := syscall.Unlink(name)
+	if err = UnwrapOSError(err); err == syscall.EPERM {
+		err = syscall.EISDIR
+	}
+	return err
+}

internal/platform/unlink_test.go

@@ -0,0 +1,58 @@
+package platform
+
+import (
+	"os"
+	"path"
+	"syscall"
+	"testing"
+
+	"github.com/tetratelabs/wazero/internal/testing/require"
+)
+
+func TestUnlink(t *testing.T) {
+	t.Run("doesn't exist", func(t *testing.T) {
+		name := "non-existent"
+		err := Unlink(name)
+		require.EqualErrno(t, syscall.ENOENT, err)
+	})
+
+	t.Run("target: dir", func(t *testing.T) {
+		tmpDir := t.TempDir()
+
+		dir := path.Join(tmpDir, "dir")
+		require.NoError(t, os.Mkdir(dir, 0o700))
+
+		err := Unlink(dir)
+		require.EqualErrno(t, syscall.EISDIR, err)
+
+		require.NoError(t, os.Remove(dir))
+	})
+
+	t.Run("target: symlink to dir", func(t *testing.T) {
+		tmpDir := t.TempDir()
+
+		// Create link target dir.
+		subDirRealPath := path.Join(tmpDir, "subdir")
+		require.NoError(t, os.Mkdir(subDirRealPath, 0o700))
+
+		// Create a symlink to the subdirectory.
+		const symlinkName = "symlink-to-dir"
+		require.NoError(t, os.Symlink("subdir", symlinkName))
+
+		// Unlinking the symlink should suceed.
+		err := Unlink(symlinkName)
+		require.NoError(t, err)
+	})
+
+	t.Run("file exists", func(t *testing.T) {
+		tmpDir := t.TempDir()
+
+		name := path.Join(tmpDir, "unlink")
+
+		require.NoError(t, os.WriteFile(name, []byte{}, 0o600))
+
+		require.NoError(t, Unlink(name))
+		_, err := os.Stat(name)
+		require.Error(t, err)
+	})
+}

internal/platform/unlink_windows.go

@@ -0,0 +1,25 @@
+//go:build windows
+
+package platform
+
+import (
+	"os"
+	"syscall"
+)
+
+func Unlink(name string) (err error) {
+	err = syscall.Unlink(name)
+	if err == nil {
+		return
+	}
+	err = UnwrapOSError(err)
+	if err == syscall.EPERM {
+		lstat, errLstat := os.Lstat(name)
+		if errLstat == nil && lstat.Mode()&os.ModeSymlink != 0 {
+			err = UnwrapOSError(os.Remove(name))
+		} else {
+			err = syscall.EISDIR
+		}
+	}
+	return
+}

internal/sysfs/dirfs.go

@@ -110,11 +110,7 @@ func (d *dirFS) Rmdir(name string) error {
 
 // Unlink implements FS.Unlink
 func (d *dirFS) Unlink(name string) (err error) {
-	err = syscall.Unlink(d.join(name))
-	if err = platform.UnwrapOSError(err); err == syscall.EPERM {
-		err = syscall.EISDIR
-	}
-	return
+	return platform.Unlink(d.join(name))
 }
 
 // Symlink implements FS.Symlink

internal/sysfs/dirfs_test.go

@@ -336,18 +336,23 @@ func TestDirFS_Rename(t *testing.T) {
 }
 
 func TestDirFS_Rmdir(t *testing.T) {
-	tmpDir := t.TempDir()
-	testFS := NewDirFS(tmpDir)
+	t.Run("doesn't exist", func(t *testing.T) {
+		tmpDir := t.TempDir()
+		testFS := NewDirFS(tmpDir)
 
-	name := "rmdir"
-	realPath := pathutil.Join(tmpDir, name)
+		name := "rmdir"
 
-	t.Run("doesn't exist", func(t *testing.T) {
 		err := testFS.Rmdir(name)
 		require.EqualErrno(t, syscall.ENOENT, err)
 	})
 
 	t.Run("dir not empty", func(t *testing.T) {
+		tmpDir := t.TempDir()
+		testFS := NewDirFS(tmpDir)
+
+		name := "rmdir"
+		realPath := pathutil.Join(tmpDir, name)
+
 		require.NoError(t, os.Mkdir(realPath, 0o700))
 		fileInDir := pathutil.Join(realPath, "file")
 		require.NoError(t, os.WriteFile(fileInDir, []byte{}, 0o600))
@@ -358,13 +363,62 @@ func TestDirFS_Rmdir(t *testing.T) {
 		require.NoError(t, os.Remove(fileInDir))
 	})
 
+	t.Run("dir previously not empty", func(t *testing.T) {
+		tmpDir := t.TempDir()
+		testFS := NewDirFS(tmpDir)
+
+		name := "rmdir"
+		realPath := pathutil.Join(tmpDir, name)
+		require.NoError(t, os.Mkdir(realPath, 0o700))
+
+		// Create a file and then delete it.
+		fileInDir := pathutil.Join(realPath, "file")
+		require.NoError(t, os.WriteFile(fileInDir, []byte{}, 0o600))
+		require.NoError(t, os.Remove(fileInDir))
+
+		// After deletion, try removing directory.
+		err := testFS.Rmdir(name)
+		require.NoError(t, err)
+	})
+
 	t.Run("dir empty", func(t *testing.T) {
+		tmpDir := t.TempDir()
+		testFS := NewDirFS(tmpDir)
+
+		name := "rmdir"
+		realPath := pathutil.Join(tmpDir, name)
+		require.NoError(t, os.Mkdir(realPath, 0o700))
 		require.NoError(t, testFS.Rmdir(name))
 		_, err := os.Stat(realPath)
 		require.Error(t, err)
 	})
 
+	t.Run("dir empty while opening", func(t *testing.T) {
+		tmpDir := t.TempDir()
+		testFS := NewDirFS(tmpDir)
+
+		name := "rmdir"
+		realPath := pathutil.Join(tmpDir, name)
+		require.NoError(t, os.Mkdir(realPath, 0o700))
+
+		f, err := testFS.OpenFile(name, platform.O_DIRECTORY, 0o700)
+		require.NoError(t, err)
+		defer func() {
+			require.NoError(t, f.Close())
+		}()
+
+		require.NoError(t, testFS.Rmdir(name))
+		_, err = os.Stat(realPath)
+		require.Error(t, err)
+	})
+
 	t.Run("not directory", func(t *testing.T) {
+		tmpDir := t.TempDir()
+		testFS := NewDirFS(tmpDir)
+
+		name := "rmdir"
+		realPath := pathutil.Join(tmpDir, name)
+
 		require.NoError(t, os.WriteFile(realPath, []byte{}, 0o600))
 
 		err := testFS.Rmdir(name)
@@ -375,27 +429,55 @@ func TestDirFS_Rmdir(t *testing.T) {
 }
 
 func TestDirFS_Unlink(t *testing.T) {
-	tmpDir := t.TempDir()
-	testFS := NewDirFS(tmpDir)
-
-	name := "unlink"
-	realPath := pathutil.Join(tmpDir, name)
-
 	t.Run("doesn't exist", func(t *testing.T) {
+		tmpDir := t.TempDir()
+		testFS := NewDirFS(tmpDir)
+		name := "unlink"
+
 		err := testFS.Unlink(name)
 		require.EqualErrno(t, syscall.ENOENT, err)
 	})
 
-	t.Run("not file", func(t *testing.T) {
+	t.Run("target: dir", func(t *testing.T) {
+		tmpDir := t.TempDir()
+		testFS := NewDirFS(tmpDir)
+
+		dir := "dir"
+		realPath := pathutil.Join(tmpDir, dir)
+
 		require.NoError(t, os.Mkdir(realPath, 0o700))
 
-		err := testFS.Unlink(name)
+		err := testFS.Unlink(dir)
 		require.EqualErrno(t, syscall.EISDIR, err)
 
 		require.NoError(t, os.Remove(realPath))
 	})
 
+	t.Run("target: symlink to dir", func(t *testing.T) {
+		tmpDir := t.TempDir()
+		testFS := NewDirFS(tmpDir)
+
+		// Create link target dir.
+		subDirName := "subdir"
+		subDirRealPath := pathutil.Join(tmpDir, subDirName)
+		require.NoError(t, os.Mkdir(subDirRealPath, 0o700))
+
+		// Create a symlink to the subdirectory.
+		const symlinkName = "symlink-to-dir"
+		require.NoError(t, testFS.Symlink("subdir", symlinkName))
+
+		// Unlinking the symlink should suceed.
+		err := testFS.Unlink(symlinkName)
+		require.NoError(t, err)
+	})
+
 	t.Run("file exists", func(t *testing.T) {
+		tmpDir := t.TempDir()
+		testFS := NewDirFS(tmpDir)
+
+		name := "unlink"
+		realPath := pathutil.Join(tmpDir, name)
+
 		require.NoError(t, os.WriteFile(realPath, []byte{}, 0o600))
 
 		require.NoError(t, testFS.Unlink(name))

internal/sysfs/sysfs.go

@@ -153,6 +153,12 @@ type FS interface {
 	//   - syscall.EINVAL: `path` is invalid.
 	//   - syscall.ENOENT: `path` doesn't exist.
 	//   - syscall.EISDIR: `path` exists, but is a directory.
+	//
+	// # Notes
+	//
+	//   - On Windows, syscall.Unlink doesn't delete symlink to directory unlike other platforms. Implementations might
+	//     want to combine syscall.RemoveDirectory with syscall.Unlink in order to delete such links on Windows.
+	//     See https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-removedirectorya
 	Unlink(path string) error
 
 	// Link is similar to syscall.Link, except the path is relative to this