From a9b3301862be7bb33f8123898c84c63242f9e596 Mon Sep 17 00:00:00 2001
From: Crypt Keeper <64215+codefromthecrypt@users.noreply.github.com>
Date: Sun, 5 Mar 2023 16:11:36 +0800
Subject: [PATCH] gojs: implements remaining link functions (#1198)

This implements the last remaining link functions using the same logic
as WASI. In doing so, this changes the signature for FS.ReadLink to be
more similar to the rest of the functions. Particularly, it stops
reading the result into a buffer.

After this, the only syscalls left to implement in gojs are chown.

Signed-off-by: Adrian Cole <adrian@tetrate.io>
---
 imports/wasi_snapshot_preview1/fs.go      | 13 +++----
 imports/wasi_snapshot_preview1/fs_test.go | 27 ++++++++++++--
 internal/gojs/fs.go                       |  9 ++---
 internal/gojs/testdata/writefs/main.go    | 45 +++++++++++++++++++----
 internal/platform/path.go                 |  6 +++
 internal/platform/path_test.go            | 14 +++++++
 internal/platform/path_windows.go         | 17 +++++++++
 internal/platform/separator.go            |  7 ----
 internal/platform/separator_test.go       | 14 -------
 internal/platform/separator_windows.go    | 10 -----
 internal/sysfs/dirfs.go                   | 17 ++-------
 internal/sysfs/readfs.go                  |  4 +-
 internal/sysfs/sysfs.go                   |  2 +-
 internal/sysfs/sysfs_test.go              | 12 +++---
 internal/sysfs/unsupported.go             |  4 +-
 15 files changed, 122 insertions(+), 79 deletions(-)
 create mode 100644 internal/platform/path.go
 create mode 100644 internal/platform/path_test.go
 create mode 100644 internal/platform/path_windows.go
 delete mode 100644 internal/platform/separator.go
 delete mode 100644 internal/platform/separator_test.go
 delete mode 100644 internal/platform/separator_windows.go

diff --git a/imports/wasi_snapshot_preview1/fs.go b/imports/wasi_snapshot_preview1/fs.go
index 5767b1e2..8f4138e5 100644
--- a/imports/wasi_snapshot_preview1/fs.go
+++ b/imports/wasi_snapshot_preview1/fs.go
@@ -1730,17 +1730,16 @@ func pathReadlinkFn(_ context.Context, mod api.Module, params []uint64) Errno {
 		return en
 	}
 
-	buf, ok := mem.Read(bufPtr, bufLen)
-	if !ok {
-		return ErrnoFault
-	}
-
-	n, err := preopen.Readlink(p, buf)
+	dst, err := preopen.Readlink(p)
 	if err != nil {
 		return ToErrno(err)
 	}
 
-	if !mem.WriteUint32Le(resultBufUsedPtr, uint32(n)) {
+	if ok := mem.WriteString(bufPtr, dst); !ok {
+		return ErrnoFault
+	}
+
+	if !mem.WriteUint32Le(resultBufUsedPtr, uint32(len(dst))) {
 		return ErrnoFault
 	}
 	return ErrnoSuccess
diff --git a/imports/wasi_snapshot_preview1/fs_test.go b/imports/wasi_snapshot_preview1/fs_test.go
index 8505e223..c36a3fb6 100644
--- a/imports/wasi_snapshot_preview1/fs_test.go
+++ b/imports/wasi_snapshot_preview1/fs_test.go
@@ -3826,14 +3826,30 @@ func Test_pathReadlink(t *testing.T) {
 
 	t.Run("errors", func(t *testing.T) {
 		for _, tc := range []struct {
+			name                                                      string
 			errno                                                     Errno
 			dirFd, pathPtr, pathLen, bufPtr, bufLen, resultBufUsedPtr uint64
 		}{
 			{errno: ErrnoInval},
 			{errno: ErrnoInval, pathLen: 100},
 			{errno: ErrnoInval, bufLen: 100},
-			{errno: ErrnoFault, dirFd: uint64(topFd), bufLen: 100, pathLen: 100, bufPtr: math.MaxUint32},
-			{errno: ErrnoFault, bufLen: 100, pathLen: 100, bufPtr: 50, pathPtr: math.MaxUint32},
+			{
+				name:    "bufLen too short",
+				errno:   ErrnoFault,
+				dirFd:   uint64(topFd),
+				bufLen:  10,
+				pathPtr: destinationPathNamePtr,
+				pathLen: uint64(len(destinationPathName)),
+				bufPtr:  math.MaxUint32,
+			},
+			{
+				name:    "pathPtr past memory",
+				errno:   ErrnoFault,
+				bufLen:  100,
+				pathLen: 100,
+				bufPtr:  50,
+				pathPtr: math.MaxUint32,
+			},
 			{errno: ErrnoNotdir, bufLen: 100, pathLen: 100, bufPtr: 50, pathPtr: 50, dirFd: 1},
 			{errno: ErrnoBadf, bufLen: 100, pathLen: 100, bufPtr: 50, pathPtr: 50, dirFd: 1000},
 			{
@@ -3843,12 +3859,15 @@ func Test_pathReadlink(t *testing.T) {
 				dirFd: uint64(topFd),
 			},
 		} {
-			name := ErrnoName(tc.errno)
+			name := tc.name
+			if name == "" {
+				name = ErrnoName(tc.errno)
+			}
 			t.Run(name, func(t *testing.T) {
 				requireErrnoResult(t, tc.errno, mod, PathReadlinkName,
 					tc.dirFd, tc.pathPtr, tc.pathLen, tc.bufPtr,
 					tc.bufLen, tc.resultBufUsedPtr)
-				require.Contains(t, log.String(), name)
+				require.Contains(t, log.String(), ErrnoName(tc.errno))
 			})
 		}
 	})
diff --git a/internal/gojs/fs.go b/internal/gojs/fs.go
index 4d4babce..55d3d3a0 100644
--- a/internal/gojs/fs.go
+++ b/internal/gojs/fs.go
@@ -664,9 +664,8 @@ func (jsfsReadlink) invoke(ctx context.Context, mod api.Module, args ...interfac
 	path := args[0].(string)
 	callback := args[1].(funcWrapper)
 
-	_ = path // TODO
-	var dst string
-	var err error = syscall.ENOSYS
+	fsc := mod.(*wasm.CallContext).Sys.FS()
+	dst, err := fsc.RootFS().Readlink(path)
 
 	return callback.invoke(ctx, mod, goos.RefJsfs, err, dst) // note: error first
 }
@@ -681,8 +680,8 @@ func (jsfsLink) invoke(ctx context.Context, mod api.Module, args ...interface{})
 	link := args[1].(string)
 	callback := args[2].(funcWrapper)
 
-	_, _ = path, link // TODO
-	var err error = syscall.ENOSYS
+	fsc := mod.(*wasm.CallContext).Sys.FS()
+	err := fsc.RootFS().Link(path, link)
 
 	return jsfsInvoke(ctx, mod, callback, err)
 }
diff --git a/internal/gojs/testdata/writefs/main.go b/internal/gojs/testdata/writefs/main.go
index 24e9a5f9..c40e2990 100644
--- a/internal/gojs/testdata/writefs/main.go
+++ b/internal/gojs/testdata/writefs/main.go
@@ -122,22 +122,53 @@ func Main() {
 		log.Panicln("unexpected contents:", string(bytes))
 	}
 
-	// Test lstat which should be about the link not its target.
+	// create a hard link
 	link := file1 + "-link"
-	if err = os.Symlink(file1, link); err != nil {
+	if err = os.Link(file1, link); err != nil {
 		log.Panicln(err)
 	}
 
-	lstat, err := os.Lstat(link)
+	// Ensure this is a hard link, so they have the same inode.
+	file1Stat, err := os.Lstat(file1)
+	if err != nil {
+		log.Panicln(err)
+	}
+	linkStat, err := os.Lstat(link)
+	if err != nil {
+		log.Panicln(err)
+	}
+	if !os.SameFile(file1Stat, linkStat) {
+		log.Panicln("expected file == link stat", file1Stat, linkStat)
+	}
+
+	// create a symbolic link
+	symlink := file1 + "-symlink"
+	if err = os.Symlink(file1, symlink); err != nil {
+		log.Panicln(err)
+	}
+
+	// verify we can read the symbolic link back
+	if dst, err := os.Readlink(symlink); err != nil {
+		log.Panicln(err)
+	} else if dst != dst {
+		log.Panicln("expected link dst = old value", dst, dst)
+	}
+
+	// Test lstat which should be about the link not its target.
+	symlinkStat, err := os.Lstat(symlink)
 	if err != nil {
 		log.Panicln(err)
 	}
 
-	if lstat.Mode().Type() != fs.ModeSymlink {
-		log.Panicln("expected type = symlink", lstat.Mode().Type())
+	if symlinkStat.Mode().Type() != fs.ModeSymlink {
+		log.Panicln("expected type = symlink", symlinkStat.Mode().Type())
 	}
-	if size := int64(len(file1)); lstat.Size() != size {
-		log.Panicln("unexpected symlink size", lstat.Size(), size)
+	if size := int64(len(file1)); symlinkStat.Size() != size {
+		log.Panicln("unexpected symlink size", symlinkStat.Size(), size)
+	}
+	// A symbolic link isn't the same file as what it points to.
+	if os.SameFile(file1Stat, symlinkStat) {
+		log.Panicln("expected file != link stat", file1Stat, symlinkStat)
 	}
 
 	// Test removing a non-empty empty directory
diff --git a/internal/platform/path.go b/internal/platform/path.go
new file mode 100644
index 00000000..361049ae
--- /dev/null
+++ b/internal/platform/path.go
@@ -0,0 +1,6 @@
+//go:build !windows
+
+package platform
+
+// ToPosixPath returns the input, as only windows might return backslashes.
+func ToPosixPath(in string) string { return in }
diff --git a/internal/platform/path_test.go b/internal/platform/path_test.go
new file mode 100644
index 00000000..73456a68
--- /dev/null
+++ b/internal/platform/path_test.go
@@ -0,0 +1,14 @@
+package platform
+
+import (
+	"path/filepath"
+	"testing"
+
+	"github.com/tetratelabs/wazero/internal/testing/require"
+)
+
+func TestToPosixPath(t *testing.T) {
+	orig := filepath.Join("a", "b", "c")
+	fixed := ToPosixPath(orig)
+	require.Equal(t, "a/b/c", fixed)
+}
diff --git a/internal/platform/path_windows.go b/internal/platform/path_windows.go
new file mode 100644
index 00000000..77c4187d
--- /dev/null
+++ b/internal/platform/path_windows.go
@@ -0,0 +1,17 @@
+package platform
+
+import "strings"
+
+// ToPosixPath returns the input, converting any backslashes to forward ones.
+func ToPosixPath(in string) string {
+	// strings.Map only allocates on change, which is good enough especially as
+	// path.Join uses forward slash even on windows.
+	return strings.Map(windowsToPosixSeparator, in)
+}
+
+func windowsToPosixSeparator(r rune) rune {
+	if r == '\\' {
+		return '/'
+	}
+	return r
+}
diff --git a/internal/platform/separator.go b/internal/platform/separator.go
deleted file mode 100644
index a98328ef..00000000
--- a/internal/platform/separator.go
+++ /dev/null
@@ -1,7 +0,0 @@
-//go:build !windows
-
-package platform
-
-// SanitizeSeparator sanitizes the path separator in the given buffer.
-// This does nothing on the non-windows platforms.
-func SanitizeSeparator([]byte) {}
diff --git a/internal/platform/separator_test.go b/internal/platform/separator_test.go
deleted file mode 100644
index 5951eb7f..00000000
--- a/internal/platform/separator_test.go
+++ /dev/null
@@ -1,14 +0,0 @@
-package platform
-
-import (
-	"path/filepath"
-	"testing"
-
-	"github.com/tetratelabs/wazero/internal/testing/require"
-)
-
-func TestSanitizeSeparator(t *testing.T) {
-	orig := []byte(filepath.Join("a", "b", "c"))
-	SanitizeSeparator(orig)
-	require.Equal(t, "a/b/c", string(orig))
-}
diff --git a/internal/platform/separator_windows.go b/internal/platform/separator_windows.go
deleted file mode 100644
index afe1cfe6..00000000
--- a/internal/platform/separator_windows.go
+++ /dev/null
@@ -1,10 +0,0 @@
-package platform
-
-// SanitizeSeparator sanitizes the path separator in the given buffer.
-func SanitizeSeparator(in []byte) {
-	for i := range in {
-		if in[i] == '\\' {
-			in[i] = '/'
-		}
-	}
-}
diff --git a/internal/sysfs/dirfs.go b/internal/sysfs/dirfs.go
index 5d54c6d9..5ab4b9a5 100644
--- a/internal/sysfs/dirfs.go
+++ b/internal/sysfs/dirfs.go
@@ -78,23 +78,14 @@ func (d *dirFS) Rename(from, to string) error {
 }
 
 // Readlink implements FS.Readlink
-func (d *dirFS) Readlink(path string, buf []byte) (n int, err error) {
+func (d *dirFS) Readlink(path string) (string, error) {
 	// Note: do not use syscall.Readlink as that causes race on Windows.
 	// In any case, syscall.Readlink does almost the same logic as os.Readlink.
-	res, err := os.Readlink(d.join(path))
+	dst, err := os.Readlink(d.join(path))
 	if err != nil {
-		err = platform.UnwrapOSError(err)
-		return
+		return "", platform.UnwrapOSError(err)
 	}
-
-	// We need to copy here, but syscall.Readlink does copy internally, so the cost is the same.
-	copy(buf, res)
-	n = len(res)
-	if n > len(buf) {
-		n = len(buf)
-	}
-	platform.SanitizeSeparator(buf[:n])
-	return
+	return platform.ToPosixPath(dst), nil
 }
 
 // Link implements FS.Link.
diff --git a/internal/sysfs/readfs.go b/internal/sysfs/readfs.go
index 4caa7a96..66f57863 100644
--- a/internal/sysfs/readfs.go
+++ b/internal/sysfs/readfs.go
@@ -152,6 +152,6 @@ func (r *readFS) Stat(path string, stat *platform.Stat_t) error {
 }
 
 // Readlink implements FS.Readlink
-func (r *readFS) Readlink(path string, buf []byte) (n int, err error) {
-	return r.fs.Readlink(path, buf)
+func (r *readFS) Readlink(path string) (dst string, err error) {
+	return r.fs.Readlink(path)
 }
diff --git a/internal/sysfs/sysfs.go b/internal/sysfs/sysfs.go
index b4806025..906bedaa 100644
--- a/internal/sysfs/sysfs.go
+++ b/internal/sysfs/sysfs.go
@@ -225,7 +225,7 @@ type FS interface {
 	//   - On Windows, the path separator is different from other platforms,
 	//     but to provide consistent results to Wasm, this normalizes to a "/"
 	//     separator.
-	Readlink(path string, buf []byte) (n int, err error)
+	Readlink(path string) (string, error)
 
 	// Truncate is similar to syscall.Truncate, except the path is relative to
 	// this file system.
diff --git a/internal/sysfs/sysfs_test.go b/internal/sysfs/sysfs_test.go
index 34fc9f4d..cecbc49e 100644
--- a/internal/sysfs/sysfs_test.go
+++ b/internal/sysfs/sysfs_test.go
@@ -370,23 +370,21 @@ func testReadlink(t *testing.T, readFS, writeFS FS) {
 		{old: "sub/test.txt", dst: "symlinked-zoo.txt"},
 	}
 
-	buf := make([]byte, 200)
 	for _, tl := range testLinks {
 		err := writeFS.Symlink(tl.old, tl.dst) // not os.Symlink for windows compat
 		require.NoError(t, err, "%v", tl)
 
-		n, err := readFS.Readlink(tl.dst, buf)
+		dst, err := readFS.Readlink(tl.dst)
 		require.NoError(t, err)
-		require.Equal(t, tl.old, string(buf[:n]))
-		require.Equal(t, len(tl.old), n)
+		require.Equal(t, tl.old, dst)
 	}
 
 	t.Run("errors", func(t *testing.T) {
-		_, err := readFS.Readlink("sub/test.txt", buf)
+		_, err := readFS.Readlink("sub/test.txt")
 		require.Error(t, err)
-		_, err = readFS.Readlink("", buf)
+		_, err = readFS.Readlink("")
 		require.Error(t, err)
-		_, err = readFS.Readlink("animals.txt", buf)
+		_, err = readFS.Readlink("animals.txt")
 		require.Error(t, err)
 	})
 }
diff --git a/internal/sysfs/unsupported.go b/internal/sysfs/unsupported.go
index b7e4d0e4..21b956be 100644
--- a/internal/sysfs/unsupported.go
+++ b/internal/sysfs/unsupported.go
@@ -57,8 +57,8 @@ func (UnimplementedFS) Rmdir(path string) error {
 }
 
 // Readlink implements FS.Readlink
-func (UnimplementedFS) Readlink(string, []byte) (int, error) {
-	return 0, syscall.ENOSYS
+func (UnimplementedFS) Readlink(path string) (string, error) {
+	return "", syscall.ENOSYS
 }
 
 // Link implements FS.Link