Repository: tetratelabs/wazero
Pull request #1166 (2023-02-25 00:48:32)

Readdir: handles io.EOF
This handles EOF even if current and possibly future wasi don't have a way to propagate an EOF signal. This is mainly to match up with go semantics and ensure we don't have any error conditions (by adding tests).


Patch:
diff --git a/imports/wasi_snapshot_preview1/fs.go b/imports/wasi_snapshot_preview1/fs.go
index 0c9b4550..c708d665 100644
--- a/imports/wasi_snapshot_preview1/fs.go
+++ b/imports/wasi_snapshot_preview1/fs.go
@@ -855,10 +855,15 @@ func fdReaddirFn(_ context.Context, mod api.Module, params []uint64) Errno {
 		if errno = ToErrno(err); errno != ErrnoSuccess {
 			return errno
 		}
-		dir.CountRead += uint64(len(l))
-		dirents = append(dirents, l...)
-		// Replace the cache with up to maxDirEntries, starting at cookie.
-		dir.Dirents = dirents
+
+		// Zero length read is possible on an empty directory or on io.EOF,
+		// which coerces to nil in WASI because there's no Errno for it.
+		if len(l) > 0 {
+			dir.CountRead += uint64(len(l))
+			dirents = append(dirents, l...)
+			// Replace the cache with up to maxDirEntries, starting at cookie.
+			dir.Dirents = dirents
+		}
 	}
 
 	// Determine how many dirents we can write, excluding a potentially
