Skip to content

dirfd: preliminary unix and windows implementations #139514

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
244 changes: 244 additions & 0 deletions library/std/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,33 @@ pub enum TryLockError {
WouldBlock,
}

/// An object providing access to a directory on the filesystem.
///
/// Files are automatically closed when they go out of scope. Errors detected
/// on closing are ignored by the implementation of `Drop`.
///
/// # Examples
///
/// Opens a directory and then a file inside it.
///
/// ```no_run
/// #![feature(dirfd)]
/// use std::{fs::Dir, io::Read};
///
/// fn main() -> std::io::Result<()> {
/// let dir = Dir::new("foo")?;
/// let mut file = dir.open("bar.txt")?;
/// let mut s = String::new();
/// file.read_to_string(&mut s)?;
/// println!("{}", s);
/// Ok(())
/// }
/// ```
#[unstable(feature = "dirfd", issue = "120426")]
pub struct Dir {
inner: fs_imp::Dir,
}

/// Metadata information about a file.
///
/// This structure is returned from the [`metadata`] or
Expand Down Expand Up @@ -1453,6 +1480,223 @@ impl Seek for Arc<File> {
}
}

impl Dir {
/// Attempts to open a directory at `path` in read-only mode.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we should guarantee read-only. Directories aren't entirely consistent across unixes.
They different flavors of O_EXEC, O_SEARCH or O_PATH to indicate to open directories for traversal, not necessarily for readdir.

That kind of access can be sufficient for openat for example to traverse deeper into a directory with 0o100 permissions.

Either we can try read access and fallback to traversal-only or always try traversal-only and require the use of OpenOptions::read(true) to make that request.

Currently OpenOptions has no way to express those O_EXEC&Co. except via platform-specifc extension traits.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have tips to find which flag it is for each unix?

I think requiring OpenOptions::read(true) is the better option. Trying for read access and then falling back would either lead to a more complex api (to communicate which method succeeded) or ambiguous results (if we don't communicate that). There might also be a TOCTOU risk, but I'm not sure of that.

Might there also be potential for confusion when refactoring from open to open_with? Is it worth adding a function like open_for_traversal which uses O_EXEC or similar when available and delegates to open otherwise?

Finally, is this blocking? It seems more important that I finish the DirEntry api, and I suspect this is only one of several aspects that face bikeshedding before or during an FCP.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have tips to find which flag it is for each unix?

Generally by looking at their manpages.

Finally, is this blocking?

Well, if you don't want to change the implementation in this PR at least leave an internal note and relax the documentation.

///
/// See [`new_with`] for more options.
///
/// # Errors
///
/// This function will return an error if `path` does not point to an existing directory.
/// Other errors may also be returned according to [`OpenOptions::open`].
///
/// # Examples
///
/// ```no_run
/// #![feature(dirfd)]
/// use std::{fs::Dir, io::Read};
///
/// fn main() -> std::io::Result<()> {
/// let dir = Dir::new("foo")?;
/// let mut f = dir.open("bar.txt")?;
/// let mut data = vec![];
/// f.read_to_end(&mut data)?;
/// Ok(())
/// }
/// ```
///
/// [`new_with`]: Dir::new_with
#[unstable(feature = "dirfd", issue = "120426")]
pub fn new<P: AsRef<Path>>(path: P) -> io::Result<Self> {
Ok(Self { inner: fs_imp::Dir::new(path)? })
}

/// Attempts to open a directory at `path` with the options specified by `opts`.
///
/// # Errors
///
/// This function may return an error according to [`OpenOptions::open`].
///
/// # Examples
///
/// ```no_run
/// #![feature(dirfd)]
/// use std::fs::{Dir, OpenOptions};
///
/// fn main() -> std::io::Result<()> {
/// let dir = Dir::new_with("foo", OpenOptions::new().write(true))?;
/// let mut f = dir.remove_file("bar.txt")?;
/// Ok(())
/// }
/// ```
#[unstable(feature = "dirfd", issue = "120426")]
pub fn new_with<P: AsRef<Path>>(path: P, opts: &OpenOptions) -> io::Result<Self> {
Ok(Self { inner: fs_imp::Dir::new_with(path, &opts.0)? })
}

/// Attempts to open a file in read-only mode relative to this directory.
///
/// # Errors
///
/// This function will return an error if `path` does not point to an existing file.
/// Other errors may also be returned according to [`OpenOptions::open`].
///
/// # Examples
///
/// ```no_run
/// #![feature(dirfd)]
/// use std::{fs::Dir, io::Read};
///
/// fn main() -> std::io::Result<()> {
/// let dir = Dir::new("foo")?;
/// let mut f = dir.open("bar.txt")?;
/// let mut data = vec![];
/// f.read_to_end(&mut data)?;
/// Ok(())
/// }
/// ```
#[unstable(feature = "dirfd", issue = "120426")]
pub fn open<P: AsRef<Path>>(&self, path: P) -> io::Result<File> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This gives us a File, but for traversal one might need a child-Dir. So we'll need another method for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be in addition to create_dir()?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create_dir seems unrelated? I'm talking about something like open_dir(&self, path: P) -> Result<Dir> that opens an existing subdirectory. It's closer to new(), but takes a self.

self.inner.open(path).map(|f| File { inner: f })
}

/// Attempts to open a file relative to this directory with the options specified by `opts`.
///
/// # Errors
///
/// This function may return an error according to [`OpenOptions::open`].
///
/// # Examples
///
/// ```no_run
/// #![feature(dirfd)]
/// use std::{fs::{Dir, OpenOptions}, io::Read};
///
/// fn main() -> std::io::Result<()> {
/// let dir = Dir::new("foo")?;
/// let mut f = dir.open_with("bar.txt", OpenOptions::new().read(true))?;
/// let mut data = vec![];
/// f.read_to_end(&mut data)?;
/// Ok(())
/// }
/// ```
#[unstable(feature = "dirfd", issue = "120426")]
pub fn open_with<P: AsRef<Path>>(&self, path: P, opts: &OpenOptions) -> io::Result<File> {
self.inner.open_with(path, &opts.0).map(|f| File { inner: f })
}

/// Attempts to create a directory relative to this directory.
///
/// # Errors
///
/// This function will return an error if `path` points to an existing file or directory.
/// Other errors may also be returned according to [`OpenOptions::open`].
///
/// # Examples
///
/// ```no_run
/// #![feature(dirfd)]
/// use std::{fs::{Dir, OpenOptions}, io::Read};
///
/// fn main() -> std::io::Result<()> {
/// let dir = Dir::new("foo")?;
/// let mut f = dir.open_with("bar.txt", OpenOptions::new().read(true))?;
/// let mut data = vec![];
/// f.read_to_end(&mut data)?;
/// Ok(())
/// }
/// ```
#[unstable(feature = "dirfd", issue = "120426")]
pub fn create_dir<P: AsRef<Path>>(&self, path: P) -> io::Result<()> {
self.inner.create_dir(path)
}

/// Attempts to remove a file relative to this directory.
///
/// # Errors
///
/// This function will return an error if `path` does not point to an existing file.
/// Other errors may also be returned according to [`OpenOptions::open`].
///
/// # Examples
///
/// ```no_run
/// #![feature(dirfd)]
/// use std::fs::Dir;
///
/// fn main() -> std::io::Result<()> {
/// let dir = Dir::new("foo")?;
/// dir.remove_file("bar.txt")?;
/// Ok(())
/// }
/// ```
#[unstable(feature = "dirfd", issue = "120426")]
pub fn remove_file<P: AsRef<Path>>(&self, path: P) -> io::Result<()> {
self.inner.remove_file(path)
}

/// Attempts to remove a directory relative to this directory.
///
/// # Errors
///
/// This function will return an error if `path` does not point to an existing, non-empty directory.
/// Other errors may also be returned according to [`OpenOptions::open`].
///
/// # Examples
///
/// ```no_run
/// #![feature(dirfd)]
/// use std::fs::Dir;
///
/// fn main() -> std::io::Result<()> {
/// let dir = Dir::new("foo")?;
/// dir.remove_dir("baz")?;
/// Ok(())
/// }
/// ```
#[unstable(feature = "dirfd", issue = "120426")]
pub fn remove_dir<P: AsRef<Path>>(&self, path: P) -> io::Result<()> {
self.inner.remove_dir(path)
}

/// Attempts to rename a file or directory relative to this directory to a new name, replacing
/// the destination file if present.
///
/// # Errors
///
/// This function will return an error if `from` does not point to an existing file or directory.
/// Other errors may also be returned according to [`OpenOptions::open`].
///
/// # Examples
///
/// ```no_run
/// #![feature(dirfd)]
/// use std::fs::Dir;
///
/// fn main() -> std::io::Result<()> {
/// let dir = Dir::new("foo")?;
/// dir.rename("bar.txt", &dir, "quux.txt")?;
/// Ok(())
/// }
/// ```
#[unstable(feature = "dirfd", issue = "120426")]
pub fn rename<P: AsRef<Path>, Q: AsRef<Path>>(
&self,
from: P,
to_dir: &Self,
to: Q,
) -> io::Result<()> {
self.inner.rename(from, &to_dir.inner, to)
}
}

