From 798ff20f81ab6e5d2638c5ddc4cfc8e92083d456 Mon Sep 17 00:00:00 2001 From: Crypt Keeper <64215+codefromthecrypt@users.noreply.github.com> Date: Mon, 27 Jun 2022 13:29:35 +0800 Subject: [PATCH] Removes WithWorkDirFS and "." resolution (#660) This removes WithWorkDirFS and any other attempts to resolve the current directory (".") in host functions. This is a reaction to reality of compilers who track this inside wasm (not via host functions). One nice side effect is substantially simpler internal implementation of file-systems. This also allows experimental.WithFS to block file access via passing nil. Signed-off-by: Adrian Cole --- RATIONALE.md | 28 +++ assemblyscript/assemblyscript.go | 4 +- config.go | 58 ++--- config_test.go | 117 ++-------- examples/wasi/cat.go | 3 +- examples/wasi/cat_test.go | 4 +- experimental/fs.go | 18 +- experimental/fs_example_test.go | 5 +- experimental/fs_test.go | 22 +- internal/integration_test/fs/fs_test.go | 2 +- .../integration_test/spectest/spectest.go | 2 +- internal/sys/fs.go | 203 +++++++----------- internal/sys/fs_test.go | 89 ++++---- internal/sys/sys.go | 27 +-- internal/sys/sys_test.go | 17 +- internal/testing/fs/fs.go | 31 +++ internal/testing/fs/fs_test.go | 38 ++++ internal/testing/require/require.go | 3 +- internal/wasm/call_context.go | 7 +- internal/wasm/call_context_test.go | 29 +-- internal/wasm/namespace_test.go | 10 +- internal/wasm/store_test.go | 4 +- wasi_snapshot_preview1/wasi.go | 69 +++--- wasi_snapshot_preview1/wasi_test.go | 197 ++++++++--------- 24 files changed, 473 insertions(+), 514 deletions(-) create mode 100644 internal/testing/fs/fs.go create mode 100644 internal/testing/fs/fs_test.go diff --git a/RATIONALE.md b/RATIONALE.md index 4b869a8a..01c85de5 100644 --- a/RATIONALE.md +++ b/RATIONALE.md @@ -379,6 +379,34 @@ file descriptors if resources aren't managed properly. In other words, blind cop violate isolation or endanger the embedding process. In summary, we try to be similar to normal Go code, but often need act differently and document `ModuleConfig` is more about emulating, not necessarily performing real system calls. +## File systems + +### Why doesn't wazero implement the working directory? + +An early design of wazero's API included a `WithWorkDirFS` which allowed +control over which file a relative path such as "./config.yml" resolved to, +independent of the root file system. This intended to help separate concerns +like mutability of files, but it didn't work and was removed. + +Compilers that target wasm act differently with regard to the working +directory. For example, while `GOOS=js` uses host functions to track the +working directory, WASI host functions do not. wasi-libc, used by TinyGo, +tracks working directory changes in compiled wasm instead: initially "/" until +code calls `chdir`. + +The only place wazero can standardize a layered concern is via a host function. +Since WASI doesn't use host functions to track the working directory, we can't +standardize the storage and initial value of it. + +Meanwhile, code may be able to affect the working directory by compiling +`chdir` into their main function, using an argument or ENV for the initial +value (possibly `PWD`). Those unable to control the compiled code should only +use absolute paths in configuration. + +See +* https://github.com/golang/go/blob/go1.19beta1/src/syscall/fs_js.go#L324 +* https://github.com/WebAssembly/wasi-libc/pull/214#issue-673090117 + ### FdPrestatDirName `FdPrestatDirName` is a WASI function to return the path of the pre-opened directory of a file descriptor. diff --git a/assemblyscript/assemblyscript.go b/assemblyscript/assemblyscript.go index a2ed9241..aff0eb6a 100644 --- a/assemblyscript/assemblyscript.go +++ b/assemblyscript/assemblyscript.go @@ -57,12 +57,12 @@ type Builder interface { // Compile compiles the "env" module that can instantiated in any namespace (wazero.Namespace). // - // Note: This has the same effect as the same function name on wazero.ModuleBuilder. + // Note: This has the same effect as the same function on wazero.ModuleBuilder. Compile(context.Context, wazero.CompileConfig) (wazero.CompiledModule, error) // Instantiate instantiates the "env" module into the provided namespace. // - // Note: This has the same effect as the same function name on wazero.ModuleBuilder. + // Note: This has the same effect as the same function on wazero.ModuleBuilder. Instantiate(context.Context, wazero.Namespace) (api.Closer, error) } diff --git a/config.go b/config.go index f7fca6a2..cf0c87f5 100644 --- a/config.go +++ b/config.go @@ -394,10 +394,10 @@ type ModuleConfig interface { // See https://linux.die.net/man/3/environ and https://en.wikipedia.org/wiki/Null-terminated_string WithEnv(key, value string) ModuleConfig - // WithFS assigns the file system to use for any paths beginning at "/". Defaults to not found. - // Note: This sets WithWorkDirFS to the same file-system unless already set. + // WithFS assigns the file system to use for any paths beginning at "/". + // Defaults return fs.ErrNotExist. // - // Ex. This sets a read-only, embedded file-system to serve files under the root ("/") and working (".") directories: + // Ex. This sets a read-only, embedded file-system: // // //go:embed testdata/index.html // var testdataIndex embed.FS @@ -405,9 +405,24 @@ type ModuleConfig interface { // rooted, err := fs.Sub(testdataIndex, "testdata") // require.NoError(t, err) // - // // "index.html" is accessible as both "/index.html" and "./index.html" because we didn't use WithWorkDirFS. + // // "index.html" is accessible as "/index.html". // config := wazero.NewModuleConfig().WithFS(rooted) // + // Ex. This sets a mutable file-system: + // + // // Files relative to "/work/appA" are accessible as "/". + // config := wazero.NewModuleConfig().WithFS(os.DirFS("/work/appA")) + // + // Isolation + // + // os.DirFS documentation includes important notes about isolation, which + // also applies to fs.Sub. As of Go 1.19, the built-in file-systems are not + // jailed (chroot). See https://github.com/golang/go/issues/42322 + // + // Working Directory "." + // + // Relative path resolution, such as "./config.yml" to "/config.yml" or + // otherwise, is compiler-specific. See /RATIONALE.md for notes. WithFS(fs.FS) ModuleConfig // WithName configures the module name. Defaults to what was decoded or overridden via CompileConfig.WithModuleName. @@ -531,20 +546,6 @@ type ModuleConfig interface { // // Note: The caller is responsible to close any io.Reader they supply: It is not closed on api.Module Close. WithRandSource(io.Reader) ModuleConfig - - // WithWorkDirFS indicates the file system to use for any paths beginning at "./". Defaults to the same as WithFS. - // - // Ex. This sets a read-only, embedded file-system as the root ("/"), and a mutable one as the working directory ("."): - // - // //go:embed appA - // var rootFS embed.FS - // - // // Files relative to this source under appA are available under "/" and files relative to "/work/appA" under ".". - // config := wazero.NewModuleConfig().WithFS(rootFS).WithWorkDirFS(os.DirFS("/work/appA")) - // - // Note: os.DirFS documentation includes important notes about isolation, which also applies to fs.Sub. As of Go 1.18, - // the built-in file-systems are not jailed (chroot). See https://github.com/golang/go/issues/42322 - WithWorkDirFS(fs.FS) ModuleConfig } type moduleConfig struct { @@ -564,7 +565,8 @@ type moduleConfig struct { environ []string // environKeys allow overwriting of existing values. environKeys map[string]int - fs *internalsys.FSConfig + // fs is the file system to open files with + fs fs.FS } // NewModuleConfig returns a ModuleConfig that can be used for configuring module instantiation. @@ -572,7 +574,6 @@ func NewModuleConfig() ModuleConfig { return &moduleConfig{ startFunctions: []string{"_start"}, environKeys: map[string]int{}, - fs: internalsys.NewFSConfig(), } } @@ -583,7 +584,6 @@ func (c *moduleConfig) clone() *moduleConfig { for key, value := range c.environKeys { ret.environKeys[key] = value } - ret.fs = c.fs.Clone() return &ret } @@ -610,7 +610,7 @@ func (c *moduleConfig) WithEnv(key, value string) ModuleConfig { // WithFS implements ModuleConfig.WithFS func (c *moduleConfig) WithFS(fs fs.FS) ModuleConfig { ret := c.clone() - ret.fs = ret.fs.WithFS(fs) + ret.fs = fs return ret } @@ -698,13 +698,6 @@ func (c *moduleConfig) WithRandSource(source io.Reader) ModuleConfig { return ret } -// WithWorkDirFS implements ModuleConfig.WithWorkDirFS -func (c *moduleConfig) WithWorkDirFS(fs fs.FS) ModuleConfig { - ret := c.clone() - ret.fs = ret.fs.WithWorkDirFS(fs) - return ret -} - // toSysContext creates a baseline wasm.Context configured by ModuleConfig. func (c *moduleConfig) toSysContext() (sysCtx *internalsys.Context, err error) { var environ []string // Intentionally doesn't pre-allocate to reduce logic to default to nil. @@ -724,11 +717,6 @@ func (c *moduleConfig) toSysContext() (sysCtx *internalsys.Context, err error) { environ = append(environ, key+"="+value) } - preopens, err := c.fs.Preopens() - if err != nil { - return nil, err - } - return internalsys.NewContext( math.MaxUint32, c.args, @@ -740,6 +728,6 @@ func (c *moduleConfig) toSysContext() (sysCtx *internalsys.Context, err error) { c.walltime, c.walltimeResolution, c.nanotime, c.nanotimeResolution, c.nanosleep, - preopens, + c.fs, ) } diff --git a/config_test.go b/config_test.go index 1871594e..0ece5ca9 100644 --- a/config_test.go +++ b/config_test.go @@ -3,6 +3,7 @@ package wazero import ( "context" "io" + "io/fs" "math" "reflect" "testing" @@ -10,6 +11,7 @@ import ( "github.com/tetratelabs/wazero/api" internalsys "github.com/tetratelabs/wazero/internal/sys" + testfs "github.com/tetratelabs/wazero/internal/testing/fs" "github.com/tetratelabs/wazero/internal/testing/require" "github.com/tetratelabs/wazero/internal/wasm" "github.com/tetratelabs/wazero/sys" @@ -304,8 +306,8 @@ func TestModuleConfig_toSysContext(t *testing.T) { base.(*moduleConfig).nanotime = &nt base.(*moduleConfig).nanotimeResolution = 1 - testFS := fstest.MapFS{} - testFS2 := fstest.MapFS{} + testFS := testfs.FS{} + testFS2 := testfs.FS{"/": &testfs.File{}} tests := []struct { name string @@ -326,7 +328,7 @@ func TestModuleConfig_toSysContext(t *testing.T) { &wt, 1, // walltime, walltimeResolution &nt, 1, // nanotime, nanotimeResolution nil, // nanosleep - nil, // openedFiles + nil, // fs ), }, { @@ -343,7 +345,7 @@ func TestModuleConfig_toSysContext(t *testing.T) { &wt, 1, // walltime, walltimeResolution &nt, 1, // nanotime, nanotimeResolution nil, // nanosleep - nil, // openedFiles + nil, // fs ), }, { @@ -360,7 +362,7 @@ func TestModuleConfig_toSysContext(t *testing.T) { &wt, 1, // walltime, walltimeResolution &nt, 1, // nanotime, nanotimeResolution nil, // nanosleep - nil, // openedFiles + nil, // fs ), }, { @@ -377,7 +379,7 @@ func TestModuleConfig_toSysContext(t *testing.T) { &wt, 1, // walltime, walltimeResolution &nt, 1, // nanotime, nanotimeResolution nil, // nanosleep - nil, // openedFiles + nil, // fs ), }, { @@ -394,7 +396,7 @@ func TestModuleConfig_toSysContext(t *testing.T) { &wt, 1, // walltime, walltimeResolution &nt, 1, // nanotime, nanotimeResolution nil, // nanosleep - nil, // openedFiles + nil, // fs ), }, { @@ -411,7 +413,7 @@ func TestModuleConfig_toSysContext(t *testing.T) { &wt, 1, // walltime, walltimeResolution &nt, 1, // nanotime, nanotimeResolution nil, // nanosleep - nil, // openedFiles + nil, // fs ), }, { @@ -428,7 +430,7 @@ func TestModuleConfig_toSysContext(t *testing.T) { &wt, 1, // walltime, walltimeResolution &nt, 1, // nanotime, nanotimeResolution nil, // nanosleep - nil, // openedFiles + nil, // fs ), }, { @@ -445,7 +447,7 @@ func TestModuleConfig_toSysContext(t *testing.T) { &wt, 1, // walltime, walltimeResolution &nt, 1, // nanotime, nanotimeResolution nil, // nanosleep - nil, // openedFiles + nil, // fs ), }, { @@ -462,7 +464,7 @@ func TestModuleConfig_toSysContext(t *testing.T) { &wt, 1, // walltime, walltimeResolution &nt, 1, // nanotime, nanotimeResolution nil, // nanosleep - nil, // openedFiles + nil, // fs ), }, { @@ -479,10 +481,7 @@ func TestModuleConfig_toSysContext(t *testing.T) { &wt, 1, // walltime, walltimeResolution &nt, 1, // nanotime, nanotimeResolution nil, // nanosleep - map[uint32]*internalsys.FileEntry{ // openedFiles - 3: {Path: "/", FS: testFS}, - 4: {Path: ".", FS: testFS}, - }, + testFS, ), }, { @@ -498,70 +497,8 @@ func TestModuleConfig_toSysContext(t *testing.T) { nil, // randSource &wt, 1, // walltime, walltimeResolution &nt, 1, // nanotime, nanotimeResolution - nil, // nanosleep - map[uint32]*internalsys.FileEntry{ // openedFiles - 3: {Path: "/", FS: testFS2}, - 4: {Path: ".", FS: testFS2}, - }, - ), - }, - { - name: "WithWorkDirFS", - input: base.WithWorkDirFS(testFS), - expected: requireSysContext(t, - math.MaxUint32, // max - nil, // args - nil, // environ - nil, // stdin - nil, // stdout - nil, // stderr - nil, // randSource - &wt, 1, // walltime, walltimeResolution - &nt, 1, // nanotime, nanotimeResolution - nil, // nanosleep - map[uint32]*internalsys.FileEntry{ // openedFiles - 3: {Path: ".", FS: testFS}, - }, - ), - }, - { - name: "WithFS and WithWorkDirFS", - input: base.WithFS(testFS).WithWorkDirFS(testFS2), - expected: requireSysContext(t, - math.MaxUint32, // max - nil, // args - nil, // environ - nil, // stdin - nil, // stdout - nil, // stderr - nil, // randSource - &wt, 1, // walltime, walltimeResolution - &nt, 1, // nanotime, nanotimeResolution - nil, // nanosleep - map[uint32]*internalsys.FileEntry{ // openedFiles - 3: {Path: "/", FS: testFS}, - 4: {Path: ".", FS: testFS2}, - }, - ), - }, - { - name: "WithWorkDirFS and WithFS", - input: base.WithWorkDirFS(testFS).WithFS(testFS2), - expected: requireSysContext(t, - math.MaxUint32, // max - nil, // args - nil, // environ - nil, // stdin - nil, // stdout - nil, // stderr - nil, // randSource - &wt, 1, // walltime, walltimeResolution - &nt, 1, // nanotime, nanotimeResolution - nil, // nanosleep - map[uint32]*internalsys.FileEntry{ // openedFiles - 3: {Path: ".", FS: testFS}, - 4: {Path: "/", FS: testFS2}, - }, + nil, // nanosleep + testFS2, // fs ), }, } @@ -764,16 +701,6 @@ func TestModuleConfig_toSysContext_Errors(t *testing.T) { input: NewModuleConfig().WithEnv("", "a"), expectedErr: "environ invalid: empty key", }, - { - name: "WithFS nil", - input: NewModuleConfig().WithFS(nil), - expectedErr: "FS for / is nil", - }, - { - name: "WithWorkDirFS nil", - input: NewModuleConfig().WithWorkDirFS(nil), - expectedErr: "FS for . is nil", - }, } for _, tt := range tests { tc := tt @@ -788,8 +715,8 @@ func TestModuleConfig_clone(t *testing.T) { mc := NewModuleConfig().(*moduleConfig) cloned := mc.clone() - fs1 := fstest.MapFS{} - mc.fs.WithWorkDirFS(fs1) + // Make post-clone changes + mc.fs = fstest.MapFS{} mc.environKeys["2"] = 2 cloned.environKeys["1"] = 1 @@ -799,9 +726,7 @@ func TestModuleConfig_clone(t *testing.T) { require.Equal(t, map[string]int{"1": 1}, cloned.environKeys) // Ensure the fs is not shared - preopens, err := cloned.fs.Preopens() - require.NoError(t, err) - require.Equal(t, map[uint32]*internalsys.FileEntry{}, preopens) + require.Nil(t, cloned.fs) } func TestCompiledCode_Close(t *testing.T) { @@ -839,7 +764,7 @@ func requireSysContext( walltime *sys.Walltime, walltimeResolution sys.ClockResolution, nanotime *sys.Nanotime, nanotimeResolution sys.ClockResolution, nanosleep *sys.Nanosleep, - openedFiles map[uint32]*internalsys.FileEntry, + fs fs.FS, ) *internalsys.Context { sysCtx, err := internalsys.NewContext( max, @@ -852,7 +777,7 @@ func requireSysContext( walltime, walltimeResolution, nanotime, nanotimeResolution, nanosleep, - openedFiles, + fs, ) require.NoError(t, err) return sysCtx diff --git a/examples/wasi/cat.go b/examples/wasi/cat.go index ae0f17a4..81576885 100644 --- a/examples/wasi/cat.go +++ b/examples/wasi/cat.go @@ -53,8 +53,7 @@ func main() { } // InstantiateModule runs the "_start" function which is what TinyGo compiles "main" to. - // * Set the program name (arg[0]) to "wasi" and add args to write "test.txt" to stdout twice. - // * We use "/test.txt" or "./test.txt" because WithFS by default maps the workdir "." to "/". + // * Set the program name (arg[0]) to "wasi" and add args to write "/test.txt" to stdout twice. if _, err = r.InstantiateModule(ctx, code, config.WithArgs("wasi", os.Args[1])); err != nil { log.Panicln(err) } diff --git a/examples/wasi/cat_test.go b/examples/wasi/cat_test.go index 28c5f683..f6f3aa1b 100644 --- a/examples/wasi/cat_test.go +++ b/examples/wasi/cat_test.go @@ -9,8 +9,8 @@ import ( // Test_main ensures the following will work: // -// go run cat.go ./test.txt +// go run cat.go /test.txt func Test_main(t *testing.T) { - stdout, _ := maintester.TestMain(t, main, "cat", "./test.txt") + stdout, _ := maintester.TestMain(t, main, "cat", "/test.txt") require.Equal(t, "greet filesystem\n", stdout) } diff --git a/experimental/fs.go b/experimental/fs.go index 9e1187ec..1e7eabbf 100644 --- a/experimental/fs.go +++ b/experimental/fs.go @@ -8,16 +8,14 @@ import ( internalfs "github.com/tetratelabs/wazero/internal/sys" ) -// WithFS overrides fs.FS in the context-based manner. Caller needs to take responsibility for closing the filesystem. +// WithFS overrides fs.FS in the context-based manner. Caller needs to take +// responsibility for closing the filesystem. // -// Note: This has the same effect as the same function name on wazero.ModuleConfig. -func WithFS(ctx context.Context, fs fs.FS) (context.Context, api.Closer, error) { - fsConfig := internalfs.NewFSConfig().WithFS(fs) - preopens, err := fsConfig.Preopens() - if err != nil { - return nil, nil, err +// Note: This has the same effect as the same function on wazero.ModuleConfig. +func WithFS(ctx context.Context, fs fs.FS) (context.Context, api.Closer) { + if fs == nil { + fs = internalfs.EmptyFS } - - fsCtx := internalfs.NewFSContext(preopens) - return context.WithValue(ctx, internalfs.FSKey{}, fsCtx), fsCtx, nil + fsCtx := internalfs.NewFSContext(fs) + return context.WithValue(ctx, internalfs.FSKey{}, fsCtx), fsCtx } diff --git a/experimental/fs_example_test.go b/experimental/fs_example_test.go index e1a85f7e..ebaaf079 100644 --- a/experimental/fs_example_test.go +++ b/experimental/fs_example_test.go @@ -37,10 +37,7 @@ func Example_withFS() { // Setup the filesystem overlay, noting that it can fail if the directory is // invalid and must be closed. - ctx, closer, err := experimental.WithFS(ctx, os.DirFS(".")) - if err != nil { - log.Panicln(err) - } + ctx, closer := experimental.WithFS(ctx, os.DirFS(".")) defer closer.Close(ctx) fdPrestatDirName := mod.ExportedFunction("fd_prestat_dir_name") diff --git a/experimental/fs_test.go b/experimental/fs_test.go index 4c5bbacd..dffaa88f 100644 --- a/experimental/fs_test.go +++ b/experimental/fs_test.go @@ -3,7 +3,6 @@ package experimental_test import ( "context" _ "embed" - "log" "testing" "testing/fstest" @@ -18,10 +17,7 @@ func TestWithFS(t *testing.T) { mapfs := fstest.MapFS{fileName: &fstest.MapFile{Data: []byte(`animals`)}} // Set context to one that has experimental fs config - ctx, closer, err := experimental.WithFS(context.Background(), mapfs) - if err != nil { - log.Panicln(err) - } + ctx, closer := experimental.WithFS(context.Background(), mapfs) defer closer.Close(ctx) v := ctx.Value(sys.FSKey{}) @@ -29,13 +25,19 @@ func TestWithFS(t *testing.T) { fsCtx, ok := v.(*sys.FSContext) require.True(t, ok) - entry, ok := fsCtx.OpenedFile(3) + entry, ok := fsCtx.OpenedFile(ctx, 3) require.True(t, ok) require.Equal(t, "/", entry.Path) - require.Equal(t, mapfs, entry.FS) - entry, ok = fsCtx.OpenedFile(4) + // Override to nil context, ex to block file access + ctx, closer = experimental.WithFS(ctx, nil) + defer closer.Close(ctx) + + v = ctx.Value(sys.FSKey{}) + require.NotNil(t, v) + fsCtx, ok = v.(*sys.FSContext) require.True(t, ok) - require.Equal(t, ".", entry.Path) - require.Equal(t, mapfs, entry.FS) + + _, ok = fsCtx.OpenedFile(ctx, 3) + require.False(t, ok) } diff --git a/internal/integration_test/fs/fs_test.go b/internal/integration_test/fs/fs_test.go index e6e1aaa4..4d511956 100644 --- a/internal/integration_test/fs/fs_test.go +++ b/internal/integration_test/fs/fs_test.go @@ -154,7 +154,7 @@ func TestReader(t *testing.T) { require.NoError(t, err) realFs := fstest.MapFS{"animals.txt": &fstest.MapFile{Data: animals}} - sys := wazero.NewModuleConfig().WithWorkDirFS(realFs) + sys := wazero.NewModuleConfig().WithFS(realFs) // Create a module that just delegates to wasi functions. compiled, err := r.CompileModule(testCtx, fsWasm, wazero.NewCompileConfig()) diff --git a/internal/integration_test/spectest/spectest.go b/internal/integration_test/spectest/spectest.go index aa825f29..78e7cdba 100644 --- a/internal/integration_test/spectest/spectest.go +++ b/internal/integration_test/spectest/spectest.go @@ -385,7 +385,7 @@ func addSpectestModule(t *testing.T, s *wasm.Store, ns *wasm.Namespace) { err = s.Engine.CompileModule(testCtx, mod) require.NoError(t, err) - _, err = s.Instantiate(testCtx, ns, mod, mod.NameSection.ModuleName, sys.DefaultContext(), nil) + _, err = s.Instantiate(testCtx, ns, mod, mod.NameSection.ModuleName, sys.DefaultContext(nil), nil) require.NoError(t, err) } diff --git a/internal/sys/fs.go b/internal/sys/fs.go index 59f12f8f..9249f831 100644 --- a/internal/sys/fs.go +++ b/internal/sys/fs.go @@ -2,7 +2,7 @@ package sys import ( "context" - "fmt" + "errors" "io/fs" "math" "sync/atomic" @@ -13,15 +13,33 @@ import ( // See https://github.com/tetratelabs/wazero/issues/491 type FSKey struct{} +// EmptyFS is exported to special-case an empty file system. +var EmptyFS = &emptyFS{} + +type emptyFS struct{} + +// compile-time check to ensure emptyFS implements fs.FS +var _ fs.FS = &emptyFS{} + +// Open implements the same method as documented on fs.FS. +func (f *emptyFS) Open(name string) (fs.File, error) { + if !fs.ValidPath(name) { + return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrInvalid} + } + return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} +} + // FileEntry maps a path to an open file in a file system. type FileEntry struct { Path string - FS fs.FS - // File when nil this is a mount like "." or "/". + // File when nil this is the root "/" (fd=3) File fs.File } type FSContext struct { + // fs is the root ("/") mount. + fs fs.FS + // openedFiles is a map of file descriptor numbers (>=3) to open files (or directories) and defaults to empty. // TODO: This is unguarded, so not goroutine-safe! openedFiles map[uint32]*FileEntry @@ -30,25 +48,31 @@ type FSContext struct { lastFD uint32 } -func NewFSContext(openedFiles map[uint32]*FileEntry) *FSContext { - var fsCtx FSContext - if openedFiles == nil { - fsCtx.openedFiles = map[uint32]*FileEntry{} - fsCtx.lastFD = 2 // STDERR - } else { - fsCtx.openedFiles = openedFiles - fsCtx.lastFD = 2 // STDERR - for fd := range openedFiles { - if fd > fsCtx.lastFD { - fsCtx.lastFD = fd - } - } +// emptyFSContext is the context associated with EmptyFS. +// +// Note: This is not mutable as operations functions do not affect field state. +var emptyFSContext = &FSContext{ + fs: EmptyFS, + openedFiles: map[uint32]*FileEntry{}, + lastFD: 2, +} + +// NewFSContext returns a mutable context if the fs is not EmptyFS. +func NewFSContext(fs fs.FS) *FSContext { + if fs == EmptyFS { + return emptyFSContext + } + return &FSContext{ + fs: fs, + openedFiles: map[uint32]*FileEntry{ + 3: {Path: "/"}, // after STDERR + }, + lastFD: 3, } - return &fsCtx } // nextFD gets the next file descriptor number in a goroutine safe way (monotonically) or zero if we ran out. -// TODO: opendFiles is still not goroutine safe! +// TODO: openedFiles is still not goroutine safe! // TODO: This can return zero if we ran out of file descriptors. A future change can optimize by re-using an FD pool. func (c *FSContext) nextFD() uint32 { if c.lastFD == math.MaxUint32 { @@ -57,22 +81,40 @@ func (c *FSContext) nextFD() uint32 { return atomic.AddUint32(&c.lastFD, 1) } -// Close implements io.Closer -func (c *FSContext) Close(_ context.Context) (err error) { - // Close any files opened in this context - for fd, entry := range c.openedFiles { - delete(c.openedFiles, fd) - if entry.File != nil { // File is nil for a mount like "." or "/" - if e := entry.File.Close(); e != nil { - err = e // This means the err returned == the last non-nil error. - } - } +// OpenedFile returns a file and true if it was opened or nil and false, if not. +func (c *FSContext) OpenedFile(_ context.Context, fd uint32) (*FileEntry, bool) { + f, ok := c.openedFiles[fd] + return f, ok +} + +// OpenFile is like syscall.Open and returns the file descriptor of the new file or an error. +// +// TODO: Consider dirflags and oflags. Also, allow non-read-only open based on config about the mount. +// Ex. allow os.O_RDONLY, os.O_WRONLY, or os.O_RDWR either by config flag or pattern on filename +// See #390 +func (c *FSContext) OpenFile(_ context.Context, name string /* TODO: flags int, perm int */) (uint32, error) { + // fs.ValidFile cannot start with '/' + fsOpenPath := name + if name[0] == '/' { + fsOpenPath = name[1:] } - return + + f, err := c.fs.Open(fsOpenPath) + if err != nil { + return 0, err // Don't wrap the underlying error which is already a PathError! + } + + newFD := c.nextFD() + if newFD == 0 { + _ = f.Close() + return 0, &fs.PathError{Op: "open", Path: name, Err: errors.New("out of file descriptors")} + } + c.openedFiles[newFD] = &FileEntry{Path: name, File: f} + return newFD, nil } // CloseFile returns true if a file was opened and closed without error, or false if not. -func (c *FSContext) CloseFile(fd uint32) (bool, error) { +func (c *FSContext) CloseFile(_ context.Context, fd uint32) (bool, error) { f, ok := c.openedFiles[fd] if !ok { return false, nil @@ -88,97 +130,16 @@ func (c *FSContext) CloseFile(fd uint32) (bool, error) { return true, nil } -// OpenedFile returns a file and true if it was opened or nil and false, if not. -func (c *FSContext) OpenedFile(fd uint32) (*FileEntry, bool) { - f, ok := c.openedFiles[fd] - return f, ok -} - -// OpenFile returns the file descriptor of the new file or false if we ran out of file descriptors -func (c *FSContext) OpenFile(f *FileEntry) (uint32, bool) { - newFD := c.nextFD() - if newFD == 0 { - return 0, false - } - c.openedFiles[newFD] = f - return newFD, true -} - -type FSConfig struct { - // preopenFD has the next FD number to use - preopenFD uint32 - // preopens are keyed on file descriptor and only include the Path and FS fields. - preopens map[uint32]*FileEntry - // preopenPaths allow overwriting of existing paths. - preopenPaths map[string]uint32 -} - -func NewFSConfig() *FSConfig { - return &FSConfig{ - preopenFD: uint32(3), // after stdin/stdout/stderr - preopens: map[uint32]*FileEntry{}, - preopenPaths: map[string]uint32{}, - } -} - -// Clone makes a deep copy of this FS config. -func (c *FSConfig) Clone() *FSConfig { - ret := *c // copy except maps which share a ref - ret.preopens = make(map[uint32]*FileEntry, len(c.preopens)) - for key, value := range c.preopens { - ret.preopens[key] = value - } - ret.preopenPaths = make(map[string]uint32, len(c.preopenPaths)) - for key, value := range c.preopenPaths { - ret.preopenPaths[key] = value - } - return &ret -} - -// setFS maps a path to a file-system. This is only used for base paths: "/" and ".". -func (c *FSConfig) setFS(path string, fs fs.FS) { - // Check to see if this key already exists and update it. - entry := &FileEntry{Path: path, FS: fs} - if fd, ok := c.preopenPaths[path]; ok { - c.preopens[fd] = entry - } else { - c.preopens[c.preopenFD] = entry - c.preopenPaths[path] = c.preopenFD - c.preopenFD++ - } -} - -func (c *FSConfig) WithFS(fs fs.FS) *FSConfig { - ret := c.Clone() - ret.setFS("/", fs) - return ret -} - -func (c *FSConfig) WithWorkDirFS(fs fs.FS) *FSConfig { - ret := c.Clone() - ret.setFS(".", fs) - return ret -} - -func (c *FSConfig) Preopens() (map[uint32]*FileEntry, error) { - // Ensure no-one set a nil FD. We do this here instead of at the call site to allow chaining as nil is unexpected. - rootFD := uint32(0) // zero is invalid - setWorkDirFS := false - preopens := c.preopens - for fd, entry := range preopens { - if entry.FS == nil { - return nil, fmt.Errorf("FS for %s is nil", entry.Path) - } else if entry.Path == "/" { - rootFD = fd - } else if entry.Path == "." { - setWorkDirFS = true +// Close implements io.Closer +func (c *FSContext) Close(_ context.Context) (err error) { + // Close any files opened in this context + for fd, entry := range c.openedFiles { + delete(c.openedFiles, fd) + if entry.File != nil { // File is nil for the root filesystem + if e := entry.File.Close(); e != nil { + err = e // This means the err returned == the last non-nil error. + } } } - - // Default the working directory to the root FS if it exists. - if rootFD != 0 && !setWorkDirFS { - preopens[c.preopenFD] = &FileEntry{Path: ".", FS: preopens[rootFD].FS} - } - - return preopens, nil + return } diff --git a/internal/sys/fs_test.go b/internal/sys/fs_test.go index 54046f21..6e3cfc40 100644 --- a/internal/sys/fs_test.go +++ b/internal/sys/fs_test.go @@ -3,25 +3,62 @@ package sys import ( "context" "errors" - "io/fs" - "path" "testing" + testfs "github.com/tetratelabs/wazero/internal/testing/fs" "github.com/tetratelabs/wazero/internal/testing/require" ) // testCtx is an arbitrary, non-default context. Non-nil also prevents linter errors. var testCtx = context.WithValue(context.Background(), struct{}{}, "arbitrary") -func TestContext_Close(t *testing.T) { - pathName := "test" - file := &testFile{} +func TestEmptyFS(t *testing.T) { + testFS := EmptyFS - fsc := NewFSContext(map[uint32]*FileEntry{ - 3: {Path: "."}, - 4: {Path: path.Join(".", pathName), File: file}, + t.Run("validates path", func(t *testing.T) { + f, err := testFS.Open("/foo.txt") + require.Nil(t, f) + require.EqualError(t, err, "open /foo.txt: invalid argument") }) + t.Run("path not found", func(t *testing.T) { + f, err := testFS.Open("foo.txt") + require.Nil(t, f) + require.EqualError(t, err, "open foo.txt: file does not exist") + }) +} + +func TestEmptyFSContext(t *testing.T) { + testFS := emptyFSContext + expected := &FSContext{ + fs: EmptyFS, + openedFiles: map[uint32]*FileEntry{}, + lastFD: 2, + } + + t.Run("OpenFile doesn't affect state", func(t *testing.T) { + fd, err := testFS.OpenFile(testCtx, "foo.txt") + require.Zero(t, fd) + require.EqualError(t, err, "open foo.txt: file does not exist") + + // Ensure this didn't modify state + require.Equal(t, expected, testFS) + }) + + t.Run("Close doesn't affect state", func(t *testing.T) { + err := testFS.Close(testCtx) + require.NoError(t, err) + + // Ensure this didn't modify state + require.Equal(t, expected, testFS) + }) +} + +func TestContext_Close(t *testing.T) { + fsc := NewFSContext(testfs.FS{"foo": &testfs.File{}}) + _, err := fsc.OpenFile(testCtx, "/foo") + require.NoError(t, err) + // Verify base case require.True(t, len(fsc.openedFiles) > 0, "fsc.openedFiles was empty") @@ -36,41 +73,13 @@ func TestContext_Close(t *testing.T) { } func TestContext_Close_Error(t *testing.T) { - file := &testFile{errors.New("error closing")} + file := &testfs.File{CloseErr: errors.New("error closing")} + fsc := NewFSContext(testfs.FS{"foo": file}) + _, err := fsc.OpenFile(testCtx, "/foo") + require.NoError(t, err) - fsc := NewFSContext(map[uint32]*FileEntry{ - 3: {Path: ".", File: file}, - 4: {Path: "/", File: file}, - }) require.EqualError(t, fsc.Close(testCtx), "error closing") // Paths should clear even under error require.Zero(t, len(fsc.openedFiles), "expected no opened files") } - -// compile-time check to ensure testFile implements fs.File -var _ fs.File = &testFile{} - -type testFile struct{ closeErr error } - -func (f *testFile) Close() error { return f.closeErr } -func (f *testFile) Stat() (fs.FileInfo, error) { return nil, nil } -func (f *testFile) Read(_ []byte) (int, error) { return 0, nil } -func (f *testFile) Seek(_ int64, _ int) (int64, error) { return 0, nil } - -func TestFSConfig_Clone(t *testing.T) { - fsc := NewFSConfig() - cloned := fsc.Clone() - - fsc.preopens[2] = nil - fsc.preopenPaths["2"] = 2 - - cloned.preopens[1] = nil - cloned.preopenPaths["1"] = 1 - - // Ensure the maps are not shared - require.Equal(t, map[uint32]*FileEntry{2: nil}, fsc.preopens) - require.Equal(t, map[string]uint32{"2": 2}, fsc.preopenPaths) - require.Equal(t, map[uint32]*FileEntry{1: nil}, cloned.preopens) - require.Equal(t, map[string]uint32{"1": 1}, cloned.preopenPaths) -} diff --git a/internal/sys/sys.go b/internal/sys/sys.go index ed0fe7bc..c33651bf 100644 --- a/internal/sys/sys.go +++ b/internal/sys/sys.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "io/fs" "time" "github.com/tetratelabs/wazero/internal/platform" @@ -28,8 +29,7 @@ type Context struct { nanotimeResolution sys.ClockResolution nanosleep *sys.Nanosleep randSource io.Reader - - fs *FSContext + fsc *FSContext } // Args is like os.Args and defaults to nil. @@ -109,7 +109,7 @@ func (c *Context) Nanosleep(ctx context.Context, ns int64) { (*(c.nanosleep))(ctx, ns) } -// FS returns the file system context. +// FS returns a possibly nil file system context. func (c *Context) FS(ctx context.Context) *FSContext { // Override Context when it is passed via context if fsValue := ctx.Value(FSKey{}); fsValue != nil { @@ -119,7 +119,7 @@ func (c *Context) FS(ctx context.Context) *FSContext { } return fsCtx } - return c.fs + return c.fsc } // RandSource is a source of random bytes and defaults to crypto/rand.Reader. @@ -137,19 +137,16 @@ func (eofReader) Read([]byte) (int, error) { return 0, io.EOF } -// DefaultContext returns Context with no values set. -// -// Note: This isn't a constant because Context.openedFiles is currently mutable even when empty. -// TODO: Make it an error to open or close files when no FS was assigned. -func DefaultContext() *Context { - if sysCtx, err := NewContext(0, nil, nil, nil, nil, nil, nil, nil, 0, nil, 0, nil, nil); err != nil { +// DefaultContext returns Context with no values set except a possibly nil fs.FS +func DefaultContext(fs fs.FS) *Context { + if sysCtx, err := NewContext(0, nil, nil, nil, nil, nil, nil, nil, 0, nil, 0, nil, fs); err != nil { panic(fmt.Errorf("BUG: DefaultContext should never error: %w", err)) } else { return sysCtx } } -var _ = DefaultContext() // Force panic on bug. +var _ = DefaultContext(nil) // Force panic on bug. var ns sys.Nanosleep = platform.FakeNanosleep // NewContext is a factory function which helps avoid needing to know defaults or exporting all fields. @@ -163,7 +160,7 @@ func NewContext( walltime *sys.Walltime, walltimeResolution sys.ClockResolution, nanotime *sys.Nanotime, nanotimeResolution sys.ClockResolution, nanosleep *sys.Nanosleep, - openedFiles map[uint32]*FileEntry, + fs fs.FS, ) (sysCtx *Context, err error) { sysCtx = &Context{args: args, environ: environ} @@ -227,7 +224,11 @@ func NewContext( sysCtx.nanosleep = &ns } - sysCtx.fs = NewFSContext(openedFiles) + if fs != nil { + sysCtx.fsc = NewFSContext(fs) + } else { + sysCtx.fsc = NewFSContext(EmptyFS) + } return } diff --git a/internal/sys/sys_test.go b/internal/sys/sys_test.go index 3ffa928a..c10bbf85 100644 --- a/internal/sys/sys_test.go +++ b/internal/sys/sys_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/tetratelabs/wazero/internal/platform" + testfs "github.com/tetratelabs/wazero/internal/testing/fs" "github.com/tetratelabs/wazero/internal/testing/require" "github.com/tetratelabs/wazero/sys" ) @@ -24,8 +25,8 @@ func TestDefaultSysContext(t *testing.T) { nil, // randSource nil, 0, // walltime, walltimeResolution nil, 0, // nanotime, nanotimeResolution - nil, // nanosleep - nil, // openedFiles + nil, // nanosleep + testfs.FS{}, // fs ) require.NoError(t, err) @@ -45,7 +46,7 @@ func TestDefaultSysContext(t *testing.T) { require.Equal(t, sys.ClockResolution(1), sysCtx.NanotimeResolution()) require.Equal(t, &ns, sysCtx.nanosleep) require.Equal(t, rand.Reader, sysCtx.RandSource()) - require.Equal(t, NewFSContext(map[uint32]*FileEntry{}), sysCtx.FS(testCtx)) + require.Equal(t, NewFSContext(testfs.FS{}), sysCtx.FS(testCtx)) } func TestNewContext_Args(t *testing.T) { @@ -97,7 +98,7 @@ func TestNewContext_Args(t *testing.T) { nil, 0, // walltime, walltimeResolution nil, 0, // nanotime, nanotimeResolution nil, // nanosleep - nil, // openedFiles + nil, // fs ) if tc.expectedErr == "" { require.Nil(t, err) @@ -159,7 +160,7 @@ func TestNewContext_Environ(t *testing.T) { nil, 0, // walltime, walltimeResolution nil, 0, // nanotime, nanotimeResolution nil, // nanosleep - nil, // openedFiles + nil, // fs ) if tc.expectedErr == "" { require.Nil(t, err) @@ -207,7 +208,7 @@ func TestNewContext_Walltime(t *testing.T) { tc.time, tc.resolution, // walltime, walltimeResolution nil, 0, // nanotime, nanotimeResolution nil, // nanosleep - nil, // openedFiles + nil, // fs ) if tc.expectedErr == "" { require.Nil(t, err) @@ -255,7 +256,7 @@ func TestNewContext_Nanotime(t *testing.T) { nil, 0, // nanotime, nanotimeResolution tc.time, tc.resolution, // nanotime, nanotimeResolution nil, // nanosleep - nil, // openedFiles + nil, // fs ) if tc.expectedErr == "" { require.Nil(t, err) @@ -313,7 +314,7 @@ func TestNewContext_Nanosleep(t *testing.T) { nil, 0, // Nanosleep, NanosleepResolution nil, 0, // Nanosleep, NanosleepResolution &aNs, // nanosleep - nil, // openedFiles + nil, // fs ) require.Nil(t, err) require.Equal(t, &aNs, sysCtx.nanosleep) diff --git a/internal/testing/fs/fs.go b/internal/testing/fs/fs.go new file mode 100644 index 00000000..c169b0f1 --- /dev/null +++ b/internal/testing/fs/fs.go @@ -0,0 +1,31 @@ +package testfs + +import "io/fs" + +// compile-time check to ensure File implements fs.File +var _ fs.File = &File{} + +// compile-time check to ensure FS implements fs.FS +var _ fs.FS = &FS{} + +// FS emulates fs.FS. Note: the path (map key) cannot begin with "/"! +type FS map[string]*File + +// Open implements the same method as documented on fs.FS. +func (f FS) Open(name string) (fs.File, error) { + if !fs.ValidPath(name) { + return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrInvalid} + } + if file, ok := f[name]; !ok { + return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist} + } else { + return file, nil + } +} + +type File struct{ CloseErr error } + +func (f *File) Close() error { return f.CloseErr } +func (f *File) Stat() (fs.FileInfo, error) { return nil, nil } +func (f *File) Read(_ []byte) (int, error) { return 0, nil } +func (f *File) Seek(_ int64, _ int) (int64, error) { return 0, nil } diff --git a/internal/testing/fs/fs_test.go b/internal/testing/fs/fs_test.go new file mode 100644 index 00000000..d1261b16 --- /dev/null +++ b/internal/testing/fs/fs_test.go @@ -0,0 +1,38 @@ +package testfs + +import ( + "io/fs" + "testing" + + "github.com/tetratelabs/wazero/internal/testing/require" +) + +func TestFS(t *testing.T) { + testFS := &FS{} + + t.Run("validates path", func(t *testing.T) { + f, err := testFS.Open("/foo.txt") + require.Nil(t, f) + require.EqualError(t, err, "open /foo.txt: invalid argument") + }) + + t.Run("path not found", func(t *testing.T) { + f, err := testFS.Open("foo.txt") + require.Nil(t, f) + require.EqualError(t, err, "open foo.txt: file does not exist") + }) + + (*testFS)["foo.txt"] = &File{} + f, err := testFS.Open("foo.txt") + require.NoError(t, err) + require.Equal(t, f, &File{}) +} + +func TestFile(t *testing.T) { + f := &File{CloseErr: fs.ErrClosed} + + t.Run("returns close error", func(t *testing.T) { + err := f.Close() + require.Equal(t, fs.ErrClosed, err) + }) +} diff --git a/internal/testing/require/require.go b/internal/testing/require/require.go index 2d26f2f0..d1454eb3 100644 --- a/internal/testing/require/require.go +++ b/internal/testing/require/require.go @@ -63,7 +63,8 @@ func Equal(t TestingT, expected, actual interface{}, formatWithArgs ...interface // Inline the comparison if the types are likely small: if expectString { - fail(t, fmt.Sprintf("expected %q, but was %q", expected, actual), "", formatWithArgs...) + // Don't use %q as it escapes newlines! + fail(t, fmt.Sprintf("expected \"%s\", but was \"%s\"", expected, actual), "", formatWithArgs...) return } else if et.Kind() < reflect.Array { fail(t, fmt.Sprintf("expected %v, but was %v", expected, actual), "", formatWithArgs...) diff --git a/internal/wasm/call_context.go b/internal/wasm/call_context.go index a244b8f8..b14a6680 100644 --- a/internal/wasm/call_context.go +++ b/internal/wasm/call_context.go @@ -115,10 +115,11 @@ func (m *CallContext) close(ctx context.Context, exitCode uint32) (c bool, err e if !atomic.CompareAndSwapUint64(m.closed, 0, closed) { return false, nil } - if sysCtx := m.Sys; sysCtx != nil { // ex nil if from ModuleBuilder - return true, sysCtx.FS(ctx).Close(ctx) + c = true + if sysCtx := m.Sys; sysCtx != nil { // nil if from ModuleBuilder + err = sysCtx.FS(ctx).Close(ctx) } - return true, nil + return } // Memory implements the same method as documented on api.Module. diff --git a/internal/wasm/call_context_test.go b/internal/wasm/call_context_test.go index a9b977c6..ca95e199 100644 --- a/internal/wasm/call_context_test.go +++ b/internal/wasm/call_context_test.go @@ -4,10 +4,10 @@ import ( "context" "errors" "fmt" - "io/fs" "testing" "github.com/tetratelabs/wazero/internal/sys" + testfs "github.com/tetratelabs/wazero/internal/testing/fs" "github.com/tetratelabs/wazero/internal/testing/require" ) @@ -143,24 +143,25 @@ func TestCallContext_Close(t *testing.T) { } t.Run("calls Context.Close()", func(t *testing.T) { - sysCtx := sys.DefaultContext() + sysCtx := sys.DefaultContext(testfs.FS{"foo": &testfs.File{}}) fsCtx := sysCtx.FS(testCtx) - fsCtx.OpenFile(&sys.FileEntry{Path: "."}) + _, err := fsCtx.OpenFile(testCtx, "/foo") + require.NoError(t, err) m, err := s.Instantiate(context.Background(), ns, &Module{}, t.Name(), sysCtx, nil) require.NoError(t, err) // We use side effects to determine if Close in fact called Context.Close (without repeating sys_test.go). // One side effect of Context.Close is that it clears the openedFiles map. Verify our base case. - _, ok := fsCtx.OpenedFile(3) + _, ok := fsCtx.OpenedFile(testCtx, 3) require.True(t, ok, "sysCtx.openedFiles was empty") // Closing should not err. require.NoError(t, m.Close(testCtx)) // Verify our intended side-effect - _, ok = fsCtx.OpenedFile(3) + _, ok = fsCtx.OpenedFile(testCtx, 3) require.False(t, ok, "expected no opened files") // Verify no error closing again. @@ -169,10 +170,12 @@ func TestCallContext_Close(t *testing.T) { t.Run("error closing", func(t *testing.T) { // Right now, the only way to err closing the sys context is if a File.Close erred. - sysCtx := sys.DefaultContext() + testFS := testfs.FS{"foo": &testfs.File{CloseErr: errors.New("error closing")}} + sysCtx := sys.DefaultContext(testFS) fsCtx := sysCtx.FS(testCtx) - fsCtx.OpenFile(&sys.FileEntry{Path: ".", File: &testFile{errors.New("error closing")}}) + _, err := fsCtx.OpenFile(testCtx, "/foo") + require.NoError(t, err) m, err := s.Instantiate(context.Background(), ns, &Module{}, t.Name(), sysCtx, nil) require.NoError(t, err) @@ -180,17 +183,7 @@ func TestCallContext_Close(t *testing.T) { require.EqualError(t, m.Close(testCtx), "error closing") // Verify our intended side-effect - _, ok := fsCtx.OpenedFile(3) + _, ok := fsCtx.OpenedFile(testCtx, 3) require.False(t, ok, "expected no opened files") }) } - -// compile-time check to ensure testFile implements fs.File -var _ fs.File = &testFile{} - -type testFile struct{ closeErr error } - -func (f *testFile) Close() error { return f.closeErr } -func (f *testFile) Stat() (fs.FileInfo, error) { return nil, nil } -func (f *testFile) Read(_ []byte) (int, error) { return 0, nil } -func (f *testFile) Seek(_ int64, _ int) (int64, error) { return 0, nil } diff --git a/internal/wasm/namespace_test.go b/internal/wasm/namespace_test.go index 788addf1..2d8f1097 100644 --- a/internal/wasm/namespace_test.go +++ b/internal/wasm/namespace_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/tetratelabs/wazero/internal/sys" + testfs "github.com/tetratelabs/wazero/internal/testing/fs" "github.com/tetratelabs/wazero/internal/testing/require" ) @@ -162,15 +163,18 @@ func TestNamespace_CloseWithExitCode(t *testing.T) { } t.Run("error closing", func(t *testing.T) { - sysCtx := sys.DefaultContext() + // Right now, the only way to err closing the sys context is if a File.Close erred. + testFS := testfs.FS{"foo": &testfs.File{CloseErr: errors.New("error closing")}} + sysCtx := sys.DefaultContext(testFS) fsCtx := sysCtx.FS(testCtx) - fsCtx.OpenFile(&sys.FileEntry{Path: ".", File: &testFile{errors.New("error closing")}}) + _, err := fsCtx.OpenFile(testCtx, "/foo") + require.NoError(t, err) ns, m1, m2 := newTestNamespace() m1.CallCtx.Sys = sysCtx // This should err, but both should close - err := ns.CloseWithExitCode(testCtx, 2) + err = ns.CloseWithExitCode(testCtx, 2) require.EqualError(t, err, "error closing") // Both modules were closed diff --git a/internal/wasm/store_test.go b/internal/wasm/store_test.go index 7db78348..0e1a99f2 100644 --- a/internal/wasm/store_test.go +++ b/internal/wasm/store_test.go @@ -100,7 +100,7 @@ func TestStore_Instantiate(t *testing.T) { ) require.NoError(t, err) - sysCtx := sys.DefaultContext() + sysCtx := sys.DefaultContext(nil) mod, err := s.Instantiate(testCtx, ns, m, "", sysCtx, nil) require.NoError(t, err) defer mod.Close(testCtx) @@ -211,7 +211,7 @@ func TestStore_hammer(t *testing.T) { N = 100 } hammer.NewHammer(t, P, N).Run(func(name string) { - mod, instantiateErr := s.Instantiate(testCtx, ns, importingModule, name, sys.DefaultContext(), nil) + mod, instantiateErr := s.Instantiate(testCtx, ns, importingModule, name, sys.DefaultContext(nil), nil) require.NoError(t, instantiateErr) require.NoError(t, mod.Close(testCtx)) }, nil) diff --git a/wasi_snapshot_preview1/wasi.go b/wasi_snapshot_preview1/wasi.go index 890a2451..c33469eb 100644 --- a/wasi_snapshot_preview1/wasi.go +++ b/wasi_snapshot_preview1/wasi.go @@ -20,7 +20,6 @@ import ( "fmt" "io" "io/fs" - "path" "github.com/tetratelabs/wazero" "github.com/tetratelabs/wazero/api" @@ -48,12 +47,12 @@ type Builder interface { // Compile compiles the ModuleName module that can instantiated in any namespace (wazero.Namespace). // - // Note: This has the same effect as the same function name on wazero.ModuleBuilder. + // Note: This has the same effect as the same function on wazero.ModuleBuilder. Compile(context.Context, wazero.CompileConfig) (wazero.CompiledModule, error) // Instantiate instantiates the ModuleName module into the provided namespace. // - // Note: This has the same effect as the same function name on wazero.ModuleBuilder. + // Note: This has the same effect as the same function on wazero.ModuleBuilder. Instantiate(context.Context, wazero.Namespace) (api.Closer, error) } @@ -677,7 +676,7 @@ func (a *wasi) FdAllocate(ctx context.Context, mod api.Module, fd uint32, offset // See https://github.com/WebAssembly/WASI/blob/main/phases/snapshot/docs.md#fd_close // See https://linux.die.net/man/3/close func (a *wasi) FdClose(ctx context.Context, mod api.Module, fd uint32) Errno { - if ok, err := getSysCtx(mod).FS(ctx).CloseFile(fd); err != nil { + if ok, err := getSysCtx(mod).FS(ctx).CloseFile(ctx, fd); err != nil { return ErrnoIo } else if !ok { return ErrnoBadf @@ -724,7 +723,7 @@ func (a *wasi) FdDatasync(ctx context.Context, mod api.Module, fd uint32) Errno // See https://github.com/WebAssembly/WASI/blob/main/phases/snapshot/docs.md#fd_fdstat_get // See https://linux.die.net/man/3/fsync func (a *wasi) FdFdstatGet(ctx context.Context, mod api.Module, fd uint32, resultStat uint32) Errno { - if _, ok := getSysCtx(mod).FS(ctx).OpenedFile(fd); !ok { + if _, ok := getSysCtx(mod).FS(ctx).OpenedFile(ctx, fd); !ok { return ErrnoBadf } return ErrnoSuccess @@ -758,7 +757,7 @@ func (a *wasi) FdFdstatGet(ctx context.Context, mod api.Module, fd uint32, resul // See https://github.com/WebAssembly/WASI/blob/snapshot-01/phases/snapshot/docs.md#prestat // See https://github.com/WebAssembly/WASI/blob/main/phases/snapshot/docs.md#fd_prestat_get func (a *wasi) FdPrestatGet(ctx context.Context, mod api.Module, fd uint32, resultPrestat uint32) Errno { - entry, ok := getSysCtx(mod).FS(ctx).OpenedFile(fd) + entry, ok := getSysCtx(mod).FS(ctx).OpenedFile(ctx, fd) if !ok { return ErrnoBadf } @@ -831,7 +830,7 @@ func (a *wasi) FdPread(ctx context.Context, mod api.Module, fd, iovs, iovsCount // See FdPrestatGet // See https://github.com/WebAssembly/WASI/blob/snapshot-01/phases/snapshot/docs.md#fd_prestat_dir_name func (a *wasi) FdPrestatDirName(ctx context.Context, mod api.Module, fd uint32, pathPtr uint32, pathLen uint32) Errno { - f, ok := getSysCtx(mod).FS(ctx).OpenedFile(fd) + f, ok := getSysCtx(mod).FS(ctx).OpenedFile(ctx, fd) if !ok { return ErrnoBadf } @@ -975,7 +974,7 @@ func (a *wasi) FdRenumber(ctx context.Context, mod api.Module, fd, to uint32) Er func (a *wasi) FdSeek(ctx context.Context, mod api.Module, fd uint32, offset uint64, whence uint32, resultNewoffset uint32) Errno { var seeker io.Seeker // Check to see if the file descriptor is available - if f, ok := getSysCtx(mod).FS(ctx).OpenedFile(fd); !ok || f.File == nil { + if f, ok := getSysCtx(mod).FS(ctx).OpenedFile(ctx, fd); !ok || f.File == nil { return ErrnoBadf // fs.FS doesn't declare io.Seeker, but implementations such as os.File implement it. } else if seeker, ok = f.File.(io.Seeker); !ok { @@ -1094,7 +1093,7 @@ func fdReader(ctx context.Context, mod api.Module, fd uint32) io.Reader { sysCtx := getSysCtx(mod) if fd == fdStdin { return sysCtx.Stdin() - } else if f, ok := sysCtx.FS(ctx).OpenedFile(fd); !ok || f.File == nil { + } else if f, ok := sysCtx.FS(ctx).OpenedFile(ctx, fd); !ok { return nil } else { return f.File @@ -1111,9 +1110,10 @@ func fdWriter(ctx context.Context, mod api.Module, fd uint32) io.Writer { return sysCtx.Stderr() default: // Check to see if the file descriptor is available - if f, ok := sysCtx.FS(ctx).OpenedFile(fd); !ok || f.File == nil { + if f, ok := sysCtx.FS(ctx).OpenedFile(ctx, fd); !ok || f.File == nil { return nil - // fs.FS doesn't declare io.Writer, but implementations such as os.File implement it. + // fs.FS doesn't declare io.Writer, but implementations such as + // os.File implement it. } else if writer, ok := f.File.(io.Writer); !ok { return nil } else { @@ -1189,9 +1189,9 @@ func (a *wasi) PathLink(ctx context.Context, mod api.Module, oldFd, oldFlags, ol // See https://linux.die.net/man/3/openat func (a *wasi) PathOpen(ctx context.Context, mod api.Module, fd, dirflags, pathPtr, pathLen, oflags uint32, fsRightsBase, fsRightsInheriting uint64, fdflags, resultOpenedFd uint32) (errno Errno) { - fsc := getSysCtx(mod).FS(ctx) - dir, ok := fsc.OpenedFile(fd) - if !ok || dir.FS == nil { + sysCtx := getSysCtx(mod) + fsc := sysCtx.FS(ctx) + if _, ok := fsc.OpenedFile(ctx, fd); !ok { return ErrnoBadf } @@ -1200,19 +1200,17 @@ func (a *wasi) PathOpen(ctx context.Context, mod api.Module, fd, dirflags, pathP return ErrnoFault } - // TODO: Consider dirflags and oflags. Also, allow non-read-only open based on config about the mount. - // Ex. allow os.O_RDONLY, os.O_WRONLY, or os.O_RDWR either by config flag or pattern on filename - // See #390 - entry, errno := openFileEntry(dir.FS, path.Join(dir.Path, string(b))) - if errno != ErrnoSuccess { - return errno - } - - if newFD, ok := fsc.OpenFile(entry); !ok { - _ = entry.File.Close() - return ErrnoIo + if newFD, err := fsc.OpenFile(ctx, string(b)); err != nil { + switch { + case errors.Is(err, fs.ErrNotExist): + return ErrnoNoent + case errors.Is(err, fs.ErrExist): + return ErrnoExist + default: + return ErrnoIo + } } else if !mod.Memory().WriteUint32Le(ctx, resultOpenedFd, newFD) { - _ = entry.File.Close() + _, _ = fsc.CloseFile(ctx, newFD) return ErrnoFault } return ErrnoSuccess @@ -1320,25 +1318,6 @@ func getSysCtx(mod api.Module) *sys.Context { } } -func openFileEntry(rootFS fs.FS, pathName string) (*sys.FileEntry, Errno) { - f, err := rootFS.Open(pathName) - if err != nil { - switch { - case errors.Is(err, fs.ErrNotExist): - return nil, ErrnoNoent - case errors.Is(err, fs.ErrExist): - return nil, ErrnoExist - default: - return nil, ErrnoIo - } - } - - // TODO: verify if oflags is a directory and fail with ErrnoNotdir if not - // See https://github.com/WebAssembly/WASI/blob/snapshot-01/phases/snapshot/docs.md#-oflags-flagsu16 - - return &sys.FileEntry{Path: pathName, FS: rootFS, File: f}, ErrnoSuccess -} - func writeOffsetsAndNullTerminatedValues(ctx context.Context, mem api.Memory, values []string, offsets, bytes uint32) Errno { for _, value := range values { // Write current offset and advance it. diff --git a/wasi_snapshot_preview1/wasi_test.go b/wasi_snapshot_preview1/wasi_test.go index 707a1042..069243b7 100644 --- a/wasi_snapshot_preview1/wasi_test.go +++ b/wasi_snapshot_preview1/wasi_test.go @@ -442,23 +442,25 @@ func Test_FdAllocate(t *testing.T) { } func Test_FdClose(t *testing.T) { - fdToClose := uint32(3) // arbitrary fd - fdToKeep := uint32(4) // another arbitrary fd + fdToClose := uint32(4) // arbitrary fd + fdToKeep := uint32(5) // another arbitrary fd setupFD := func() (api.Module, api.Function, *wasi) { // fd_close needs to close an open file descriptor. Open two files so that we can tell which is closed. path1, path2 := "a", "b" - testFs := fstest.MapFS{path1: {Data: make([]byte, 0)}, path2: {Data: make([]byte, 0)}} - entry1, errno := openFileEntry(testFs, path1) - require.Zero(t, errno, ErrnoName(errno)) - entry2, errno := openFileEntry(testFs, path2) - require.Zero(t, errno, ErrnoName(errno)) - sysCtx, err := newSysContext(nil, nil, map[uint32]*internalsys.FileEntry{ - fdToClose: entry1, - fdToKeep: entry2, - }) + testFS := fstest.MapFS{path1: {Data: make([]byte, 0)}, path2: {Data: make([]byte, 0)}} + sysCtx, err := newSysContext(nil, nil, testFS) require.NoError(t, err) + fsc := sysCtx.FS(testCtx) + + fd, err := fsc.OpenFile(testCtx, path1) + require.NoError(t, err) + require.Equal(t, fdToClose, fd) + + fd, err = fsc.OpenFile(testCtx, path2) + require.NoError(t, err) + require.Equal(t, fdToKeep, fd) mod, fn := instantiateModule(testCtx, t, functionFdClose, importFdClose, sysCtx) return mod, fn, a @@ -467,11 +469,11 @@ func Test_FdClose(t *testing.T) { verify := func(mod api.Module) { // Verify fdToClose is closed and removed from the opened FDs. fsc := getSysCtx(mod).FS(testCtx) - _, ok := fsc.OpenedFile(fdToClose) + _, ok := fsc.OpenedFile(testCtx, fdToClose) require.False(t, ok) // Verify fdToKeep is not closed - _, ok = fsc.OpenedFile(fdToKeep) + _, ok = fsc.OpenedFile(testCtx, fdToKeep) require.True(t, ok) } @@ -633,11 +635,8 @@ func Test_FdPread(t *testing.T) { } func Test_FdPrestatGet(t *testing.T) { - fd := uint32(3) // arbitrary fd after 0, 1, and 2, that are stdin/out/err - pathName := "/tmp" - sysCtx, err := newSysContext(nil, nil, map[uint32]*internalsys.FileEntry{fd: {Path: pathName}}) - require.NoError(t, err) + sysCtx, fd := requireOpenDir(t, pathName) mod, fn := instantiateModule(testCtx, t, functionFdPrestatGet, importFdPrestatGet, sysCtx) defer mod.Close(testCtx) @@ -677,12 +676,21 @@ func Test_FdPrestatGet(t *testing.T) { }) } +func requireOpenDir(t *testing.T, pathName string) (*internalsys.Context, uint32) { + testFS := fstest.MapFS{pathName[1:]: {Mode: fs.ModeDir}} + sysCtx, err := newSysContext(nil, nil, testFS) + require.NoError(t, err) + fsc := sysCtx.FS(testCtx) + fd, err := fsc.OpenFile(testCtx, pathName) + require.NoError(t, err) + return sysCtx, fd +} + func Test_FdPrestatGet_Errors(t *testing.T) { - fd := uint32(3) // fd 3 will be opened for the "/tmp" directory after 0, 1, and 2, that are stdin/out/err validAddress := uint32(0) // Arbitrary valid address as arguments to fd_prestat_get. We chose 0 here. - sysCtx, err := newSysContext(nil, nil, map[uint32]*internalsys.FileEntry{fd: {Path: "/tmp"}}) - require.NoError(t, err) + pathName := "/tmp" + sysCtx, fd := requireOpenDir(t, pathName) mod, _ := instantiateModule(testCtx, t, functionFdPrestatGet, importFdPrestatGet, sysCtx) defer mod.Close(testCtx) @@ -721,10 +729,8 @@ func Test_FdPrestatGet_Errors(t *testing.T) { } func Test_FdPrestatDirName(t *testing.T) { - fd := uint32(3) // arbitrary fd after 0, 1, and 2, that are stdin/out/err - - sysCtx, err := newSysContext(nil, nil, map[uint32]*internalsys.FileEntry{fd: {Path: "/tmp"}}) - require.NoError(t, err) + pathName := "/tmp" + sysCtx, fd := requireOpenDir(t, pathName) mod, fn := instantiateModule(testCtx, t, functionFdPrestatDirName, importFdPrestatDirName, sysCtx) defer mod.Close(testCtx) @@ -763,9 +769,8 @@ func Test_FdPrestatDirName(t *testing.T) { } func Test_FdPrestatDirName_Errors(t *testing.T) { - fd := uint32(3) // arbitrary fd after 0, 1, and 2, that are stdin/out/err - sysCtx, err := newSysContext(nil, nil, map[uint32]*internalsys.FileEntry{fd: {Path: "/tmp"}}) - require.NoError(t, err) + pathName := "/tmp" + sysCtx, fd := requireOpenDir(t, pathName) mod, _ := instantiateModule(testCtx, t, functionFdPrestatDirName, importFdPrestatDirName, sysCtx) defer mod.Close(testCtx) @@ -841,7 +846,7 @@ func Test_FdPwrite(t *testing.T) { } func Test_FdRead(t *testing.T) { - fd := uint32(3) // arbitrary fd after 0, 1, and 2, that are stdin/out/err + var fd uint32 iovs := uint32(1) // arbitrary offset initialMemory := []byte{ '?', // `iovs` is after this @@ -885,10 +890,12 @@ func Test_FdRead(t *testing.T) { tc := tt t.Run(tc.name, func(t *testing.T) { // Create a fresh file to read the contents from - file, testFS := createFile(t, "test_path", []byte("wazero")) - sysCtx, err := newSysContext(nil, nil, map[uint32]*internalsys.FileEntry{ - fd: {Path: "test_path", FS: testFS, File: file}, - }) + _, testFS := createFile(t, "test_path", []byte("wazero")) + sysCtx, err := newSysContext(nil, nil, testFS) + require.NoError(t, err) + fsc := sysCtx.FS(testCtx) + + fd, err = fsc.OpenFile(testCtx, "test_path") require.NoError(t, err) mod, fn := instantiateModule(testCtx, t, functionFdRead, importFdRead, sysCtx) @@ -910,12 +917,12 @@ func Test_FdRead(t *testing.T) { } func Test_FdRead_Errors(t *testing.T) { - validFD := uint32(3) // arbitrary valid fd after 0, 1, and 2, that are stdin/out/err - file, testFS := createFile(t, "test_path", []byte{}) // file with empty contents + _, testFS := createFile(t, "test_path", []byte{}) // file with empty contents + sysCtx, err := newSysContext(nil, nil, testFS) + require.NoError(t, err) + fsc := sysCtx.FS(testCtx) - sysCtx, err := newSysContext(nil, nil, map[uint32]*internalsys.FileEntry{ - validFD: {Path: "test_path", FS: testFS, File: file}, - }) + validFD, err := fsc.OpenFile(testCtx, "test_path") require.NoError(t, err) mod, _ := instantiateModule(testCtx, t, functionFdRead, importFdRead, sysCtx) @@ -1038,16 +1045,14 @@ func Test_FdRenumber(t *testing.T) { } func Test_FdSeek(t *testing.T) { - fd := uint32(3) // arbitrary fd after 0, 1, and 2, that are stdin/out/err - resultNewoffset := uint32(1) // arbitrary offset in `ctx.Memory` for the new offset value - file, testFS := createFile(t, "test_path", []byte("wazero")) // arbitrary non-empty contents - - sysCtx, err := newSysContext(nil, nil, map[uint32]*internalsys.FileEntry{ - fd: {Path: "test_path", FS: testFS, File: file}, - }) + resultNewoffset := uint32(1) // arbitrary offset in `ctx.Memory` for the new offset value + _, testFS := createFile(t, "test_path", []byte("wazero")) // arbitrary non-empty contents + sysCtx, err := newSysContext(nil, nil, testFS) require.NoError(t, err) fsCtx := sysCtx.FS(testCtx) + fd, err := fsCtx.OpenFile(testCtx, "test_path") + require.NoError(t, err) mod, fn := instantiateModule(testCtx, t, functionFdSeek, importFdSeek, sysCtx) defer mod.Close(testCtx) @@ -1121,7 +1126,7 @@ func Test_FdSeek(t *testing.T) { maskMemory(t, testCtx, mod, len(tc.expectedMemory)) // Since we initialized this file, we know it is a seeker (because it is a MapFile) - f, ok := fsCtx.OpenedFile(fd) + f, ok := fsCtx.OpenedFile(testCtx, fd) require.True(t, ok) seeker := f.File.(io.Seeker) @@ -1147,12 +1152,12 @@ func Test_FdSeek(t *testing.T) { } func Test_FdSeek_Errors(t *testing.T) { - validFD := uint32(3) // arbitrary valid fd after 0, 1, and 2, that are stdin/out/err - file, testFS := createFile(t, "test_path", []byte("wazero")) // arbitrary valid file with non-empty contents + _, testFS := createFile(t, "test_path", []byte("wazero")) // arbitrary non-empty contents + sysCtx, err := newSysContext(nil, nil, testFS) + require.NoError(t, err) - sysCtx, err := newSysContext(nil, nil, map[uint32]*internalsys.FileEntry{ - validFD: {Path: "test_path", FS: testFS, File: file}, - }) + fsCtx := sysCtx.FS(testCtx) + validFD, err := fsCtx.OpenFile(testCtx, "test_path") require.NoError(t, err) mod, _ := instantiateModule(testCtx, t, functionFdSeek, importFdSeek, sysCtx) @@ -1193,7 +1198,6 @@ func Test_FdSeek_Errors(t *testing.T) { require.Equal(t, tc.expectedErrno, errno, ErrnoName(errno)) }) } - } // Test_FdSync only tests it is stubbed for GrainLang per #271 @@ -1233,7 +1237,7 @@ func Test_FdTell(t *testing.T) { } func Test_FdWrite(t *testing.T) { - fd := uint32(3) // arbitrary fd after 0, 1, and 2, that are stdin/out/err + fd := uint32(4) iovs := uint32(1) // arbitrary offset initialMemory := []byte{ '?', // `iovs` is after this @@ -1280,11 +1284,7 @@ func Test_FdWrite(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Create a fresh file to write the contents to pathName := "test_path" - file, testFS := createWriteableFile(t, tmpDir, pathName, []byte{}) - sysCtx, err := newSysContext(nil, nil, map[uint32]*internalsys.FileEntry{ - fd: {Path: pathName, FS: testFS, File: file}, - }) - require.NoError(t, err) + sysCtx := newContextWithWritableFile(t, tmpDir, pathName) mod, fn := instantiateModule(testCtx, t, functionFdWrite, importFdWrite, sysCtx) defer mod.Close(testCtx) @@ -1310,16 +1310,10 @@ func Test_FdWrite(t *testing.T) { } func Test_FdWrite_Errors(t *testing.T) { - validFD := uint32(3) // arbitrary valid fd after 0, 1, and 2, that are stdin/out/err - tmpDir := t.TempDir() // open before loop to ensure no locking problems. pathName := "test_path" - file, testFS := createWriteableFile(t, tmpDir, pathName, []byte{}) - - sysCtx, err := newSysContext(nil, nil, map[uint32]*internalsys.FileEntry{ - validFD: {Path: pathName, FS: testFS, File: file}, - }) - require.NoError(t, err) + validFD := uint32(4) + sysCtx := newContextWithWritableFile(t, tmpDir, pathName) mod, _ := instantiateModule(testCtx, t, functionFdWrite, importFdWrite, sysCtx) defer mod.Close(testCtx) @@ -1461,7 +1455,6 @@ func Test_PathLink(t *testing.T) { func Test_PathOpen(t *testing.T) { type pathOpenArgs struct { - fd uint32 dirflags uint32 pathPtr uint32 pathLen uint32 @@ -1472,11 +1465,13 @@ func Test_PathOpen(t *testing.T) { resultOpenedFd uint32 } - setup := func(workdirFD uint32, pathName string) (api.Module, api.Function, pathOpenArgs, []byte, uint32) { + rootFD := uint32(3) // after 0, 1, and 2, that are stdin/out/err + expectedFD := rootFD + 1 + + setup := func(pathName string) (api.Module, api.Function, pathOpenArgs, []byte) { // Setup the initial memory to include the path name starting at an offset. initialMemory := append([]byte{'?'}, pathName...) - expectedFD := workdirFD + 1 expectedMemory := append( initialMemory, '?', // `resultOpenedFd` is after this @@ -1485,7 +1480,6 @@ func Test_PathOpen(t *testing.T) { ) args := pathOpenArgs{ - fd: workdirFD, dirflags: 0, pathPtr: 1, pathLen: uint32(len(pathName)), @@ -1497,18 +1491,17 @@ func Test_PathOpen(t *testing.T) { } testFS := fstest.MapFS{pathName: &fstest.MapFile{Mode: os.ModeDir}} - sysCtx, err := newSysContext(nil, nil, map[uint32]*internalsys.FileEntry{ - workdirFD: {Path: ".", FS: testFS}, - }) + sysCtx, err := newSysContext(nil, nil, testFS) require.NoError(t, err) + mod, fn := instantiateModule(testCtx, t, functionPathOpen, importPathOpen, sysCtx) maskMemory(t, testCtx, mod, len(expectedMemory)) ok := mod.Memory().Write(testCtx, 0, initialMemory) require.True(t, ok) - return mod, fn, args, expectedMemory, expectedFD + return mod, fn, args, expectedMemory } - verify := func(ctx context.Context, errno Errno, mod api.Module, pathName string, expectedMemory []byte, expectedFD uint32) { + verify := func(ctx context.Context, errno Errno, mod api.Module, pathName string, expectedMemory []byte) { require.Zero(t, errno, ErrnoName(errno)) actual, ok := mod.Memory().Read(testCtx, 0, uint32(len(expectedMemory))) @@ -1517,54 +1510,48 @@ func Test_PathOpen(t *testing.T) { // verify the file was actually opened fsc := getSysCtx(mod).FS(ctx) - f, ok := fsc.OpenedFile(expectedFD) + f, ok := fsc.OpenedFile(testCtx, expectedFD) require.True(t, ok) require.Equal(t, pathName, f.Path) } t.Run("wasi.PathOpen", func(t *testing.T) { - workdirFD := uint32(3) // arbitrary fd after 0, 1, and 2, that are stdin/out/err pathName := "wazero" - mod, _, args, expectedMemory, expectedFD := setup(workdirFD, pathName) - errno := a.PathOpen(testCtx, mod, args.fd, args.dirflags, args.pathPtr, args.pathLen, args.oflags, + mod, _, args, expectedMemory := setup(pathName) + errno := a.PathOpen(testCtx, mod, rootFD, args.dirflags, args.pathPtr, args.pathLen, args.oflags, args.fsRightsBase, args.fsRightsInheriting, args.fdflags, args.resultOpenedFd) - verify(testCtx, errno, mod, pathName, expectedMemory, expectedFD) + verify(testCtx, errno, mod, pathName, expectedMemory) }) t.Run(functionPathOpen, func(t *testing.T) { - workdirFD := uint32(3) // arbitrary fd after 0, 1, and 2, that are stdin/out/err pathName := "wazero" - mod, fn, args, expectedMemory, expectedFD := setup(workdirFD, pathName) - results, err := fn.Call(testCtx, uint64(args.fd), uint64(args.dirflags), uint64(args.pathPtr), uint64(args.pathLen), + mod, fn, args, expectedMemory := setup(pathName) + results, err := fn.Call(testCtx, uint64(rootFD), uint64(args.dirflags), uint64(args.pathPtr), uint64(args.pathLen), uint64(args.oflags), args.fsRightsBase, args.fsRightsInheriting, uint64(args.fdflags), uint64(args.resultOpenedFd)) require.NoError(t, err) errno := Errno(results[0]) - verify(testCtx, errno, mod, pathName, expectedMemory, expectedFD) + verify(testCtx, errno, mod, pathName, expectedMemory) }) t.Run("wasi.PathOpen.WithFS", func(t *testing.T) { - workdirFD := uint32(100) // dummy fd as it is not used pathName := "wazero" // The filesystem initialized in setup() is not used as it will be overridden. - mod, _, args, expectedMemory, _ := setup(workdirFD, pathName) + mod, _, args, expectedMemory := setup(pathName) // Override fs.FS through context - workdirFD = uint32(4) // 3 is '/' and 4 is '.' - expectedFD := workdirFD + 1 expectedMemory[8] = byte(expectedFD) // replace expected memory with expected fd testFS := fstest.MapFS{pathName: &fstest.MapFile{Mode: os.ModeDir}} - ctx, closer, err := experimental.WithFS(testCtx, testFS) - require.NoError(t, err) + ctx, closer := experimental.WithFS(testCtx, testFS) defer closer.Close(ctx) - errno := a.PathOpen(ctx, mod, workdirFD, args.dirflags, args.pathPtr, args.pathLen, args.oflags, + errno := a.PathOpen(ctx, mod, rootFD, args.dirflags, args.pathPtr, args.pathLen, args.oflags, args.fsRightsBase, args.fsRightsInheriting, args.fdflags, args.resultOpenedFd) require.Zero(t, errno, ErrnoName(errno)) - verify(ctx, errno, mod, pathName, expectedMemory, expectedFD) + verify(ctx, errno, mod, pathName, expectedMemory) }) } @@ -1572,10 +1559,7 @@ func Test_PathOpen_Errors(t *testing.T) { validFD := uint32(3) // arbitrary valid fd after 0, 1, and 2, that are stdin/out/err pathName := "wazero" testFS := fstest.MapFS{pathName: &fstest.MapFile{Mode: os.ModeDir}} - - sysCtx, err := newSysContext(nil, nil, map[uint32]*internalsys.FileEntry{ - validFD: {Path: ".", FS: testFS}, - }) + sysCtx, err := newSysContext(nil, nil, testFS) require.NoError(t, err) mod, _ := instantiateModule(testCtx, t, functionPathOpen, importPathOpen, sysCtx) @@ -2008,7 +1992,7 @@ func instantiateModule(ctx context.Context, t *testing.T, wasiFunction, wasiImpo return mod, fn } -func newSysContext(args, environ []string, openedFiles map[uint32]*internalsys.FileEntry) (sysCtx *internalsys.Context, err error) { +func newSysContext(args, environ []string, fs fs.FS) (sysCtx *internalsys.Context, err error) { return internalsys.NewContext( math.MaxUint32, args, @@ -2020,7 +2004,7 @@ func newSysContext(args, environ []string, openedFiles map[uint32]*internalsys.F nil, 0, nil, 0, nil, // nanosleep - openedFiles, + fs, ) } @@ -2035,6 +2019,25 @@ func createFile(t *testing.T, pathName string, data []byte) (fs.File, fs.FS) { return f, mapFS } +// newContextWithWritableFile is temporary until we add the ability to open files for writing. +func newContextWithWritableFile(t *testing.T, tmpDir string, pathName string) *internalsys.Context { + writeable, testFS := createWriteableFile(t, tmpDir, pathName, []byte{}) + sysCtx, err := newSysContext(nil, nil, testFS) + require.NoError(t, err) + + fsc := sysCtx.FS(testCtx) + fd, err := fsc.OpenFile(testCtx, pathName) + require.NoError(t, err) + + // Swap the read-only file with a writeable one until #390 + f, ok := fsc.OpenedFile(testCtx, fd) + require.True(t, ok) + f.File.Close() + f.File = writeable + + return sysCtx +} + // createWriteableFile uses real files when io.Writer tests are needed. func createWriteableFile(t *testing.T, tmpDir string, pathName string, data []byte) (fs.File, fs.FS) { require.NotNil(t, data)