Skip to content

Commit

Permalink
Change Symlink()/Readlink() to take/return path.Parser (#1880)
Browse files Browse the repository at this point in the history 
On Windows, symlink targets may contain drive letters and are supposed
to use backslashes. This is problematic, because the Directory.Symlink()
and Directory.Readlink() methods are currently supposed to return
UNIX-like pathname strings. By altering these functions to use
path.Parser, users of this API don't need to care about pathname
conventions used on the host operating system.
  • Loading branch information
@EdSchouten
EdSchouten committed Mar 8, 2024
1 parent 4d0d7a3 commit e8fd693
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 26 deletions.
4 changes: 2 additions & 2 deletions pkg/filesystem/directory.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ type Directory interface {
// ReadDir is the equivalent of os.ReadDir().
ReadDir() ([]FileInfo, error)
// Readlink is the equivalent of os.Readlink().
Readlink(name path.Component) (string, error)
Readlink(name path.Component) (path.Parser, error)
// Remove is the equivalent of os.Remove().
Remove(name path.Component) error
// RemoveAll is the equivalent of os.RemoveAll().
Expand All @@ -96,7 +96,7 @@ type Directory interface {
// Rename is the equivalent of os.Rename().
Rename(oldName path.Component, newDirectory Directory, newName path.Component) error
// Symlink is the equivalent of os.Symlink().
Symlink(oldName string, newName path.Component) error
Symlink(oldName path.Parser, newName path.Component) error
// Sync the contents of a directory (i.e., the list of names) to
// disk. This does not sync the contents of the files
// themselves.
Expand Down
24 changes: 13 additions & 11 deletions pkg/filesystem/local_directory_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func TestLocalDirectoryEnterFile(t *testing.T) {

func TestLocalDirectoryEnterSymlink(t *testing.T) {
d := openTmpDir(t)
require.NoError(t, d.Symlink("/", path.MustNewComponent("symlink")))
require.NoError(t, d.Symlink(path.MustNewUNIXParser("/"), path.MustNewComponent("symlink")))
_, err := d.EnterDirectory(path.MustNewComponent("symlink"))
require.Equal(t, syscall.ENOTDIR, err)
require.NoError(t, d.Close())
Expand Down Expand Up @@ -123,7 +123,7 @@ func TestLocalDirectoryLstatFile(t *testing.T) {

func TestLocalDirectoryLstatSymlink(t *testing.T) {
d := openTmpDir(t)
require.NoError(t, d.Symlink("/", path.MustNewComponent("symlink")))
require.NoError(t, d.Symlink(path.MustNewUNIXParser("/"), path.MustNewComponent("symlink")))
fi, err := d.Lstat(path.MustNewComponent("symlink"))
require.NoError(t, err)
require.Equal(t, path.MustNewComponent("symlink"), fi.Name())
Expand All @@ -143,7 +143,7 @@ func TestLocalDirectoryLstatDirectory(t *testing.T) {

func TestLocalDirectoryMkdirExisting(t *testing.T) {
d := openTmpDir(t)
require.NoError(t, d.Symlink("/", path.MustNewComponent("symlink")))
require.NoError(t, d.Symlink(path.MustNewUNIXParser("/"), path.MustNewComponent("symlink")))
require.True(t, os.IsExist(d.Mkdir(path.MustNewComponent("symlink"), 0o777)))
require.NoError(t, d.Close())
}
Expand Down Expand Up @@ -173,7 +173,7 @@ func TestLocalDirectoryOpenReadNonExistent(t *testing.T) {

func TestLocalDirectoryOpenReadSymlink(t *testing.T) {
d := openTmpDir(t)
require.NoError(t, d.Symlink("/etc/passwd", path.MustNewComponent("symlink")))
require.NoError(t, d.Symlink(path.MustNewUNIXParser("/etc/passwd"), path.MustNewComponent("symlink")))
_, err := d.OpenRead(path.MustNewComponent("symlink"))
require.Equal(t, syscall.ELOOP, err)
require.NoError(t, d.Close())
Expand All @@ -195,7 +195,7 @@ func TestLocalDirectoryReadDir(t *testing.T) {
require.NoError(t, err)
require.NoError(t, f.Close())
require.NoError(t, d.Mkdir(path.MustNewComponent("directory"), 0o777))
require.NoError(t, d.Symlink("/", path.MustNewComponent("symlink")))
require.NoError(t, d.Symlink(path.MustNewUNIXParser("/"), path.MustNewComponent("symlink")))

// Validate directory listing.
files, err := d.ReadDir()
Expand Down Expand Up @@ -238,10 +238,12 @@ func TestLocalDirectoryReadlinkFile(t *testing.T) {

func TestLocalDirectoryReadlinkSuccess(t *testing.T) {
d := openTmpDir(t)
require.NoError(t, d.Symlink("/foo/bar/baz", path.MustNewComponent("symlink")))
target, err := d.Readlink(path.MustNewComponent("symlink"))
require.NoError(t, d.Symlink(path.MustNewUNIXParser("/foo/bar/baz"), path.MustNewComponent("symlink")))
targetParser, err := d.Readlink(path.MustNewComponent("symlink"))
require.NoError(t, err)
require.Equal(t, "/foo/bar/baz", target)
targetPath, scopeWalker := path.EmptyBuilder.Join(path.VoidScopeWalker)
require.NoError(t, path.Resolve(targetParser, scopeWalker))
require.Equal(t, "/foo/bar/baz", targetPath.String())
require.NoError(t, d.Close())
}

Expand Down Expand Up @@ -273,7 +275,7 @@ func TestLocalDirectoryRemoveFile(t *testing.T) {

func TestLocalDirectoryRemoveSymlink(t *testing.T) {
d := openTmpDir(t)
require.NoError(t, d.Symlink("/", path.MustNewComponent("symlink")))
require.NoError(t, d.Symlink(path.MustNewUNIXParser("/"), path.MustNewComponent("symlink")))
require.NoError(t, d.Remove(path.MustNewComponent("symlink")))
_, err := d.OpenRead(path.MustNewComponent("symlink"))
require.True(t, os.IsNotExist(err))
Expand Down Expand Up @@ -301,13 +303,13 @@ func TestLocalDirectoryRenameSuccess(t *testing.T) {
func TestLocalDirectorySymlinkExistent(t *testing.T) {
d := openTmpDir(t)
require.NoError(t, d.Mkdir(path.MustNewComponent("directory"), 0o777))
require.True(t, os.IsExist(d.Symlink("/", path.MustNewComponent("directory"))))
require.True(t, os.IsExist(d.Symlink(path.MustNewUNIXParser("/"), path.MustNewComponent("directory"))))
require.NoError(t, d.Close())
}

func TestLocalDirectorySymlinkSuccess(t *testing.T) {
d := openTmpDir(t)
require.NoError(t, d.Symlink("/", path.MustNewComponent("symlink")))
require.NoError(t, d.Symlink(path.MustNewUNIXParser("/"), path.MustNewComponent("symlink")))
require.NoError(t, d.Close())
}

Expand Down
15 changes: 10 additions & 5 deletions pkg/filesystem/local_directory_unix.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,17 +231,17 @@ func (d *localDirectory) ReadDir() ([]FileInfo, error) {
return list, nil
}

func (d *localDirectory) Readlink(name path.Component) (string, error) {
func (d *localDirectory) Readlink(name path.Component) (path.Parser, error) {
defer runtime.KeepAlive(d)

for l := 128; ; l *= 2 {
b := make([]byte, l)
n, err := unix.Readlinkat(d.fd, name.String(), b)
if err != nil {
return "", err
return nil, err
}
if n < l {
return string(b[0:n]), nil
return path.MustNewUNIXParser(string(b[0:n])), nil
}
}
}
Expand Down Expand Up @@ -380,10 +380,15 @@ func (d *localDirectory) Rename(oldName path.Component, newDirectory Directory,
})
}

func (d *localDirectory) Symlink(oldName string, newName path.Component) error {
func (d *localDirectory) Symlink(oldName path.Parser, newName path.Component) error {
defer runtime.KeepAlive(d)

return unix.Symlinkat(oldName, d.fd, newName.String())
oldNamePath, scopeWalker := path.EmptyBuilder.Join(path.VoidScopeWalker)
if err := path.Resolve(oldName, scopeWalker); err != nil {
return err
}

return unix.Symlinkat(oldNamePath.String(), d.fd, newName.String())
}

func (d *localDirectory) Sync() error {
Expand Down
20 changes: 12 additions & 8 deletions pkg/filesystem/local_directory_windows.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -502,11 +502,11 @@ func (d *localDirectory) ReadDir() ([]FileInfo, error) {
return list, nil
}

func (d *localDirectory) Readlink(name path.Component) (string, error) {
func (d *localDirectory) Readlink(name path.Component) (path.Parser, error) {
var handle windows.Handle
err := ntCreateFile(&handle, windows.FILE_GENERIC_READ, d.handle, name.String(), windows.FILE_OPEN, windows.FILE_OPEN_REPARSE_POINT)
if err != nil {
return "", err
return nil, err
}
outBufferSize := uint32(512)
outBuffer := make([]byte, outBufferSize)
Expand All @@ -518,18 +518,18 @@ func (d *localDirectory) Readlink(name path.Component) (string, error) {
break
}
if err.(syscall.Errno) == windows.ERROR_NOT_A_REPARSE_POINT {
return "", syscall.EINVAL
return nil, syscall.EINVAL
}
if err.(syscall.Errno) == windows.ERROR_INSUFFICIENT_BUFFER {
outBufferSize *= 2
outBuffer = make([]byte, outBufferSize)
} else {
return "", err
return nil, err
}
}
reparseDataBufferPtr := (*windowsext.REPARSE_DATA_BUFFER)(unsafe.Pointer(&outBuffer[0]))
if reparseDataBufferPtr.ReparseTag != windows.IO_REPARSE_TAG_SYMLINK {
return "", syscall.EINVAL
return nil, syscall.EINVAL
}
symlinkReparseBufferPtr := (*windowsext.SymbolicLinkReparseBuffer)(unsafe.Pointer(&reparseDataBufferPtr.DUMMYUNIONNAME[0]))
contentPtr := unsafe.Pointer(uintptr(unsafe.Pointer(&symlinkReparseBufferPtr.PathBuffer[0])) + uintptr(symlinkReparseBufferPtr.SubstituteNameOffset))
Expand All @@ -539,7 +539,7 @@ func (d *localDirectory) Readlink(name path.Component) (string, error) {
contentUTF16[i] = *(*uint16)(contentPtr)
contentPtr = unsafe.Pointer(uintptr(contentPtr) + uintptr(2))
}
return filepath.ToSlash(windows.UTF16ToString(contentUTF16)), nil
return path.NewUNIXParser(filepath.ToSlash(windows.UTF16ToString(contentUTF16)))
}

func (d *localDirectory) Remove(name path.Component) error {
Expand Down Expand Up @@ -715,13 +715,17 @@ func getAbsPathByHandle(handle windows.Handle) (string, error) {
return windows.UTF16ToString(buffer), nil
}

func (d *localDirectory) Symlink(oldName string, newName path.Component) error {
func (d *localDirectory) Symlink(oldNameParser path.Parser, newName path.Component) error {
// Creating symlinks on windows requires one of the following:
// 1. Run as an administrator.
// 2. Developer mode is on.
defer runtime.KeepAlive(d)

oldName = filepath.FromSlash(oldName)
oldNamePath, scopeWalker := path.EmptyBuilder.Join(path.VoidScopeWalker)
if err := path.Resolve(oldNameParser, scopeWalker); err != nil {
return err
}
oldName := filepath.FromSlash(oldNamePath.String())
// Path with one leading slash (but not UNC) should also be considered absolute.
isRelative := !(oldName[0] == '\\' || filepath.IsAbs(oldName))
// On windows, you have to know if the target is a directory when creating a symlink.
Expand Down

0 comments on commit e8fd693

Please sign in to comment.