#[unstable(feature = "dirfd", issue = "120426")]
impl fmt::Debug for Dir {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
self.inner.fmt(f)
}
}

impl OpenOptions {
/// Creates a blank new set of options ready for configuration.
///
Expand Down
93 changes: 92 additions & 1 deletion library/std/src/fs/tests.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use rand::RngCore;

use super::Dir;
#[cfg(any(
windows,
target_os = "freebsd",
Expand All @@ -17,7 +18,7 @@ use crate::char::MAX_LEN_UTF8;
target_vendor = "apple",
))]
use crate::fs::TryLockError;
use crate::fs::{self, File, FileTimes, OpenOptions};
use crate::fs::{self, File, FileTimes, OpenOptions, create_dir};
use crate::io::prelude::*;
use crate::io::{BorrowedBuf, ErrorKind, SeekFrom};
use crate::mem::MaybeUninit;
Expand Down Expand Up @@ -2084,3 +2085,93 @@ fn test_rename_junction() {
// Junction links are always absolute so we just check the file name is correct.
assert_eq!(fs::read_link(&dest).unwrap().file_name(), Some(not_exist.as_os_str()));
}

#[test]
fn test_dir_smoke_test() {
let tmpdir = tmpdir();
check!(Dir::new(tmpdir.path()));
}