diff --git a/internal/gojs/errno.go b/internal/gojs/errno.go
index 6ee5e0fb..d4ba7192 100644
--- a/internal/gojs/errno.go
+++ b/internal/gojs/errno.go
@@ -1,6 +1,7 @@
 package gojs
 
 import (
+	"io"
 	"syscall"
 
 	"github.com/tetratelabs/wazero/internal/platform"
@@ -60,6 +61,9 @@ var (
 //
 // This should match wasi_snapshot_preview1.ToErrno for maintenance ease.
 func ToErrno(err error) *Errno {
+	if err == nil || err == io.EOF {
+		return nil // io.EOF has no value in GOOS=js, and isn't an error.
+	}
 	errno := platform.UnwrapOSError(err)
 
 	switch errno {
diff --git a/internal/platform/dir.go b/internal/platform/dir.go
index dd2152bd..d6d58ec6 100644
--- a/internal/platform/dir.go
+++ b/internal/platform/dir.go
@@ -67,8 +67,15 @@ func (d *Dirent) IsDir() bool {
 // If n > 0, Readdir returns at most n entries or an error.
 // If n <= 0, Readdir returns all remaining entries or an error.
 //
-// Note: The error will be nil or a syscall.Errno. There's no error when the
-// file is closed or removed while open. No error is returned on io.EOF.
+// # Errors
+//
+// Any value of n can result in io.EOF, which is the result of calling this on
+// an already exhausted directory. Otherwise, if the error is non-nil, it is a
+// syscall.Errno.
+//
+// For portability reasons, no error is returned when the file is closed or
+// removed while open.
+// See https://github.com/ziglang/zig/blob/0.10.1/lib/std/fs.zig#L635-L637
 func Readdir(f fs.File, n int) (dirents []*Dirent, err error) {
 	// ^^ case format is to match POSIX and similar to os.File.Readdir
 
@@ -76,11 +83,9 @@ func Readdir(f fs.File, n int) (dirents []*Dirent, err error) {
 	case fs.ReadDirFile:
 		var entries []fs.DirEntry
 		entries, err = f.ReadDir(n)
-		if err == io.EOF {
-			err = nil // not an error
+		if err == io.EOF { // retain io.EOF for the caller
 		} else if err = UnwrapOSError(err); err != nil {
 			// Ignore errors when the file was closed or removed.
-			// See https://github.com/ziglang/zig/blob/0.10.1/lib/std/fs.zig#L635-L637
 			switch err {
 			case syscall.EIO, syscall.EBADF: // closed while open
 				err = nil
diff --git a/internal/wasi_snapshot_preview1/errno.go b/internal/wasi_snapshot_preview1/errno.go
index e19e9a69..3b59a240 100644
--- a/internal/wasi_snapshot_preview1/errno.go
+++ b/internal/wasi_snapshot_preview1/errno.go
@@ -2,6 +2,7 @@ package wasi_snapshot_preview1
 
 import (
 	"fmt"
+	"io"
 	"syscall"
 
 	"github.com/tetratelabs/wazero/internal/platform"
@@ -266,8 +267,8 @@ var errnoToString = [...]string{
 // error codes. For example, wasi-filesystem and GOOS=js don't map to these
 // Errno.
 func ToErrno(err error) Errno {
-	if err == nil {
-		return ErrnoSuccess
+	if err == nil || err == io.EOF {
+		return ErrnoSuccess // io.EOF has no value in WASI, and isn't an error.
 	}
 	errno := platform.UnwrapOSError(err)
 
diff --git a/site/content/community/users.md b/site/content/community/users.md
index c0586e62..026bf476 100644
--- a/site/content/community/users.md
+++ b/site/content/community/users.md
@@ -29,9 +29,10 @@ considering their efforts before starting your own!
 
 ### Libraries
 
-| Name        | Description                          |
-|:------------|--------------------------------------|
-| [go-re2][7] | high performance regular expressions |
+| Name             | Description                                  |
+|:-----------------|----------------------------------------------|
+| [go-re2][7]      | high performance regular expressions         |
+| [go-sqlite3][11] | [SQLite][12] bindings, `database/sql` driver |
 
 ## Updating this list
 
@@ -61,3 +62,7 @@ experience.
 [9]: https://mosn.io/
 
 [10]: https://github.com/proxy-wasm/spec
+
+[11]: https://github.com/ncruces/go-sqlite3
+
+[12]: https://sqlite.org


Test patch:
diff --git a/imports/wasi_snapshot_preview1/fs_test.go b/imports/wasi_snapshot_preview1/fs_test.go
index 9f0a3be0..7ef4fd5c 100644
--- a/imports/wasi_snapshot_preview1/fs_test.go
+++ b/imports/wasi_snapshot_preview1/fs_test.go
@@ -2096,6 +2096,30 @@ func Test_fdReaddir(t *testing.T) {
 				Dirents:   testDirents[2:],
 			},
 		},
+		{
+			name: "read exhausted directory",
+			dir: func() *sys.FileEntry {
+				dir, err := fstest.FS.Open("dir")
+				require.NoError(t, err)
+				_, err = platform.Readdir(dir, 3)
+				require.NoError(t, err)
+
+				return &sys.FileEntry{
+					File: dir,
+					ReadDir: &sys.ReadDir{
+						CountRead: 5,
+						Dirents:   testDirents,
+					},
+				}
+			},
+			bufLen:          300, // length is long enough for third and more
+			cookie:          5,   // d_next after entries.
+			expectedBufused: 0,   // nothing read
+			expectedReadDir: &sys.ReadDir{
+				CountRead: 5,
+				Dirents:   testDirents,
+			},
+		},
 	}
 
 	for _, tt := range tests {
diff --git a/internal/gojs/errno_test.go b/internal/gojs/errno_test.go
index b99c482f..3166f901 100644
--- a/internal/gojs/errno_test.go
+++ b/internal/gojs/errno_test.go
@@ -1,6 +1,7 @@
 package gojs
 
 import (
+	"io"
 	"syscall"
 	"testing"
 )
@@ -11,6 +12,13 @@ func TestToErrno(t *testing.T) {
 		input    error
 		expected *Errno
 	}{
+		{
+			name: "nil is not an error",
+		},
+		{
+			name:  "io.EOF is not an error",
+			input: io.EOF,
+		},
 		{
 			name:     "syscall.EACCES",
 			input:    syscall.EACCES,
diff --git a/internal/platform/dir_test.go b/internal/platform/dir_test.go
index 5b7ec728..72eebf09 100644
--- a/internal/platform/dir_test.go
+++ b/internal/platform/dir_test.go
@@ -1,6 +1,7 @@
 package platform_test
 
 import (
+	"io"
 	"io/fs"
 	"os"
 	"path"
@@ -104,24 +105,20 @@ func TestReaddir(t *testing.T) {
 
 			t.Run("dir", func(t *testing.T) {
 				dirents, err := platform.Readdir(dirF, -1)
-				require.NoError(t, err)
+				require.NoError(t, err) // no io.EOF when -1 is used
 				sort.Slice(dirents, func(i, j int) bool { return dirents[i].Name < dirents[j].Name })
 
-				require.Equal(t, 5, len(dirents))
-				require.Equal(t, "animals.txt", dirents[0].Name)
-				require.Zero(t, dirents[0].Type)
-				require.Equal(t, "dir", dirents[1].Name)
-				require.Equal(t, fs.ModeDir, dirents[1].Type)
-				require.Equal(t, "empty.txt", dirents[2].Name)
-				require.Zero(t, dirents[2].Type)
-				require.Equal(t, "emptydir", dirents[3].Name)
-				require.Equal(t, fs.ModeDir, dirents[3].Type)
-				require.Equal(t, "sub", dirents[4].Name)
-				require.Equal(t, fs.ModeDir, dirents[4].Type)
+				require.Equal(t, []*platform.Dirent{
+					{Name: "animals.txt", Type: 0},
+					{Name: "dir", Type: fs.ModeDir},
+					{Name: "empty.txt", Type: 0},
+					{Name: "emptydir", Type: fs.ModeDir},
+					{Name: "sub", Type: fs.ModeDir},
+				}, dirents)
 
 				// read again even though it is exhausted
 				dirents, err = platform.Readdir(dirF, 100)
-				require.NoError(t, err)
+				require.Equal(t, io.EOF, err)
 				require.Zero(t, len(dirents))
 			})
 
@@ -141,6 +138,37 @@ func TestReaddir(t *testing.T) {
 				require.EqualErrno(t, syscall.ENOTDIR, err)
 			})
 
+			dirF, err = tc.fs.Open("dir")
+			require.NoError(t, err)
+			defer dirF.Close()
+
+			t.Run("partial read", func(t *testing.T) {
+				dirents1, err := platform.Readdir(dirF, 1)
+				require.NoError(t, err)
+				require.Equal(t, 1, len(dirents1))
+
+				dirents2, err := platform.Readdir(dirF, 1)
+				require.NoError(t, err)
+				require.Equal(t, 1, len(dirents2))
+
+				// read exactly the last entry
+				dirents3, err := platform.Readdir(dirF, 1)
+				require.NoError(t, err)
+				require.Equal(t, 1, len(dirents3))
+
+				dirents := []*platform.Dirent{dirents1[0], dirents2[0], dirents3[0]}
+				sort.Slice(dirents, func(i, j int) bool { return dirents[i].Name < dirents[j].Name })
+
+				require.Equal(t, []*platform.Dirent{
+					{Name: "-", Type: 0},
+					{Name: "a-", Type: fs.ModeDir},
+					{Name: "ab-", Type: 0},
+				}, dirents)
+
+				_, err = platform.Readdir(dirF, 1)
+				require.Equal(t, io.EOF, err)
+			})
+
 			subdirF, err := tc.fs.Open("sub")
 			require.NoError(t, err)
 			defer subdirF.Close()
diff --git a/internal/wasi_snapshot_preview1/errno_test.go b/internal/wasi_snapshot_preview1/errno_test.go
index a40c1073..dcedab40 100644
--- a/internal/wasi_snapshot_preview1/errno_test.go
+++ b/internal/wasi_snapshot_preview1/errno_test.go
@@ -1,6 +1,7 @@
 package wasi_snapshot_preview1
 
 import (
+	"io"
 	"syscall"
 	"testing"
 )
@@ -11,6 +12,15 @@ func TestToErrno(t *testing.T) {
 		input    error
 		expected Errno
 	}{
+		{
+			name:     "nil is not an error",
+			expected: ErrnoSuccess,
+		},
+		{
+			name:     "io.EOF is not an error",
+			input:    io.EOF,
+			expected: ErrnoSuccess,
+		},
 		{
 			name:     "syscall.EACCES",
 			input:    syscall.EACCES,