Allow WASI to open directories without O_DIRECTORY.

crates/test-programs/wasi-tests/src/bin/fd_readdir.rs

@@ -65,11 +65,10 @@ unsafe fn exec_fd_readdir(fd: wasi::Fd, cookie: wasi::Dircookie) -> (Vec<DirEntr
     (dirs, eof)
 }
 
-unsafe fn test_fd_readdir(dir_fd: wasi::Fd) {
-    let stat = wasi::fd_filestat_get(dir_fd).expect("failed filestat");
+unsafe fn assert_empty_dir(fd: wasi::Fd) {
+    let stat = wasi::fd_filestat_get(fd).expect("failed filestat");
 
-    // Check the behavior in an empty directory
-    let (mut dirs, eof) = exec_fd_readdir(dir_fd, 0);
+    let (mut dirs, eof) = exec_fd_readdir(fd, 0);
     assert!(eof, "expected to read the entire directory");
     dirs.sort_by_key(|d| d.name.clone());
     assert_eq!(dirs.len(), 2, "expected two entries in an empty directory");
@@ -91,6 +90,11 @@ unsafe fn test_fd_readdir(dir_fd: wasi::Fd) {
         dirs.next().is_none(),
         "the directory should be seen as empty"
     );
+}
+
+unsafe fn test_fd_readdir(dir_fd: wasi::Fd) {
+    // Check the behavior in an empty directory
+    assert_empty_dir(dir_fd);
 
     // Add a file and check the behavior
     let file_fd = wasi::path_open(
@@ -111,16 +115,33 @@ unsafe fn test_fd_readdir(dir_fd: wasi::Fd) {
         "file descriptor range check",
     );
 
-    let stat = wasi::fd_filestat_get(file_fd).expect("failed filestat");
+    let file_stat = wasi::fd_filestat_get(file_fd).expect("failed filestat");
     wasi::fd_close(file_fd).expect("closing a file");
 
+    wasi::path_create_directory(dir_fd, "nested").expect("create a directory");
+    let nested_fd = wasi::path_open(
+        dir_fd,
+        0,
+        "nested",
+        0,
+        wasi::RIGHTS_FD_READ | wasi::RIGHTS_FD_READDIR | wasi::RIGHTS_FD_FILESTAT_GET,
+        0,
+        0,
+    )
+    .expect("failed to open nested directory");
+    assert!(
+        nested_fd > file_fd,
+        "nested directory file descriptor range check",
+    );
+    let nested_stat = wasi::fd_filestat_get(nested_fd).expect("failed filestat");
+
     // Execute another readdir
     let (mut dirs, eof) = exec_fd_readdir(dir_fd, 0);
     assert!(eof, "expected to read the entire directory");
-    assert_eq!(dirs.len(), 3, "expected three entries");
+    assert_eq!(dirs.len(), 4, "expected four entries");
     // Save the data about the last entry. We need to do it before sorting.
-    let lastfile_cookie = dirs[1].dirent.d_next;
-    let lastfile_name = dirs[2].name.clone();
+    let lastfile_cookie = dirs[2].dirent.d_next;
+    let lastfile_name = dirs[3].name.clone();
     dirs.sort_by_key(|d| d.name.clone());
     let mut dirs = dirs.into_iter();
 
@@ -136,15 +157,29 @@ unsafe fn test_fd_readdir(dir_fd: wasi::Fd) {
         wasi::FILETYPE_REGULAR_FILE,
         "type for the real file"
     );
-    assert_eq!(dir.dirent.d_ino, stat.ino);
+    assert_eq!(dir.dirent.d_ino, file_stat.ino);
+    let dir = dirs.next().expect("fourth entry is None");
+    // check the directory info
+    assert_eq!(dir.name, "nested", "nested directory name doesn't match");
+    assert_eq!(
+        dir.dirent.d_type,
+        wasi::FILETYPE_DIRECTORY,
+        "type for the nested directory"
+    );
+    assert_eq!(dir.dirent.d_ino, nested_stat.ino);
 
     // check if cookie works as expected
     let (dirs, eof) = exec_fd_readdir(dir_fd, lastfile_cookie);
     assert!(eof, "expected to read the entire directory");
     assert_eq!(dirs.len(), 1, "expected one entry");
     assert_eq!(dirs[0].name, lastfile_name, "name of the only entry");
 
+    // check if nested directory shows up as empty
+    assert_empty_dir(nested_fd);
+    wasi::fd_close(nested_fd).expect("closing a nested directory");
+
     wasi::path_unlink_file(dir_fd, "file").expect("removing a file");
+    wasi::path_remove_directory(dir_fd, "nested").expect("removing a nested directory");
 }
 
 unsafe fn test_fd_readdir_lots(dir_fd: wasi::Fd) {

crates/wasi-common/cap-std-sync/src/dir.rs

@@ -27,6 +27,9 @@ impl Dir {
     ) -> Result<File, Error> {
         use cap_fs_ext::{FollowSymlinks, OpenOptionsFollowExt};
 
+        // The current code assumes that directories are opened with `open_dir_`.
+        assert!(!oflags.contains(OFlags::DIRECTORY));
+
         let mut opts = cap_std::fs::OpenOptions::new();
 
         if oflags.contains(OFlags::CREATE | OFlags::EXCLUSIVE) {

crates/wasi-common/src/snapshots/preview_1.rs

@@ -17,7 +17,7 @@ use std::ops::Deref;
 use wiggle::GuestPtr;
 
 pub mod error;
-use error::{Error, ErrorExt};
+use error::{Errno, Error, ErrorExt};
 
 // Limit the size of intermediate buffers when copying to WebAssembly shared
 // memory.
@@ -39,6 +39,28 @@ impl wiggle::GuestErrorType for types::Errno {
     }
 }
 
+fn is_error_notdir(e: &Error) -> bool {
+    if let Some(e) = e.downcast_ref() {
+        match e {
+            Errno::Notdir => true,
+            _ => false,
+        }
+    } else {
+        false
+    }
+}
+
+fn is_error_noent(e: &Error) -> bool {
+    if let Some(e) = e.downcast_ref() {
+        match e {
+            Errno::Noent => true,
+            _ => false,
+        }
+    } else {
+        false
+    }
+}
+
 #[wiggle::async_trait]
 impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
     async fn args_get<'b>(
@@ -757,41 +779,77 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
         let oflags = OFlags::from(&oflags);
         let fdflags = FdFlags::from(fdflags);
         let path = path.as_cow()?;
-        if oflags.contains(OFlags::DIRECTORY) {
-            if oflags.contains(OFlags::CREATE)
-                || oflags.contains(OFlags::EXCLUSIVE)
-                || oflags.contains(OFlags::TRUNCATE)
-            {
-                return Err(Error::invalid_argument().context("directory oflags"));
-            }
-            let dir_caps = dir_entry.child_dir_caps(DirCaps::from(&fs_rights_base));
-            let file_caps = dir_entry.child_file_caps(FileCaps::from(&fs_rights_inheriting));
-            let dir = dir_entry.get_cap(DirCaps::OPEN)?;
-            let child_dir = dir.open_dir(symlink_follow, path.deref()).await?;
-            drop(dir);
-            let fd = table.push(Box::new(DirEntry::new(
-                dir_caps, file_caps, None, child_dir,
-            )))?;
-            Ok(types::Fd::from(fd))
-        } else {
-            let mut required_caps = DirCaps::OPEN;
-            if oflags.contains(OFlags::CREATE) {
-                required_caps = required_caps | DirCaps::CREATE_FILE;
-            }
 
-            let file_caps = dir_entry.child_file_caps(FileCaps::from(&fs_rights_base));
-            let dir = dir_entry.get_cap(required_caps)?;
-            let read = file_caps.contains(FileCaps::READ);
-            let write = file_caps.contains(FileCaps::WRITE)
-                || file_caps.contains(FileCaps::ALLOCATE)
-                || file_caps.contains(FileCaps::FILESTAT_SET_SIZE);
-            let file = dir
-                .open_file(symlink_follow, path.deref(), oflags, read, write, fdflags)
-                .await?;
-            drop(dir);
-            let fd = table.push(Box::new(FileEntry::new(file_caps, file)))?;
-            Ok(types::Fd::from(fd))
+        let mut required_caps = DirCaps::OPEN;
+        if oflags.contains(OFlags::CREATE) {
+            required_caps = required_caps | DirCaps::CREATE_FILE;
+        }
+        let dir = dir_entry.get_cap(required_caps)?;
+
+        // OFlags that are not supported for directories
+        let non_dir_oflags =
+            oflags.intersects(OFlags::CREATE | OFlags::EXCLUSIVE | OFlags::TRUNCATE);
+
+        // Check for `O_DIRECTORY` being used with OFlags not supported for directories.
+        if oflags.contains(OFlags::DIRECTORY) && non_dir_oflags {
+            return Err(Error::invalid_argument().context("directory oflags"));
+        }
+
+        // Determine whether we think we're not meant to open a directory: If
+        // we have any non-dir OFlags, or if we have `FD_WRITE` among the
+        // requested rights.
+        let non_dir = non_dir_oflags || fs_rights_base.contains(types::Rights::FD_WRITE);
+
+        // If the user requested a directory, or a directory-specific right, or
+        // if even they didn't make a request that's obviously not intending to
+        // open a directory, try to open it as a directory.
+        //
+        // On Unix, this kind of dance isn't necessary, because we could use
+        // a single open for either a file or a directory. On Windows, opening
+        // a directory requires special flags. In the future, we should specialize
+        // this code to use Unix-specific APIs instead of portable APIs so that
+        // it can avoid doing two opens.
+        if oflags.contains(OFlags::DIRECTORY)
+            || fs_rights_base.contains(types::Rights::FD_READDIR)
+            || !non_dir
+        {
+            match dir.open_dir(symlink_follow, path.deref()).await {
+                Ok(child_dir) => {
+                    let dir_caps = dir_entry.child_dir_caps(DirCaps::from(&fs_rights_base));
+                    let file_caps =
+                        dir_entry.child_file_caps(FileCaps::from(&fs_rights_inheriting));
+                    drop(dir);
+                    let fd = table.push(Box::new(DirEntry::new(
+                        dir_caps, file_caps, None, child_dir,
+                    )))?;
+                    return Ok(types::Fd::from(fd));
+                }
+                Err(e) => {
+                    // If we required a directory and didn't get one, fail.
+                    if oflags.contains(OFlags::DIRECTORY) {
+                        return Err(e);
+                    }
+                    // If the name didn't exist, or existed but isn't a
+                    // directory, proceed to try to open it as a file.
+                    // Otherwise fail.
+                    if !is_error_noent(&e) && !is_error_notdir(&e) {
+                        return Err(e);
+                    }
+                }
+            }
         }
+
+        let file_caps = dir_entry.child_file_caps(FileCaps::from(&fs_rights_base));
+        let dir = dir_entry.get_cap(required_caps)?;
+        let read = file_caps.contains(FileCaps::READ);
+        let write = file_caps
+            .intersects(FileCaps::WRITE | FileCaps::ALLOCATE | FileCaps::FILESTAT_SET_SIZE);
+        let file = dir
+            .open_file(symlink_follow, path.deref(), oflags, read, write, fdflags)
+            .await?;
+        drop(dir);
+        let fd = table.push(Box::new(FileEntry::new(file_caps, file)))?;
+        Ok(types::Fd::from(fd))
     }
 
     async fn path_readlink<'a>(