diff --git a/RELEASES.md b/RELEASES.md index af21ddf99868..79ebed75799d 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -1,5 +1,23 @@ -------------------------------------------------------------------------------- +## 8.0.1 + +Released 2023-04-27. + +### Changed + +* Breaking: Files opened using Wasmtime's implementation of WASI on Windows now + cannot be deleted until the file handle is closed. This was already true for + open directories. The change was necessary for the bug fix in + [#6163](https://github.com/bytecodealliance/wasmtime/pull/6163). + +### Fixed + +* Fixed wasi-common's implementation of the `O_DIRECTORY` flag to match POSIX. + [#6163](https://github.com/bytecodealliance/wasmtime/pull/6163) + +-------------------------------------------------------------------------------- + ## 8.0.0 Released 2023-04-20. diff --git a/crates/test-programs/wasi-tests/src/bin/fd_readdir.rs b/crates/test-programs/wasi-tests/src/bin/fd_readdir.rs index 87aa112c2b04..bc4bdd2d8524 100644 --- a/crates/test-programs/wasi-tests/src/bin/fd_readdir.rs +++ b/crates/test-programs/wasi-tests/src/bin/fd_readdir.rs @@ -65,10 +65,9 @@ unsafe fn exec_fd_readdir(fd: wasi::Fd, cookie: wasi::Dircookie) -> (Vec 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,7 +157,16 @@ 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); @@ -144,7 +174,12 @@ unsafe fn test_fd_readdir(dir_fd: wasi::Fd) { 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) { diff --git a/crates/test-programs/wasi-tests/src/bin/path_link.rs b/crates/test-programs/wasi-tests/src/bin/path_link.rs index 1216c334f95e..d229ba8aa5bb 100644 --- a/crates/test-programs/wasi-tests/src/bin/path_link.rs +++ b/crates/test-programs/wasi-tests/src/bin/path_link.rs @@ -91,9 +91,9 @@ unsafe fn test_path_link(dir_fd: wasi::Fd) { wasi::path_link(dir_fd, 0, "file", subdir_fd, "link").expect("creating a link in subdirectory"); let link_fd = open_link(subdir_fd, "link"); check_rights(file_fd, link_fd); + wasi::fd_close(link_fd).expect("Closing link_fd"); // needed for Windows wasi::path_unlink_file(subdir_fd, "link").expect("removing a link"); wasi::fd_close(subdir_fd).expect("Closing subdir_fd"); // needed for Windows - wasi::fd_close(link_fd).expect("Closing link_fd"); // needed for Windows wasi::path_remove_directory(dir_fd, "subdir").expect("removing a subdirectory"); // Create a link to a path that already exists diff --git a/crates/test-programs/wasi-tests/src/bin/poll_oneoff_files.rs b/crates/test-programs/wasi-tests/src/bin/poll_oneoff_files.rs index cbf9803130cf..8709fd8603d4 100644 --- a/crates/test-programs/wasi-tests/src/bin/poll_oneoff_files.rs +++ b/crates/test-programs/wasi-tests/src/bin/poll_oneoff_files.rs @@ -231,6 +231,7 @@ unsafe fn test_fd_readwrite_valid_fd(dir_fd: wasi::Fd) { test_fd_readwrite(readable_fd, writable_fd, wasi::ERRNO_SUCCESS); wasi::fd_close(readable_fd).expect("closing readable_file"); + wasi::fd_close(writable_fd).expect("closing writable_file"); wasi::path_unlink_file(dir_fd, "readable_file").expect("removing readable_file"); wasi::path_unlink_file(dir_fd, "writable_file").expect("removing writable_file"); } diff --git a/crates/wasi-common/cap-std-sync/src/dir.rs b/crates/wasi-common/cap-std-sync/src/dir.rs index 750bd3786d7b..18a1f8a5348b 100644 --- a/crates/wasi-common/cap-std-sync/src/dir.rs +++ b/crates/wasi-common/cap-std-sync/src/dir.rs @@ -1,18 +1,24 @@ use crate::file::{filetype_from, File}; -use cap_fs_ext::{DirEntryExt, DirExt, MetadataExt, SystemTimeSpec}; +use cap_fs_ext::{DirEntryExt, DirExt, MetadataExt, OpenOptionsMaybeDirExt, SystemTimeSpec}; +use cap_std::fs; use std::any::Any; use std::path::{Path, PathBuf}; use system_interface::fs::GetSetFdFlags; use wasi_common::{ dir::{ReaddirCursor, ReaddirEntity, WasiDir}, - file::{FdFlags, FileType, Filestat, OFlags, WasiFile}, + file::{FdFlags, FileType, Filestat, OFlags}, Error, ErrorExt, }; -pub struct Dir(cap_std::fs::Dir); +pub struct Dir(fs::Dir); + +pub enum OpenResult { + File(File), + Dir(Dir), +} impl Dir { - pub fn from_cap_std(dir: cap_std::fs::Dir) -> Self { + pub fn from_cap_std(dir: fs::Dir) -> Self { Dir(dir) } @@ -24,10 +30,11 @@ impl Dir { read: bool, write: bool, fdflags: FdFlags, - ) -> Result { + ) -> Result { use cap_fs_ext::{FollowSymlinks, OpenOptionsFollowExt}; - let mut opts = cap_std::fs::OpenOptions::new(); + let mut opts = fs::OpenOptions::new(); + opts.maybe_dir(true); if oflags.contains(OFlags::CREATE | OFlags::EXCLUSIVE) { opts.create_new(true); @@ -71,22 +78,31 @@ impl Dir { return Err(Error::not_supported().context("SYNC family of FdFlags")); } + 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 mut f = self.0.open_with(Path::new(path), &opts)?; // NONBLOCK does not have an OpenOption either, but we can patch that on with set_fd_flags: if fdflags.contains(wasi_common::file::FdFlags::NONBLOCK) { let set_fd_flags = f.new_set_fd_flags(system_interface::fs::FdFlags::NONBLOCK)?; f.set_fd_flags(set_fd_flags)?; } - Ok(File::from_cap_std(f)) - } - pub fn open_dir_(&self, symlink_follow: bool, path: &str) -> Result { - let d = if symlink_follow { - self.0.open_dir(Path::new(path))? + if f.metadata()?.is_dir() { + Ok(OpenResult::Dir(Dir::from_cap_std(fs::Dir::from_std_file( + f.into_std(), + )))) + } else if oflags.contains(OFlags::DIRECTORY) { + Err(Error::not_dir().context("expected directory but got file")) } else { - self.0.open_dir_nofollow(Path::new(path))? - }; - Ok(Dir::from_cap_std(d)) + Ok(OpenResult::File(File::from_cap_std(f))) + } } pub fn rename_(&self, src_path: &str, dest_dir: &Self, dest_path: &str) -> Result<(), Error> { @@ -120,14 +136,12 @@ impl WasiDir for Dir { read: bool, write: bool, fdflags: FdFlags, - ) -> Result, Error> { + ) -> Result { let f = self.open_file_(symlink_follow, path, oflags, read, write, fdflags)?; - Ok(Box::new(f)) - } - - async fn open_dir(&self, symlink_follow: bool, path: &str) -> Result, Error> { - let d = self.open_dir_(symlink_follow, path)?; - Ok(Box::new(d)) + match f { + OpenResult::File(f) => Ok(wasi_common::dir::OpenResult::File(Box::new(f))), + OpenResult::Dir(d) => Ok(wasi_common::dir::OpenResult::Dir(Box::new(d))), + } } async fn create_dir(&self, path: &str) -> Result<(), Error> { @@ -327,6 +341,7 @@ fn convert_systimespec(t: Option) -> Option), + Dir(Box), +} + #[wiggle::async_trait] pub trait WasiDir: Send + Sync { fn as_any(&self) -> &dyn Any; @@ -17,15 +22,7 @@ pub trait WasiDir: Send + Sync { _read: bool, _write: bool, _fdflags: FdFlags, - ) -> Result, Error> { - Err(Error::not_supported()) - } - - async fn open_dir( - &self, - _symlink_follow: bool, - _path: &str, - ) -> Result, Error> { + ) -> Result { Err(Error::not_supported()) } diff --git a/crates/wasi-common/src/snapshots/preview_1.rs b/crates/wasi-common/src/snapshots/preview_1.rs index d7b0d16f7aea..0773b8e82967 100644 --- a/crates/wasi-common/src/snapshots/preview_1.rs +++ b/crates/wasi-common/src/snapshots/preview_1.rs @@ -1,5 +1,8 @@ use crate::{ - dir::{DirCaps, DirEntry, DirEntryExt, DirFdStat, ReaddirCursor, ReaddirEntity, TableDirExt}, + dir::{ + DirCaps, DirEntry, DirEntryExt, DirFdStat, OpenResult, ReaddirCursor, ReaddirEntity, + TableDirExt, + }, file::{ Advice, FdFlags, FdStat, FileCaps, FileEntry, FileEntryExt, FileType, Filestat, OFlags, RiFlags, RoFlags, SdFlags, SiFlags, TableFileExt, WasiFile, @@ -741,41 +744,36 @@ 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(Arc::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(Arc::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_caps = dir_entry.child_dir_caps(DirCaps::from(&fs_rights_base)); + let dir_file_caps = dir_entry.child_file_caps(FileCaps::from(&fs_rights_inheriting)); + 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 = match file { + OpenResult::File(file) => table.push(Arc::new(FileEntry::new(file_caps, file)))?, + OpenResult::Dir(child_dir) => table.push(Arc::new(DirEntry::new( + dir_dir_caps, + dir_file_caps, + None, + child_dir, + )))?, + }; + Ok(types::Fd::from(fd)) } async fn path_readlink<'a>( diff --git a/crates/wasi-common/tokio/src/dir.rs b/crates/wasi-common/tokio/src/dir.rs index f95e61219eaa..5482a08c7619 100644 --- a/crates/wasi-common/tokio/src/dir.rs +++ b/crates/wasi-common/tokio/src/dir.rs @@ -3,7 +3,7 @@ use std::any::Any; use std::path::PathBuf; use wasi_common::{ dir::{ReaddirCursor, ReaddirEntity, WasiDir}, - file::{FdFlags, Filestat, OFlags, WasiFile}, + file::{FdFlags, Filestat, OFlags}, Error, ErrorExt, }; @@ -28,18 +28,19 @@ impl WasiDir for Dir { read: bool, write: bool, fdflags: FdFlags, - ) -> Result, Error> { + ) -> Result { let f = block_on_dummy_executor(move || async move { self.0 .open_file_(symlink_follow, path, oflags, read, write, fdflags) })?; - Ok(Box::new(File::from_inner(f))) - } - - async fn open_dir(&self, symlink_follow: bool, path: &str) -> Result, Error> { - let d = - block_on_dummy_executor(move || async move { self.0.open_dir_(symlink_follow, path) })?; - Ok(Box::new(Dir(d))) + match f { + wasi_cap_std_sync::dir::OpenResult::File(f) => Ok(wasi_common::dir::OpenResult::File( + Box::new(File::from_inner(f)), + )), + wasi_cap_std_sync::dir::OpenResult::Dir(d) => { + Ok(wasi_common::dir::OpenResult::Dir(Box::new(Dir(d)))) + } + } } async fn create_dir(&self, path: &str) -> Result<(), Error> { @@ -127,6 +128,7 @@ impl WasiDir for Dir { mod test { use super::Dir; use cap_std::ambient_authority; + use wasi_common::file::{FdFlags, OFlags}; #[tokio::test(flavor = "multi_thread")] async fn scratch_dir() { @@ -137,9 +139,17 @@ mod test { let preopen_dir = cap_std::fs::Dir::open_ambient_dir(tempdir.path(), ambient_authority()) .expect("open ambient temporary dir"); let preopen_dir = Dir::from_cap_std(preopen_dir); - wasi_common::WasiDir::open_dir(&preopen_dir, false, ".") - .await - .expect("open the same directory via WasiDir abstraction"); + wasi_common::WasiDir::open_file( + &preopen_dir, + false, + ".", + OFlags::empty(), + false, + false, + FdFlags::empty(), + ) + .await + .expect("open the same directory via WasiDir abstraction"); } // Readdir does not work on windows, so we won't test it there. diff --git a/crates/wasi-common/tokio/tests/poll_oneoff.rs b/crates/wasi-common/tokio/tests/poll_oneoff.rs index f305b7c44d3d..852abce2aad2 100644 --- a/crates/wasi-common/tokio/tests/poll_oneoff.rs +++ b/crates/wasi-common/tokio/tests/poll_oneoff.rs @@ -2,6 +2,7 @@ use anyhow::{Context, Error}; use cap_std::time::Duration; use std::collections::HashMap; use wasi_common::{ + dir::OpenResult, file::{FdFlags, OFlags}, sched::{Poll, RwEventFlags, SubscriptionResult, Userdata}, WasiDir, WasiFile, @@ -25,18 +26,26 @@ async fn empty_file_readable() -> Result<(), Error> { .await .context("create writable file f")?; let to_write: Vec = vec![0]; - f.write_vectored(&vec![std::io::IoSlice::new(&to_write)]) - .await - .context("write to f")?; + if let OpenResult::File(ref f) = f { + f.write_vectored(&vec![std::io::IoSlice::new(&to_write)]) + .await + .context("write to f")?; + } else { + unreachable!(); + } drop(f); - let mut f = d + let f = d .open_file(false, "f", OFlags::empty(), true, false, FdFlags::empty()) .await .context("open f as readable")?; let mut poll = Poll::new(); - poll.subscribe_read(&mut *f, Userdata::from(123)); + if let OpenResult::File(ref f) = f { + poll.subscribe_read(f.as_ref(), Userdata::from(123)); + } else { + unreachable!(); + } // Timeout bounds time in poll_oneoff let monotonic = &*clocks.monotonic()?.abs_clock; poll.subscribe_monotonic_clock( @@ -73,13 +82,17 @@ async fn empty_file_writable() -> Result<(), Error> { let d = workspace.open_dir("d").context("open dir")?; let d = Dir::from_cap_std(d); - let mut writable_f = d + let writable_f = d .open_file(false, "f", OFlags::CREATE, true, true, FdFlags::empty()) .await .context("create writable file")?; let mut poll = Poll::new(); - poll.subscribe_write(&mut *writable_f, Userdata::from(123)); + if let OpenResult::File(ref writable_f) = writable_f { + poll.subscribe_write(writable_f.as_ref(), Userdata::from(123)); + } else { + unreachable!(); + } // Timeout bounds time in poll_oneoff let monotonic = &*clocks.monotonic()?.abs_clock; poll.subscribe_monotonic_clock(