diff --git a/crates/wasi/src/filesystem/primitives/dir_entry.rs b/crates/wasi/src/filesystem/primitives/dir_entry.rs deleted file mode 100644 index 013880d48926..000000000000 --- a/crates/wasi/src/filesystem/primitives/dir_entry.rs +++ /dev/null @@ -1,67 +0,0 @@ -use crate::filesystem::primitives::{DirEntryInner, Metadata}; -#[cfg(not(windows))] -use rustix::fs::DirEntryExt; -use std::ffi::OsString; -use std::{fmt, io}; - -/// Entries returned by the `ReadDir` iterator. -/// -/// This corresponds to [`std::fs::DirEntry`]. -/// -/// Unlike `std::fs::DirEntry`, this API has no `DirEntry::path`, because -/// absolute paths don't interoperate well with the capability model. -/// -/// There is a `file_name` function, however there are also `open`, -/// `open_with`, `open_dir`, `remove_file`, and `remove_dir` functions for -/// opening or removing the entry directly, which can be more efficient and -/// convenient. -/// -/// There is no `from_std` method, as `std::fs::DirEntry` doesn't provide a way -/// to construct a `DirEntry` without opening directories by ambient paths. -pub struct DirEntry { - pub(crate) inner: DirEntryInner, -} - -impl DirEntry { - /// Returns the metadata for the file that this entry points at. - /// - /// This corresponds to [`std::fs::DirEntry::metadata`]. - /// - /// # Platform-specific behavior - /// - /// On Windows, this produces a `Metadata` object which does not contain - /// the optional values returned by [`MetadataExt`]. Use - /// [`cap_fs_ext::DirEntryExt::full_metadata`] to obtain a `Metadata` with - /// the values filled in. - /// - /// [`MetadataExt`]: https://doc.rust-lang.org/std/os/windows/fs/trait.MetadataExt.html - /// [`cap_fs_ext::DirEntryExt::full_metadata`]: https://docs.rs/cap-fs-ext/latest/cap_fs_ext/trait.DirEntryExt.html#tymethod.full_metadata - #[inline] - pub fn metadata(&self) -> io::Result { - self.inner.metadata() - } - - /// Returns the bare file name of this directory entry without any other - /// leading path component. - /// - /// This corresponds to [`std::fs::DirEntry::file_name`]. - #[inline] - pub fn file_name(&self) -> OsString { - self.inner.file_name() - } -} - -#[cfg(not(windows))] -impl DirEntryExt for DirEntry { - #[inline] - fn ino(&self) -> u64 { - self.inner.ino() - } -} - -impl fmt::Debug for DirEntry { - // Like libstd's version, but doesn't print the path. - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.inner.fmt(f) - } -} diff --git a/crates/wasi/src/filesystem/primitives/metadata.rs b/crates/wasi/src/filesystem/primitives/metadata.rs index ee2d9d770c4a..d2ed46c2288d 100644 --- a/crates/wasi/src/filesystem/primitives/metadata.rs +++ b/crates/wasi/src/filesystem/primitives/metadata.rs @@ -31,21 +31,6 @@ impl Metadata { Ok(Self::from_parts(std, ext, file_type)) } - /// Constructs a new instance of `Self` from the given - /// [`std::fs::Metadata`]. - /// - /// As with the comments in [`std::fs::Metadata::volume_serial_number`] and - /// nearby functions, some fields of the resulting metadata will be `None`. - /// - /// [`std::fs::Metadata::volume_serial_number`]: https://doc.rust-lang.org/std/os/windows/fs/trait.MetadataExt.html#tymethod.volume_serial_number - #[cfg(windows)] - #[inline] - pub fn from_just_metadata(std: fs::Metadata) -> Self { - let ext = ImplMetadataExt::from_just_metadata(&std); - let file_type = ImplFileTypeExt::from_just_metadata(&std); - Self::from_parts(std, ext, file_type) - } - #[inline] fn from_parts(std: fs::Metadata, ext: ImplMetadataExt, file_type: FileType) -> Self { Self { diff --git a/crates/wasi/src/filesystem/primitives/mod.rs b/crates/wasi/src/filesystem/primitives/mod.rs index 4f6e1d89d980..be4f6721be1a 100644 --- a/crates/wasi/src/filesystem/primitives/mod.rs +++ b/crates/wasi/src/filesystem/primitives/mod.rs @@ -20,14 +20,12 @@ use std::path::{Component, Path, PathBuf}; use std::{fs, io}; -mod dir_entry; mod file_type; mod maybe_owned_file; mod metadata; mod open_options; mod open_parent; mod open_unchecked_error; -mod read_dir; mod errors; mod manually; @@ -51,7 +49,6 @@ use open_parent::open_parent; use open_unchecked_error::*; use sys::*; -pub(crate) use dir_entry::DirEntry; pub(crate) use file_type::FileType; #[cfg(any(unix, target_os = "vxworks"))] pub(crate) use file_type::FileTypeExt; @@ -59,8 +56,8 @@ pub(crate) use file_type::FileTypeExt; pub(crate) use metadata::_WindowsByHandle; pub(crate) use metadata::{Metadata, MetadataExt}; pub(crate) use open_options::*; -pub(crate) use read_dir::read_base_dir; pub(crate) use sys::open_ambient_dir; +pub(crate) use sys::read_dir; pub(crate) use sys::set_times; pub(crate) use sys::set_times_nofollow; @@ -285,18 +282,6 @@ fn open_dir_unchecked(start: &fs::File, path: &Path) -> io::Result { open_unchecked(start, path, &dir_options()).map_err(Into::into) } -/// Like `open_dir_unchecked`, but additionally request the ability to read the -/// directory entries. -#[inline] -#[allow(dead_code)] -fn open_dir_for_reading_unchecked( - start: &fs::File, - path: &Path, - follow: FollowSymlinks, -) -> io::Result { - open_unchecked(start, path, readdir_options().follow(follow)).map_err(Into::into) -} - pub(crate) fn remove_file(start: &fs::File, path: &Path) -> io::Result<()> { #[cfg(target_os = "freebsd")] if sys::remove_file_fast(start, path)? { diff --git a/crates/wasi/src/filesystem/primitives/open_options.rs b/crates/wasi/src/filesystem/primitives/open_options.rs index 73b43e2ef287..13a7227224a5 100644 --- a/crates/wasi/src/filesystem/primitives/open_options.rs +++ b/crates/wasi/src/filesystem/primitives/open_options.rs @@ -41,6 +41,7 @@ pub struct OpenOptions { pub(crate) rsync: bool, #[cfg(not(windows))] pub(crate) nonblock: bool, + #[cfg(not(windows))] pub(crate) readdir_required: bool, pub(crate) follow: FollowSymlinks, @@ -80,6 +81,7 @@ impl OpenOptions { rsync: false, #[cfg(not(windows))] nonblock: false, + #[cfg(not(windows))] readdir_required: false, follow: FollowSymlinks::Yes, @@ -150,6 +152,7 @@ impl OpenOptions { /// Sets the option to request the ability to read directory entries. #[inline] + #[cfg(not(windows))] pub(crate) fn readdir_required(&mut self, readdir_required: bool) -> &mut Self { self.readdir_required = readdir_required; self diff --git a/crates/wasi/src/filesystem/primitives/read_dir.rs b/crates/wasi/src/filesystem/primitives/read_dir.rs deleted file mode 100644 index dd6fd0c07f8d..000000000000 --- a/crates/wasi/src/filesystem/primitives/read_dir.rs +++ /dev/null @@ -1,38 +0,0 @@ -use crate::filesystem::primitives::{DirEntry, ReadDirInner}; -use std::{fmt, fs, io}; - -/// Like `read_dir` but operates on the base directory itself, rather than -/// on a path based on it. -#[inline] -pub fn read_base_dir(start: &fs::File) -> io::Result { - Ok(ReadDir { - inner: ReadDirInner::read_base_dir(start)?, - }) -} - -/// Iterator over the entries in a directory. -/// -/// This corresponds to [`std::fs::ReadDir`]. -/// -/// There is no `from_std` method, as `std::fs::ReadDir` doesn't provide a way -/// to construct a `ReadDir` without opening directories by ambient paths. -pub struct ReadDir { - pub(crate) inner: ReadDirInner, -} - -impl Iterator for ReadDir { - type Item = io::Result; - - #[inline] - fn next(&mut self) -> Option { - self.inner - .next() - .map(|inner| inner.map(|inner| DirEntry { inner })) - } -} - -impl fmt::Debug for ReadDir { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.inner.fmt(f) - } -} diff --git a/crates/wasi/src/filesystem/primitives/tests/fs.rs b/crates/wasi/src/filesystem/primitives/tests/fs.rs index 71a5b221b27a..b2be92453cb3 100644 --- a/crates/wasi/src/filesystem/primitives/tests/fs.rs +++ b/crates/wasi/src/filesystem/primitives/tests/fs.rs @@ -57,15 +57,13 @@ fn dir_entry_methods() { h::create_dir_all(&start, "a").unwrap(); h::create(&start, "b").unwrap(); - // `DirEntry::file_type` is gone; the metadata checks still cover this. - for file in h::read_dir(&start, ".").unwrap().map(|f| f.unwrap()) { - let fname = file.file_name(); + for (fname, ty) in p::read_dir(&start).unwrap().map(|f| f.unwrap()) { match fname.to_str() { Some("a") => { - assert!(file.metadata().unwrap().is_dir()); + assert!(ty.is_dir()); } Some("b") => { - assert!(file.metadata().unwrap().file_type().is_file()); + assert!(ty.is_file()); } f => panic!("unknown file name: {f:?}"), } @@ -600,10 +598,13 @@ fn file_test_directoryinfo_readdir() { let msg = msg_str.as_bytes(); check!(w.write(msg)); } - let files = check!(h::read_dir(&start, dir)); + let files = { + let dir_handle = check!(p::open_dir(&start, dir.as_ref())); + check!(p::read_dir(&dir_handle)) + }; let mut mem = [0; 4]; for f in files { - let f = f.unwrap().file_name(); + let (f, _ty) = f.unwrap(); { check!(check!(h::open(&start, &f)).read(&mut mem)); let read_str = str::from_utf8(&mem).unwrap(); @@ -1027,23 +1028,11 @@ fn mkdir_trailing_slash() { check!(h::create_dir_all(&start, &path.join("a/"))); } -#[test] -fn dir_entry_debug() { - let tmpdir = tmpdir(); - let start = h::dir_of(&tmpdir); - h::create(&start, "b").unwrap(); - let mut read_dir = h::read_dir(&start, ".").unwrap(); - let dir_entry = read_dir.next().unwrap().unwrap(); - let actual = format!("{dir_entry:?}"); - let expected = format!("DirEntry({:?})", dir_entry.file_name()); - assert_eq!(actual, expected); -} - #[test] fn read_dir_not_found() { let tmpdir = tmpdir(); let start = h::dir_of(&tmpdir); - let res = h::read_dir(&start, "path/that/does/not/exist"); + let res = p::open_dir(&start, "path/that/does/not/exist".as_ref()); assert_eq!(res.err().unwrap().kind(), ErrorKind::NotFound); } diff --git a/crates/wasi/src/filesystem/primitives/tests/fs_additional.rs b/crates/wasi/src/filesystem/primitives/tests/fs_additional.rs index ce0faa4315e3..8d308b9c9821 100644 --- a/crates/wasi/src/filesystem/primitives/tests/fs_additional.rs +++ b/crates/wasi/src/filesystem/primitives/tests/fs_additional.rs @@ -124,7 +124,8 @@ fn dotdot_at_end_of_symlink() { check!(h::metadata(&start, path)); - let contents = check!(h::read_dir(&start, path)); + let dir_handle = check!(p::open_dir(&start, path.as_ref())); + let contents = check!(p::read_dir(&dir_handle)); for entry in contents { let _entry = check!(entry); } @@ -149,7 +150,8 @@ fn dotdot_at_end_of_symlink_all_inside_dir() { check!(h::metadata(&start, path)); - let contents = check!(h::read_dir(&start, path)); + let dir_handle = check!(p::open_dir(&start, path.as_ref())); + let contents = check!(p::read_dir(&dir_handle)); for entry in contents { let _entry = check!(entry); } @@ -174,7 +176,8 @@ fn dotdot_slashdot_at_end_of_symlink() { check!(h::metadata(&start, path)); - let contents = check!(h::read_dir(&start, path)); + let dir_handle = check!(p::open_dir(&start, path.as_ref())); + let contents = check!(p::read_dir(&dir_handle)); for entry in contents { let _entry = check!(entry); } @@ -200,7 +203,8 @@ fn dotdot_slashdot_at_end_of_symlink_all_inside_dir() { check!(h::metadata(&start, path)); - let contents = check!(h::read_dir(&start, path)); + let dir_handle = check!(p::open_dir(&start, path.as_ref())); + let contents = check!(p::read_dir(&dir_handle)); for entry in contents { let _entry = check!(entry); } @@ -454,17 +458,17 @@ fn file_test_directoryinfo_readdir() { check!(w.write(msg)); } let sub = check!(p::open_dir(&start, Path::new(dir))); - let files = check!(p::read_base_dir(&sub)); + let files = check!(p::read_dir(&sub)); let mut mem = [0; 4]; for f in files { - let f = f.unwrap(); + let (f, _ty) = f.unwrap(); { - check!(check!(h::open(&sub, f.file_name())).read(&mut mem)); + check!(check!(h::open(&sub, &f)).read(&mut mem)); let read_str = str::from_utf8(&mem).unwrap(); - let expected = format!("{}{}", prefix, f.file_name().to_str().unwrap()); + let expected = format!("{}{}", prefix, f.to_str().unwrap()); assert_eq!(expected, read_str); } - check!(p::remove_file(&sub, Path::new(&f.file_name()))); + check!(p::remove_file(&sub, Path::new(&f))); } drop(sub); check!(p::remove_dir(&start, Path::new(dir))); @@ -895,9 +899,12 @@ fn readdir_with_trailing_slashdot() { check!(h::create(&start, "dir/green")); check!(h::create(&start, "dir/blue")); - assert_eq!(check!(h::read_dir(&start, "dir")).count(), 3); - assert_eq!(check!(h::read_dir(&start, "dir/")).count(), 3); - assert_eq!(check!(h::read_dir(&start, "dir/.")).count(), 3); + let h1 = check!(p::open_dir(&start, "dir".as_ref())); + let h2 = check!(p::open_dir(&start, "dir/".as_ref())); + let h3 = check!(p::open_dir(&start, "dir/.".as_ref())); + assert_eq!(check!(p::read_dir(&h1)).count(), 3); + assert_eq!(check!(p::read_dir(&h2)).count(), 3); + assert_eq!(check!(p::read_dir(&h3)).count(), 3); } #[test] @@ -910,13 +917,12 @@ fn metadata_vs_std_fs() { let cap_std_dir = check!(p::Metadata::from_file(&dir)); let cap_std_file = check!(p::Metadata::from_file(&file)); - let cap_std_dir_entry = { - let mut entries = check!(p::read_base_dir(&dir)); - let entry = check!(entries.next().unwrap()); - assert_eq!(entry.file_name(), "file"); + { + let mut entries = check!(p::read_dir(&dir)); + let (entry, _ty) = check!(entries.next().unwrap()); + assert_eq!(entry, "file"); assert!(entries.next().is_none(), "unexpected dir entry"); - check!(entry.metadata()) - }; + } let std_dir = check!(dir.metadata()); let std_file = check!(file.metadata()); @@ -928,7 +934,6 @@ fn metadata_vs_std_fs() { check_metadata(&std_dir, &cap_std_dir); check_metadata(&std_file, &cap_std_file); - check_metadata(&std_file, &cap_std_dir_entry); } fn check_metadata(std: &std::fs::Metadata, cap: &p::Metadata) { @@ -1278,12 +1283,9 @@ fn trailing_slash_symlink() { for path in ["hidden", "hidden/", "indirect", "indirect/"] { let open_dir = p::open_dir(&sandbox, Path::new(path)); - let read_dir = h::read_dir(&sandbox, path); assert!(open_dir.is_err()); - assert!(read_dir.is_err()); if cfg!(unix) { error_contains!(open_dir, "a path led outside of the filesystem"); - error_contains!(read_dir, "a path led outside of the filesystem"); } } } @@ -1343,12 +1345,9 @@ fn trailing_slash_symlink_more() { "root_link/", ] { let open_dir = p::open_dir(&sandbox, Path::new(path)); - let read_dir = h::read_dir(&sandbox, path); assert!(open_dir.is_err()); - assert!(read_dir.is_err()); if cfg!(unix) { error_contains!(open_dir, "a path led outside of the filesystem"); - error_contains!(read_dir, "a path led outside of the filesystem"); } } } diff --git a/crates/wasi/src/filesystem/primitives/tests/helpers/mod.rs b/crates/wasi/src/filesystem/primitives/tests/helpers/mod.rs index ff6b5b1a6743..ba022564e5ec 100644 --- a/crates/wasi/src/filesystem/primitives/tests/helpers/mod.rs +++ b/crates/wasi/src/filesystem/primitives/tests/helpers/mod.rs @@ -76,11 +76,6 @@ pub fn open_dir_nofollow(d: &File, path: impl AsRef) -> io::Result { ) } -/// `Dir::read_dir`: open the subdirectory, then read its entries. -pub fn read_dir(d: &File, path: impl AsRef) -> io::Result { - p::read_base_dir(&p::open_dir(d, path.as_ref())?) -} - pub fn exists(d: &File, path: impl AsRef) -> bool { metadata(d, path).is_ok() } diff --git a/crates/wasi/src/filesystem/primitives/tests/mod.rs b/crates/wasi/src/filesystem/primitives/tests/mod.rs index 85545e17dfd2..68cd848f043d 100644 --- a/crates/wasi/src/filesystem/primitives/tests/mod.rs +++ b/crates/wasi/src/filesystem/primitives/tests/mod.rs @@ -9,7 +9,6 @@ mod fs; mod fs_additional; mod metadata_ext; mod paths_containing_nul; -mod readdir; mod rename; mod rename_directory; mod reopendir; diff --git a/crates/wasi/src/filesystem/primitives/tests/readdir.rs b/crates/wasi/src/filesystem/primitives/tests/readdir.rs deleted file mode 100644 index 8ceea29fae5c..000000000000 --- a/crates/wasi/src/filesystem/primitives/tests/readdir.rs +++ /dev/null @@ -1,79 +0,0 @@ -use crate::filesystem::primitives::{DirEntry, open_ambient_dir, read_base_dir}; -use std::collections::HashMap; -use std::fs::File; -use std::path::Path; - -#[test] -fn test_dir_entries() { - let tmpdir = tempfile::tempdir().expect("construct tempdir"); - - let entries = dir_entries(&tmpdir.path()); - assert_eq!(entries.len(), 0, "empty dir"); - - let _f1 = std::fs::File::create(tmpdir.path().join("file1")).expect("create file1"); - - let entries = dir_entries(&tmpdir.path()); - assert!( - entries.get("file1").is_some(), - "directory contains `file1`: {entries:?}" - ); - assert_eq!(entries.len(), 1); - - let _f2 = std::fs::File::create(tmpdir.path().join("file2")).expect("create file1"); - let entries = dir_entries(&tmpdir.path()); - assert!( - entries.get("file1").is_some(), - "directory contains `file1`: {entries:?}" - ); - assert!( - entries.get("file2").is_some(), - "directory contains `file2`: {entries:?}" - ); - assert_eq!(entries.len(), 2); -} - -#[test] -fn test_reread_entries() { - let tmpdir = tempfile::tempdir().expect("construct tempdir"); - let dir = open_ambient_dir(tmpdir.path()).unwrap(); - - let entries = read_entries(&dir); - assert_eq!(entries.len(), 0, "empty dir"); - - let _f1 = std::fs::File::create(tmpdir.path().join("file1")).expect("create file1"); - - let entries = read_entries(&dir); - assert!( - entries.get("file1").is_some(), - "directory contains `file1`: {entries:?}" - ); - assert_eq!(entries.len(), 1); - - let _f2 = std::fs::File::create(tmpdir.path().join("file2")).expect("create file1"); - let entries = read_entries(&dir); - assert!( - entries.get("file1").is_some(), - "directory contains `file1`: {entries:?}" - ); - assert!( - entries.get("file2").is_some(), - "directory contains `file2`: {entries:?}" - ); - assert_eq!(entries.len(), 2); -} - -fn dir_entries(path: &Path) -> HashMap { - let dir = open_ambient_dir(path).unwrap(); - read_entries(&dir) -} - -fn read_entries(dir: &File) -> HashMap { - let mut out = HashMap::new(); - for e in read_base_dir(dir).unwrap() { - let e = e.expect("non-error entry"); - let name = e.file_name().to_str().expect("utf8 filename").to_owned(); - assert!(out.get(&name).is_none(), "name already read: {name}"); - out.insert(name, e); - } - out -} diff --git a/crates/wasi/src/filesystem/primitives/unix/dir_entry_inner.rs b/crates/wasi/src/filesystem/primitives/unix/dir_entry_inner.rs deleted file mode 100644 index 450f6df688bd..000000000000 --- a/crates/wasi/src/filesystem/primitives/unix/dir_entry_inner.rs +++ /dev/null @@ -1,41 +0,0 @@ -use crate::filesystem::primitives::{Metadata, ReadDirInner}; -use rustix::fs::DirEntry; -use std::ffi::{OsStr, OsString}; -#[cfg(unix)] -use std::os::unix::ffi::OsStrExt; -#[cfg(target_os = "wasi")] -use std::os::wasi::ffi::OsStrExt; -use std::{fmt, io}; - -pub(crate) struct DirEntryInner { - pub(super) rustix: DirEntry, - pub(super) read_dir: ReadDirInner, -} - -impl DirEntryInner { - #[inline] - pub(crate) fn metadata(&self) -> io::Result { - self.read_dir.metadata(self.file_name_bytes()) - } - - #[inline] - pub(crate) fn file_name(&self) -> OsString { - self.file_name_bytes().to_os_string() - } - - #[inline] - pub(crate) fn ino(&self) -> u64 { - self.rustix.ino() - } - - fn file_name_bytes(&self) -> &OsStr { - OsStr::from_bytes(self.rustix.file_name().to_bytes()) - } -} - -impl fmt::Debug for DirEntryInner { - // Like libstd's version, but doesn't print the path. - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_tuple("DirEntry").field(&self.file_name()).finish() - } -} diff --git a/crates/wasi/src/filesystem/primitives/unix/mod.rs b/crates/wasi/src/filesystem/primitives/unix/mod.rs index 50d509d1ef95..18cae07dca93 100644 --- a/crates/wasi/src/filesystem/primitives/unix/mod.rs +++ b/crates/wasi/src/filesystem/primitives/unix/mod.rs @@ -1,13 +1,16 @@ -use crate::filesystem::primitives::{MaybeOwnedFile, OpenOptions, open, open_parent}; -use rustix::fs::{AtFlags, utimensat}; +use crate::filesystem::primitives::{ + FileType, FollowSymlinks, MaybeOwnedFile, OpenOptions, open, open_parent, +}; +use rustix::fs::{AtFlags, Dir, utimensat}; use rustix::io::Errno; +use std::ffi::OsString; use std::fs; use std::io; -use std::path::{Path, PathBuf}; +use std::os::unix::ffi::{OsStrExt, OsStringExt}; +use std::path::{Component, Path, PathBuf}; use std::time::SystemTime; mod create_dir_unchecked; -mod dir_entry_inner; mod dir_utils; mod file_type_ext; mod hard_link_unchecked; @@ -16,7 +19,6 @@ mod metadata_ext; mod oflags; mod open_options_ext; mod open_unchecked; -mod read_dir_inner; mod read_link_unchecked; mod remove_dir_unchecked; mod remove_file_unchecked; @@ -42,7 +44,6 @@ mod linux; pub(crate) use self::linux::*; pub(crate) use create_dir_unchecked::create_dir_unchecked; -pub(crate) use dir_entry_inner::DirEntryInner; pub(crate) use dir_utils::*; pub(crate) use file_type_ext::ImplFileTypeExt; pub(crate) use hard_link_unchecked::hard_link_unchecked; @@ -51,7 +52,6 @@ pub(crate) use is_same_file::{is_different_file, is_different_file_metadata, is_ pub(crate) use metadata_ext::ImplMetadataExt; pub(crate) use open_options_ext::ImplOpenOptionsExt; pub(crate) use open_unchecked::open_unchecked; -pub(crate) use read_dir_inner::ReadDirInner; pub(crate) use read_link_unchecked::read_link_unchecked; pub(crate) use remove_dir_unchecked::remove_dir_unchecked; pub(crate) use remove_file_unchecked::remove_file_unchecked; @@ -152,3 +152,52 @@ pub(crate) fn set_times( // So neither does what we need. Err(Errno::NOTSUP.into()) } + +pub(crate) fn read_dir( + file: &fs::File, +) -> io::Result> + 'static> { + // Open ".", to obtain a new independent file descriptor. Don't use + // `dup` since in that case the resulting file descriptor would share + // a current position with the original, and `read_dir` calls after + // the first `read_dir` call wouldn't start from the beginning. + let fd = open_unchecked( + file, + Component::CurDir.as_ref(), + readdir_options().follow(FollowSymlinks::No), + )?; + let mut dir = Dir::new(fd)?; + Ok(std::iter::from_fn(move || { + let result = (|| { + loop { + let Some(entry) = dir.read() else { + return Ok(None); + }; + let entry = entry?; + let file_name = entry.file_name().to_bytes(); + if file_name == Component::CurDir.as_os_str().as_bytes() + || file_name == Component::ParentDir.as_os_str().as_bytes() + { + continue; + } + + let raw_mode = cfg_select! { + target_os = "illumos" => rustix::fs::statat( + dir.fd()?, + entry.file_name(), + AtFlags::SYMLINK_NOFOLLOW, + )?.st_mode, + _ => entry.file_type().as_raw_mode(), + + }; + + let file_type = ImplFileTypeExt::from_raw_mode(raw_mode); + return Ok(Some((OsString::from_vec(file_name.to_vec()), file_type))); + } + })(); + match result { + Ok(Some(entry)) => Some(Ok(entry)), + Ok(None) => None, + Err(e) => Some(Err(e)), + } + })) +} diff --git a/crates/wasi/src/filesystem/primitives/unix/read_dir_inner.rs b/crates/wasi/src/filesystem/primitives/unix/read_dir_inner.rs deleted file mode 100644 index 558189b2c990..000000000000 --- a/crates/wasi/src/filesystem/primitives/unix/read_dir_inner.rs +++ /dev/null @@ -1,87 +0,0 @@ -use crate::filesystem::primitives::{ - DirEntryInner, FollowSymlinks, Metadata, open_dir_for_reading_unchecked, stat_unchecked, -}; -use rustix::fd::{AsFd, OwnedFd}; -use rustix::fs::Dir; -use std::ffi::OsStr; -use std::mem::ManuallyDrop; -use std::os::fd::{AsRawFd, FromRawFd, RawFd}; -#[cfg(unix)] -use std::os::unix::ffi::OsStrExt; -#[cfg(target_os = "wasi")] -use std::os::wasi::ffi::OsStrExt; -use std::path::Component; -use std::sync::{Arc, Mutex}; -use std::{fmt, fs, io}; - -pub(crate) struct ReadDirInner { - raw_fd: RawFd, - - // `Dir` doesn't implement `AsFd`, because libc `fdopendir` has UB if the - // file descriptor is used in almost any way, so we hold a separate - // `OwnedFd` that we can do `as_fd()` on. - rustix: Arc>, -} - -impl ReadDirInner { - pub(crate) fn read_base_dir(start: &fs::File) -> io::Result { - // Open ".", to obtain a new independent file descriptor. Don't use - // `dup` since in that case the resulting file descriptor would share - // a current position with the original, and `read_dir` calls after - // the first `read_dir` call wouldn't start from the beginning. - let fd = - open_dir_for_reading_unchecked(start, Component::CurDir.as_ref(), FollowSymlinks::No)?; - let dir = Dir::read_from(fd.as_fd())?; - Ok(Self { - raw_fd: fd.as_fd().as_raw_fd(), - rustix: Arc::new(Mutex::new((dir, fd.into()))), - }) - } - - pub(super) fn metadata(&self, file_name: &OsStr) -> io::Result { - stat_unchecked(&self.as_file_view(), file_name.as_ref(), FollowSymlinks::No) - } - - #[allow(unsafe_code)] - fn as_file_view(&self) -> ManuallyDrop { - // Safety: `self.rustix` owns the file descriptor. We just hold a - // copy outside so that we can read it without taking a lock. - ManuallyDrop::new(unsafe { fs::File::from_raw_fd(self.raw_fd) }) - } -} - -impl Iterator for ReadDirInner { - type Item = io::Result; - - fn next(&mut self) -> Option { - loop { - let entry = self.rustix.lock().unwrap().0.read()?; - let entry = match entry { - Ok(entry) => entry, - Err(e) => return Some(Err(e.into())), - }; - let file_name = entry.file_name().to_bytes(); - if file_name != Component::CurDir.as_os_str().as_bytes() - && file_name != Component::ParentDir.as_os_str().as_bytes() - { - let clone = Arc::clone(&self.rustix); - return Some(Ok(DirEntryInner { - rustix: entry, - read_dir: Self { - raw_fd: self.raw_fd, - rustix: clone, - }, - })); - } - } - } -} - -impl fmt::Debug for ReadDirInner { - // Like libstd's version, but doesn't print the path. - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut b = f.debug_struct("ReadDir"); - b.field("raw_fd", &self.raw_fd); - b.finish() - } -} diff --git a/crates/wasi/src/filesystem/primitives/windows/dir_entry_inner.rs b/crates/wasi/src/filesystem/primitives/windows/dir_entry_inner.rs deleted file mode 100644 index c70104683ffe..000000000000 --- a/crates/wasi/src/filesystem/primitives/windows/dir_entry_inner.rs +++ /dev/null @@ -1,31 +0,0 @@ -use crate::filesystem::primitives::Metadata; -use std::ffi::OsString; -use std::{fmt, fs, io}; - -pub(crate) struct DirEntryInner { - std: fs::DirEntry, -} - -impl DirEntryInner { - #[inline] - pub(crate) fn metadata(&self) -> io::Result { - self.std.metadata().map(Metadata::from_just_metadata) - } - - #[inline] - pub(crate) fn file_name(&self) -> OsString { - self.std.file_name() - } - - #[inline] - pub(super) fn from_std(std: fs::DirEntry) -> Self { - Self { std } - } -} - -impl fmt::Debug for DirEntryInner { - // Like libstd's version, but doesn't print the path. - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_tuple("DirEntry").field(&self.file_name()).finish() - } -} diff --git a/crates/wasi/src/filesystem/primitives/windows/dir_utils.rs b/crates/wasi/src/filesystem/primitives/windows/dir_utils.rs index e0897fa38a6e..f9682e886e26 100644 --- a/crates/wasi/src/filesystem/primitives/windows/dir_utils.rs +++ b/crates/wasi/src/filesystem/primitives/windows/dir_utils.rs @@ -70,12 +70,6 @@ pub(crate) fn dir_options() -> OpenOptions { .clone() } -/// Like `dir_options`, but additionally request the ability to read the -/// directory entries. -pub(crate) fn readdir_options() -> OpenOptions { - dir_options().readdir_required(true).clone() -} - /// Open a directory named by a bare path, using the host process' ambient /// authority. /// diff --git a/crates/wasi/src/filesystem/primitives/windows/metadata_ext.rs b/crates/wasi/src/filesystem/primitives/windows/metadata_ext.rs index 4aaf4cb0f521..3d164c8321ce 100644 --- a/crates/wasi/src/filesystem/primitives/windows/metadata_ext.rs +++ b/crates/wasi/src/filesystem/primitives/windows/metadata_ext.rs @@ -21,18 +21,6 @@ impl ImplMetadataExt { Ok(Self::from_parts(std, Some(t32))) } - /// Constructs a new instance of `Self` from the given - /// [`std::fs::Metadata`]. - /// - /// As with the comments in [`std::fs::Metadata::volume_serial_number`] and - /// nearby functions, some fields of the resulting metadata will be `None`. - /// - /// [`std::fs::Metadata::volume_serial_number`]: https://doc.rust-lang.org/std/os/windows/fs/trait.MetadataExt.html#tymethod.volume_serial_number - #[inline] - pub(crate) fn from_just_metadata(std: &fs::Metadata) -> Self { - Self::from_parts(std, None) - } - #[inline] fn from_parts(std: &fs::Metadata, number_of_links: Option) -> Self { use std::os::windows::fs::MetadataExt; diff --git a/crates/wasi/src/filesystem/primitives/windows/mod.rs b/crates/wasi/src/filesystem/primitives/windows/mod.rs index cdab84db1b74..d36e12db3b92 100644 --- a/crates/wasi/src/filesystem/primitives/windows/mod.rs +++ b/crates/wasi/src/filesystem/primitives/windows/mod.rs @@ -1,6 +1,5 @@ mod create_dir_unchecked; mod create_file_at_w; -mod dir_entry_inner; mod dir_utils; mod file_type_ext; mod get_path; @@ -10,7 +9,7 @@ mod oflags; mod open_fast; mod open_options_ext; mod open_unchecked; -mod read_dir_inner; +mod read_dir; mod read_link; mod read_link_unchecked; mod remove_dir_unchecked; @@ -23,7 +22,6 @@ mod symlink_unchecked; pub(crate) mod errors; pub(crate) use create_dir_unchecked::*; -pub(crate) use dir_entry_inner::*; pub(crate) use dir_utils::*; pub(crate) use file_type_ext::*; pub(crate) use hard_link_unchecked::*; @@ -31,7 +29,7 @@ pub(crate) use metadata_ext::*; pub(crate) use open_fast::*; pub(crate) use open_options_ext::*; pub(crate) use open_unchecked::*; -pub(crate) use read_dir_inner::*; +pub(crate) use read_dir::*; pub(crate) use read_link::*; pub(crate) use read_link_unchecked::*; pub(crate) use remove_dir_unchecked::*; diff --git a/crates/wasi/src/filesystem/primitives/windows/read_dir.rs b/crates/wasi/src/filesystem/primitives/windows/read_dir.rs new file mode 100644 index 000000000000..80dcc0a86334 --- /dev/null +++ b/crates/wasi/src/filesystem/primitives/windows/read_dir.rs @@ -0,0 +1,18 @@ +use super::get_path::concatenate; +use crate::filesystem::primitives::FileType; +use crate::filesystem::primitives::windows::ImplFileTypeExt; +use std::ffi::OsString; +use std::path::Component; +use std::{fs, io}; + +pub(crate) fn read_dir( + file: &fs::File, +) -> io::Result> + 'static> { + let full_path = concatenate(file, Component::CurDir.as_ref())?; + let iter = fs::read_dir(full_path)?; + Ok(iter.map(|entry| { + let entry = entry?; + let file_type = ImplFileTypeExt::from_std(entry.file_type()?); + Ok((entry.file_name(), file_type)) + })) +} diff --git a/crates/wasi/src/filesystem/primitives/windows/read_dir_inner.rs b/crates/wasi/src/filesystem/primitives/windows/read_dir_inner.rs deleted file mode 100644 index 5d8941804cf6..000000000000 --- a/crates/wasi/src/filesystem/primitives/windows/read_dir_inner.rs +++ /dev/null @@ -1,41 +0,0 @@ -use super::get_path::concatenate; -use crate::filesystem::primitives::DirEntryInner; -use std::path::{Component, Path}; -use std::{fmt, fs, io}; - -pub(crate) struct ReadDirInner { - std: fs::ReadDir, -} - -impl ReadDirInner { - pub(crate) fn read_base_dir(start: &fs::File) -> io::Result { - Self::new_unchecked(&start, Component::CurDir.as_ref()) - } - - pub(crate) fn new_unchecked(start: &fs::File, path: &Path) -> io::Result { - let full_path = concatenate(start, path)?; - Ok(Self { - std: fs::read_dir(full_path)?, - }) - } -} - -impl Iterator for ReadDirInner { - type Item = io::Result; - - fn next(&mut self) -> Option { - self.std - .next() - .map(|result| result.map(DirEntryInner::from_std)) - } -} - -impl fmt::Debug for ReadDirInner { - // Like libstd's version, but doesn't print the path. - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut b = f.debug_struct("ReadDir"); - // `fs::ReadDir`'s `Debug` just prints the path, and since we're not - // printing that, we don't have anything else to print. - b.finish() - } -} diff --git a/crates/wasi/src/p2/host/filesystem.rs b/crates/wasi/src/p2/host/filesystem.rs index db42d4495cb6..56e9fda91607 100644 --- a/crates/wasi/src/p2/host/filesystem.rs +++ b/crates/wasi/src/p2/host/filesystem.rs @@ -171,13 +171,11 @@ impl HostDescriptor for WasiFilesystemCtxView<'_> { // within this `block` call, rather than delay calculating the metadata // for entries when they're demanded later in the iterator chain. Ok::<_, std::io::Error>( - crate::filesystem::primitives::read_base_dir(d)? + crate::filesystem::primitives::read_dir(d)? .map(|entry| { - let entry = entry?; - let meta = entry.metadata()?; - let type_ = descriptortype_from(meta.file_type()); - let name = entry - .file_name() + let (filename, ty) = entry?; + let type_ = descriptortype_from(ty); + let name = filename .into_string() .map_err(|_| ReaddirError::IllegalSequence)?; Ok(types::DirectoryEntry { type_, name }) diff --git a/crates/wasi/src/p3/filesystem/host.rs b/crates/wasi/src/p3/filesystem/host.rs index fbbceef7408e..92f30d198907 100644 --- a/crates/wasi/src/p3/filesystem/host.rs +++ b/crates/wasi/src/p3/filesystem/host.rs @@ -11,6 +11,7 @@ use bytes::BytesMut; use core::pin::Pin; use core::task::{Context, Poll, ready}; use core::{iter, mem}; +use std::ffi::OsString; use std::io; use std::sync::Arc; use std::time::SystemTime; @@ -204,16 +205,15 @@ impl StreamProducer for ReadStreamProducer { } fn map_dir_entry( - entry: std::io::Result, + entry: std::io::Result<(OsString, crate::filesystem::primitives::FileType)>, ) -> Result, ErrorCode> { match entry { - Ok(entry) => { - let meta = entry.metadata()?; - let Ok(name) = entry.file_name().into_string() else { + Ok((filename, ty)) => { + let Ok(name) = filename.into_string() else { return Err(ErrorCode::IllegalByteSequence); }; Ok(Some(DirectoryEntry { - type_: meta.file_type().into(), + type_: ty.into(), name, })) } @@ -250,7 +250,7 @@ impl ReadDirStream { let (tx, rx) = mpsc::channel(1); ReadDirStream { task: spawn_blocking(move || { - let entries = crate::filesystem::primitives::read_base_dir(&dir)?; + let entries = crate::filesystem::primitives::read_dir(&dir)?; for entry in entries { if let Some(entry) = map_dir_entry(entry)? { if let Err(_) = tx.blocking_send(entry) { @@ -541,7 +541,7 @@ fn read_directory( let allow_blocking_current_thread = dir.allow_blocking_current_thread; let dir = Arc::clone(dir.as_dir()); if allow_blocking_current_thread { - match crate::filesystem::primitives::read_base_dir(&dir) { + match crate::filesystem::primitives::read_dir(&dir) { Ok(readdir) => StreamReader::new( &mut store, FallibleIteratorProducer::new(