path_filestat_get: interpret LOOKUP_SYMLINK_FOLLOW (#1252)

Signed-off-by: Achille Roussel <achille.roussel@gmail.com>
This commit is contained in:
Achille
2023-03-17 17:23:44 -07:00
committed by GitHub
parent 5d8d8d05de
commit 6471ac7dd7
4 changed files with 164 additions and 12 deletions

View File

@@ -1419,11 +1419,7 @@ func pathFilestatGetFn(_ context.Context, mod api.Module, params []uint64) Errno
fsc := mod.(*wasm.CallContext).Sys.FS()
fd := uint32(params[0])
// TODO: flags is a lookupflags and it only has one bit: symlink_follow
// https://github.com/WebAssembly/WASI/blob/snapshot-01/phases/snapshot/docs.md#lookupflags
_ /* flags */ = uint32(params[1])
flags := uint16(params[1])
path := uint32(params[2])
pathLen := uint32(params[3])
@@ -1432,9 +1428,23 @@ func pathFilestatGetFn(_ context.Context, mod api.Module, params []uint64) Errno
return errno
}
// Stat the file without allocating a file descriptor
// Stat the file without allocating a file descriptor.
//
// Note: `preopen` is a `sysfs.FS` interface, so passing the address of `st`
// causes the value to escape to the heap because the compiler doesn't know
// whether the pointer will be retained by the method.
//
// This could be optimized by modifying Stat/Lstat to return the `Stat_t`
// value instead of passing a pointer as output parameter.
var st platform.Stat_t
if err := preopen.Stat(pathName, &st); err != nil {
var err error
if (flags & LOOKUP_SYMLINK_FOLLOW) == 0 {
err = preopen.Lstat(pathName, &st)
} else {
err = preopen.Stat(pathName, &st)
}
if err != nil {
return ToErrno(err)
}

View File

@@ -3086,6 +3086,7 @@ func Test_pathFilestatGet(t *testing.T) {
tests := []struct {
name string
fd, pathLen, resultFilestat uint32
flags uint16
memory, expectedMemory []byte
expectedErrno Errno
expectedLog string
@@ -3210,6 +3211,136 @@ func Test_pathFilestatGet(t *testing.T) {
expectedLog: `
==> wasi_snapshot_preview1.path_filestat_get(fd=3,flags=,path=animals.txt)
<== (filestat=,errno=EFAULT)
`,
},
{
name: "file under root (follow symlinks)",
fd: sys.FdPreopen,
flags: LOOKUP_SYMLINK_FOLLOW,
memory: initialMemoryFile,
pathLen: uint32(len(file)),
resultFilestat: uint32(len(file)) + 1,
expectedMemory: append(
initialMemoryFile,
0, 0, 0, 0, 0, 0, 0, 0, // dev
0, 0, 0, 0, 0, 0, 0, 0, // ino
4, 0, 0, 0, 0, 0, 0, 0, // filetype + padding
1, 0, 0, 0, 0, 0, 0, 0, // nlink
30, 0, 0, 0, 0, 0, 0, 0, // size
0x0, 0x82, 0x13, 0x80, 0x6b, 0x16, 0x24, 0x17, // atim
0x0, 0x82, 0x13, 0x80, 0x6b, 0x16, 0x24, 0x17, // mtim
0x0, 0x82, 0x13, 0x80, 0x6b, 0x16, 0x24, 0x17, // ctim
),
expectedLog: `
==> wasi_snapshot_preview1.path_filestat_get(fd=3,flags=SYMLINK_FOLLOW,path=animals.txt)
<== (filestat={filetype=REGULAR_FILE,size=30,mtim=1667482413000000000},errno=ESUCCESS)
`,
},
{
name: "file under dir (follow symlinks)",
fd: sys.FdPreopen, // root
flags: LOOKUP_SYMLINK_FOLLOW,
memory: initialMemoryFileInDir,
pathLen: uint32(len(fileInDir)),
resultFilestat: uint32(len(fileInDir)) + 1,
expectedMemory: append(
initialMemoryFileInDir,
0, 0, 0, 0, 0, 0, 0, 0, // dev
0, 0, 0, 0, 0, 0, 0, 0, // ino
4, 0, 0, 0, 0, 0, 0, 0, // filetype + padding
1, 0, 0, 0, 0, 0, 0, 0, // nlink
14, 0, 0, 0, 0, 0, 0, 0, // size
0x0, 0x0, 0xc2, 0xd3, 0x43, 0x6, 0x36, 0x17, // atim
0x0, 0x0, 0xc2, 0xd3, 0x43, 0x6, 0x36, 0x17, // mtim
0x0, 0x0, 0xc2, 0xd3, 0x43, 0x6, 0x36, 0x17, // ctim
),
expectedLog: `
==> wasi_snapshot_preview1.path_filestat_get(fd=3,flags=SYMLINK_FOLLOW,path=sub/test.txt)
<== (filestat={filetype=REGULAR_FILE,size=14,mtim=1672531200000000000},errno=ESUCCESS)
`,
},
{
name: "dir under root (follow symlinks)",
fd: sys.FdPreopen,
flags: LOOKUP_SYMLINK_FOLLOW,
memory: initialMemoryDir,
pathLen: uint32(len(dir)),
resultFilestat: uint32(len(dir)) + 1,
expectedMemory: append(
initialMemoryDir,
0, 0, 0, 0, 0, 0, 0, 0, // dev
0, 0, 0, 0, 0, 0, 0, 0, // ino
3, 0, 0, 0, 0, 0, 0, 0, // filetype + padding
1, 0, 0, 0, 0, 0, 0, 0, // nlink
0, 0, 0, 0, 0, 0, 0, 0, // size
0x0, 0x0, 0x1f, 0xa6, 0x70, 0xfc, 0xc5, 0x16, // atim
0x0, 0x0, 0x1f, 0xa6, 0x70, 0xfc, 0xc5, 0x16, // mtim
0x0, 0x0, 0x1f, 0xa6, 0x70, 0xfc, 0xc5, 0x16, // ctim
),
expectedLog: `
==> wasi_snapshot_preview1.path_filestat_get(fd=3,flags=SYMLINK_FOLLOW,path=sub)
<== (filestat={filetype=DIRECTORY,size=0,mtim=1640995200000000000},errno=ESUCCESS)
`,
},
{
name: "unopened FD (follow symlinks)",
fd: math.MaxUint32,
flags: LOOKUP_SYMLINK_FOLLOW,
expectedErrno: ErrnoBadf,
expectedLog: `
==> wasi_snapshot_preview1.path_filestat_get(fd=-1,flags=SYMLINK_FOLLOW,path=)
<== (filestat=,errno=EBADF)
`,
},
{
name: "Fd not a directory (follow symlinks)",
fd: fileFD,
flags: LOOKUP_SYMLINK_FOLLOW,
memory: initialMemoryFile,
pathLen: uint32(len(file)),
resultFilestat: 2,
expectedErrno: ErrnoNotdir,
expectedLog: `
==> wasi_snapshot_preview1.path_filestat_get(fd=4,flags=SYMLINK_FOLLOW,path=animals.txt)
<== (filestat=,errno=ENOTDIR)
`,
},
{
name: "path under root doesn't exist (follow symlinks)",
fd: sys.FdPreopen,
flags: LOOKUP_SYMLINK_FOLLOW,
memory: initialMemoryNotExists,
pathLen: 1,
resultFilestat: 2,
expectedErrno: ErrnoNoent,
expectedLog: `
==> wasi_snapshot_preview1.path_filestat_get(fd=3,flags=SYMLINK_FOLLOW,path=?)
<== (filestat=,errno=ENOENT)
`,
},
{
name: "path is out of memory (follow symlinks)",
fd: sys.FdPreopen,
flags: LOOKUP_SYMLINK_FOLLOW,
memory: initialMemoryFile,
pathLen: memorySize,
expectedErrno: ErrnoFault,
expectedLog: `
==> wasi_snapshot_preview1.path_filestat_get(fd=3,flags=SYMLINK_FOLLOW,path=OOM(1,65536))
<== (filestat=,errno=EFAULT)
`,
},
{
name: "resultFilestat exceeds the maximum valid address by 1 (follow symlinks)",
fd: sys.FdPreopen,
flags: LOOKUP_SYMLINK_FOLLOW,
memory: initialMemoryFile,
pathLen: uint32(len(file)),
resultFilestat: memorySize - 64 + 1,
expectedErrno: ErrnoFault,
expectedLog: `
==> wasi_snapshot_preview1.path_filestat_get(fd=3,flags=SYMLINK_FOLLOW,path=animals.txt)
<== (filestat=,errno=EFAULT)
`,
},
}
@@ -3223,8 +3354,7 @@ func Test_pathFilestatGet(t *testing.T) {
maskMemory(t, mod, len(tc.expectedMemory))
mod.Memory().Write(0, tc.memory)
flags := uint32(0)
requireErrnoResult(t, tc.expectedErrno, mod, PathFilestatGetName, uint64(tc.fd), uint64(flags), uint64(1), uint64(tc.pathLen), uint64(tc.resultFilestat))
requireErrnoResult(t, tc.expectedErrno, mod, PathFilestatGetName, uint64(tc.fd), uint64(tc.flags), uint64(1), uint64(tc.pathLen), uint64(tc.resultFilestat))
require.Equal(t, tc.expectedLog, "\n"+log.String())
actual, ok := mod.Memory().Read(0, uint32(len(tc.expectedMemory)))

View File

@@ -61,6 +61,19 @@ func (a *adapter) Stat(path string, stat *platform.Stat_t) error {
return platform.StatFile(f, stat)
}
// Lstat implements FS.Lstat
func (a *adapter) Lstat(path string, stat *platform.Stat_t) error {
// At this time, we make the assumption that fs.FS instances do not support
// symbolic links, therefore Lstat is the same as Stat. This is obviously
// not true but until fs.FS has a solid story for how to handle symlinks we
// are better off not making a decision that would be difficult to revert
// later on.
//
// For further discussions on the topic, see:
// https://github.com/golang/go/issues/49580
return a.Stat(path, stat)
}
func cleanPath(name string) string {
if len(name) == 0 {
return name

View File

@@ -4,6 +4,7 @@ import (
"errors"
"io/fs"
"os"
"path/filepath"
"runtime"
"syscall"
"testing"
@@ -116,8 +117,6 @@ func TestAdapt_Open_Read(t *testing.T) {
})
}
// TestAdapt_Lstat is unsupported because the Lstat() function is not implemented
// on os.File.
func TestAdapt_Lstat(t *testing.T) {
tmpDir := t.TempDir()
require.NoError(t, fstest.WriteTestFiles(tmpDir))
@@ -128,7 +127,7 @@ func TestAdapt_Lstat(t *testing.T) {
linkPath := joinPath(tmpDir, path+"-link")
require.NoError(t, os.Symlink(fullPath, linkPath))
var stat platform.Stat_t
require.EqualErrno(t, syscall.ENOSYS, testFS.Lstat(linkPath, &stat))
require.NoError(t, testFS.Lstat(filepath.Base(linkPath), &stat))
}
}