#[test]
fn test_dir_read_file() {
let tmpdir = tmpdir();
let mut f = check!(File::create(tmpdir.join("foo.txt")));
check!(f.write(b"bar"));
check!(f.flush());
drop(f);
let dir = check!(Dir::new(tmpdir.path()));
let mut f = check!(dir.open("foo.txt"));
let mut buf = [0u8; 3];
check!(f.read_exact(&mut buf));
assert_eq!(b"bar", &buf);
}

#[test]
fn test_dir_write_file() {
let tmpdir = tmpdir();
let dir = check!(Dir::new(tmpdir.path()));
let mut f = check!(dir.open_with("foo.txt", &OpenOptions::new().write(true).create(true)));
check!(f.write(b"bar"));
check!(f.flush());
drop(f);
let mut f = check!(File::open(tmpdir.join("foo.txt")));
let mut buf = [0u8; 3];
check!(f.read_exact(&mut buf));
assert_eq!(b"bar", &buf);
}

#[test]
fn test_dir_remove_file() {
let tmpdir = tmpdir();
let mut f = check!(File::create(tmpdir.join("foo.txt")));
check!(f.write(b"bar"));
check!(f.flush());
drop(f);
let dir = check!(Dir::new(tmpdir.path()));
check!(dir.remove_file("foo.txt"));
let result = File::open(tmpdir.join("foo.txt"));
#[cfg(all(unix, not(target_os = "vxworks")))]
error!(result, "No such file or directory");
#[cfg(target_os = "vxworks")]
error!(result, "no such file or directory");
#[cfg(windows)]
error!(result, 2); // ERROR_FILE_NOT_FOUND
}

#[test]
fn test_dir_remove_dir() {
let tmpdir = tmpdir();
check!(create_dir(tmpdir.join("foo")));
let dir = check!(Dir::new(tmpdir.path()));
check!(dir.remove_dir("foo"));
let result = Dir::new(tmpdir.join("foo"));
#[cfg(all(unix, not(target_os = "vxworks")))]
error!(result, "No such file or directory");
#[cfg(target_os = "vxworks")]
error!(result, "no such file or directory");
#[cfg(windows)]
error!(result, 2); // ERROR_FILE_NOT_FOUND
}

#[test]
fn test_dir_rename_file() {
let tmpdir = tmpdir();
let mut f = check!(File::create(tmpdir.join("foo.txt")));
check!(f.write(b"bar"));
check!(f.flush());
drop(f);
let dir = check!(Dir::new(tmpdir.path()));
check!(dir.rename("foo.txt", &dir, "baz.txt"));
let mut f = check!(File::open(tmpdir.join("baz.txt")));
let mut buf = [0u8; 3];
check!(f.read_exact(&mut buf));
assert_eq!(b"bar", &buf);
}

#[test]
fn test_dir_create_dir() {
let tmpdir = tmpdir();
let dir = check!(Dir::new(tmpdir.path()));
check!(dir.create_dir("foo"));
check!(Dir::new(tmpdir.join("foo")));
}
Loading
Loading