implement wasi-filesystem::readdir and related functions (#45)

* implement `wasi-filesystem::readdir` and related functions

This adds a `directory_list` test and provides the required host implementation.

I've also added a file length check to the `file_read` test, just to cover a bit
more of the API.

Signed-off-by: Joel Dice <joel.dice@fermyon.com>

* fix memory corruption in `fd_readdir` polyfill

We were copying `name.len() * 256` bytes instead of just `name.len()` bytes,
which was overwriting other parts of `State` and causing untold havoc.

Signed-off-by: Joel Dice <joel.dice@fermyon.com>

* check type of entry in `Table::delete`

Signed-off-by: Joel Dice <joel.dice@fermyon.com>

Signed-off-by: Joel Dice <joel.dice@fermyon.com>

host/src/filesystem.rs

@@ -5,10 +5,11 @@ use crate::{wasi_filesystem, HostResult, WasiCtx};
 use std::{
     io::{IoSlice, IoSliceMut},
     ops::BitAnd,
+    sync::Mutex,
     time::SystemTime,
 };
 use wasi_common::{
-    dir::TableDirExt,
+    dir::{ReaddirCursor, ReaddirIterator, TableDirExt},
     file::{FdFlags, FileStream, TableFileExt},
     WasiDir, WasiFile,
 };
@@ -330,21 +331,49 @@ impl wasi_filesystem::WasiFilesystem for WasiCtx {
         &mut self,
         fd: wasi_filesystem::Descriptor,
     ) -> HostResult<wasi_filesystem::DirEntryStream, wasi_filesystem::Errno> {
-        todo!()
+        let iterator = self
+            .table()
+            .get_dir(fd)
+            .map_err(convert)?
+            .readdir(ReaddirCursor::from(0))
+            .await
+            .map_err(convert)?;
+
+        self.table_mut()
+            .push(Box::new(Mutex::new(iterator)))
+            .map_err(convert)
     }
 
     async fn read_dir_entry(
         &mut self,
         stream: wasi_filesystem::DirEntryStream,
     ) -> HostResult<Option<wasi_filesystem::DirEntry>, wasi_filesystem::Errno> {
-        todo!()
+        let entity = self
+            .table()
+            .get::<Mutex<ReaddirIterator>>(stream)
+            .map_err(convert)?
+            .lock()
+            .unwrap()
+            .next()
+            .transpose()
+            .map_err(convert)?;
+
+        Ok(entity.map(|e| wasi_filesystem::DirEntry {
+            ino: Some(e.inode),
+            type_: e.filetype.into(),
+            name: e.name,
+        }))
     }
 
     async fn close_dir_entry_stream(
         &mut self,
-        fd: wasi_filesystem::DirEntryStream,
+        stream: wasi_filesystem::DirEntryStream,
     ) -> anyhow::Result<()> {
-        todo!()
+        self.table_mut()
+            .delete::<Mutex<ReaddirIterator>>(stream)
+            .map_err(convert)?;
+
+        Ok(())
     }
 
     async fn sync(
@@ -469,13 +498,11 @@ impl wasi_filesystem::WasiFilesystem for WasiCtx {
 
     async fn close(&mut self, fd: wasi_filesystem::Descriptor) -> anyhow::Result<()> {
         let table = self.table_mut();
-        if table.is::<Box<dyn WasiFile>>(fd) {
-            let _ = table.delete(fd);
-        } else if table.is::<Box<dyn WasiDir>>(fd) {
-            // TODO: `WasiCtx` no longer keeps track of which directories are preopens, so we currently have no way
-            // of preventing them from being closed.  Is that a problem?
-            let _ = table.delete(fd);
-        } else {
+        // TODO: `WasiCtx` no longer keeps track of which directories are preopens, so we currently have no way
+        // of preventing them from being closed.  Is that a problem?
+        if !(table.delete::<Box<dyn WasiFile>>(fd).is_ok()
+            || table.delete::<Box<dyn WasiDir>>(fd).is_ok())
+        {
             anyhow::bail!("{fd} is neither a file nor a directory");
         }
         Ok(())

host/src/poll.rs

@@ -28,7 +28,7 @@ enum Future {
 #[async_trait::async_trait]
 impl WasiPoll for WasiCtx {
     async fn drop_future(&mut self, future: WasiFuture) -> anyhow::Result<()> {
-        self.table_mut().delete(future);
+        self.table_mut().delete::<Future>(future).map_err(convert)?;
         Ok(())
     }
 
@@ -74,7 +74,9 @@ impl WasiPoll for WasiCtx {
     }
 
     async fn drop_stream(&mut self, stream: WasiStream) -> anyhow::Result<()> {
-        self.table_mut().delete(stream);
+        self.table_mut()
+            .delete::<Box<dyn wasi_common::WasiStream>>(stream)
+            .map_err(convert)?;
         Ok(())
     }
 

host/tests/runtime.rs

@@ -301,3 +301,32 @@ async fn run_exit_panic(mut store: Store<WasiCtx>, wasi: Wasi) -> Result<()> {
     assert!(err.downcast_ref::<wasi_common::I32Exit>().is_none());
     Ok(())
 }
+
+async fn run_directory_list(mut store: Store<WasiCtx>, wasi: Wasi) -> Result<()> {
+    let dir = tempfile::tempdir()?;
+
+    std::fs::File::create(dir.path().join("foo.txt"))?;
+    std::fs::File::create(dir.path().join("bar.txt"))?;
+    std::fs::File::create(dir.path().join("baz.txt"))?;
+    std::fs::create_dir(dir.path().join("sub"))?;
+    std::fs::File::create(dir.path().join("sub").join("wow.txt"))?;
+    std::fs::File::create(dir.path().join("sub").join("yay.txt"))?;
+
+    let descriptor =
+        store
+            .data_mut()
+            .push_dir(Box::new(wasi_cap_std_sync::dir::Dir::from_cap_std(
+                Dir::from_std_file(std::fs::File::open(dir.path())?),
+            )))?;
+
+    wasi.command(
+        &mut store,
+        0 as host::Descriptor,
+        1 as host::Descriptor,
+        &[],
+        &[],
+        &[(descriptor, "/")],
+    )
+    .await?
+    .map_err(|()| anyhow::anyhow!("command returned with failing exit status"))
+}

src/lib.rs

@@ -911,8 +911,8 @@ pub unsafe extern "C" fn fd_readdir(
                 state.dirent_cache.cookie.set(cookie - 1);
                 state.dirent_cache.cached_dirent.set(dirent);
                 std::ptr::copy(
-                    name.as_ptr().cast(),
-                    (*state.dirent_cache.path_data.get()).as_mut_ptr(),
+                    name.as_ptr().cast::<u8>(),
+                    (*state.dirent_cache.path_data.get()).as_mut_ptr() as *mut u8,
                     name.len(),
                 );
                 break;

test-programs/src/bin/directory_list.rs

@@ -0,0 +1,25 @@
+use std::{collections::HashSet, error::Error, fs, path::PathBuf};
+
+fn main() -> Result<(), Box<dyn Error>> {
+    assert_eq!(
+        ["/foo.txt", "/bar.txt", "/baz.txt", "/sub"]
+            .into_iter()
+            .map(PathBuf::from)
+            .collect::<HashSet<_>>(),
+        fs::read_dir("/")?
+            .map(|r| r.map(|d| d.path()))
+            .collect::<Result<_, _>>()?
+    );
+
+    assert_eq!(
+        ["/sub/wow.txt", "/sub/yay.txt"]
+            .into_iter()
+            .map(PathBuf::from)
+            .collect::<HashSet<_>>(),
+        fs::read_dir("/sub")?
+            .map(|r| r.map(|d| d.path()))
+            .collect::<Result<_, _>>()?
+    );
+
+    Ok(())
+}

test-programs/src/bin/file_read.rs

@@ -7,12 +7,14 @@ use std::{
 fn main() -> Result<(), Box<dyn Error>> {
     let mut file = File::open("bar.txt")?;
 
+    assert_eq!(27, file.metadata()?.len());
+
     assert_eq!(
         "And stood awhile in thought",
         &io::read_to_string(&mut file)?
     );
 
-    file.seek(SeekFrom::Start(11));
+    file.seek(SeekFrom::Start(11))?;
 
     assert_eq!("while in thought", &io::read_to_string(&mut file)?);
 

wasi-common/src/dir.rs

@@ -36,10 +36,7 @@ pub trait WasiDir: Send + Sync {
     }
 
     // XXX the iterator here needs to be asyncified as well!
-    async fn readdir(
-        &self,
-        _cursor: ReaddirCursor,
-    ) -> Result<Box<dyn Iterator<Item = Result<ReaddirEntity, Error>> + Send>, Error> {
+    async fn readdir(&self, _cursor: ReaddirCursor) -> Result<ReaddirIterator, Error> {
         Err(Error::not_supported())
     }
 
@@ -130,3 +127,5 @@ impl From<ReaddirCursor> for u64 {
         c.0
     }
 }
+
+pub type ReaddirIterator = Box<dyn Iterator<Item = Result<ReaddirEntity, Error>> + Send>;

wasi-common/src/table.rs

@@ -86,7 +86,14 @@ impl Table {
 
     /// Remove a resource at a given index from the table. Returns the resource
     /// if it was present.
-    pub fn delete(&mut self, key: u32) -> Option<Box<dyn Any + Send + Sync>> {
-        self.map.remove(&key)
+    pub fn delete<T: Any + Sized>(&mut self, key: u32) -> Result<Option<T>, Error> {
+        self.map
+            .remove(&key)
+            .map(|r| {
+                r.downcast::<T>()
+                    .map(|r| *r)
+                    .map_err(|_| Error::badf().context("element is a different type"))
+            })
+            .transpose()
     }
 }