Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 11 commits into from
Feb 27, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions internal/platform/open_file_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,10 @@ func open(path string, mode int, perm uint32) (fd syscall.Handle, err error) {
}
}
}
if createmode == syscall.OPEN_EXISTING && access == syscall.GENERIC_READ {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lifted from the change made in Go 1.20 https://github.com/golang/go/blob/go1.20/src/syscall/syscall_windows.go#L308-L379 and I guess that's why we haven't encountered it before.

// 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
}
13 changes: 13 additions & 0 deletions internal/platform/unlink.go
Original file line number Diff line number Diff line change
@@ -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
}
58 changes: 58 additions & 0 deletions internal/platform/unlink_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
25 changes: 25 additions & 0 deletions internal/platform/unlink_windows.go
Original file line number Diff line number Diff line change
@@ -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
}
6 changes: 1 addition & 5 deletions internal/sysfs/dirfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
108 changes: 95 additions & 13 deletions internal/sysfs/dirfs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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)
Expand All @@ -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) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one is the new one and only windows failing

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))
Expand Down
6 changes: 6 additions & 0 deletions internal/sysfs/sysfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down