Skip to content

Commit

Permalink
Stabilize Path::try_exists() and improve doc
Browse files Browse the repository at this point in the history
This stabilizes the `Path::try_exists()` method which returns
`Result<bool, io::Error>` instead of `bool` allowing handling of errors
unrelated to the file not existing. (e.g permission errors)

Along with the stabilization it also:

* Warns that the `exists()` method is error-prone and suggests to use
  the newly stabilized one.
* Suggests it instead of `metadata()` to handle errors.
* Mentions TOCTOU bugs to avoid false assumption that `try_exists()` is
  completely safe fixed version of `exists()`.
* Renames the feature of still-unstable `std::fs::try_exists()` to
  `fs_try_exists` to avoid name conflict.

The tracking issue #83186 remains open to track `fs_try_exists`.
  • Loading branch information
Kixunil committed Jun 14, 2022
1 parent edab34a commit 5608707
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 9 deletions.
1 change: 0 additions & 1 deletion compiler/rustc_error_messages/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#![feature(let_chains)]
#![feature(once_cell)]
#![feature(path_try_exists)]
#![feature(rustc_attrs)]
#![feature(type_alias_impl_trait)]

Expand Down
8 changes: 6 additions & 2 deletions library/std/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2317,10 +2317,14 @@ impl AsInnerMut<fs_imp::DirBuilder> for DirBuilder {
/// unrelated to the path not existing. (E.g. it will return `Err(_)` in case of permission
/// denied on some of the parent directories.)
///
/// Note that while this avoids some pitfalls of the `exists()` method, it still can not
/// prevent time-of-check to time-of-use (TOCTOU) bugs. You should only use it in scenarios
/// where those bugs are not an issue.
///
/// # Examples
///
/// ```no_run
/// #![feature(path_try_exists)]
/// #![feature(fs_try_exists)]
/// use std::fs;
///
/// assert!(!fs::try_exists("does_not_exist.txt").expect("Can't check existence of file does_not_exist.txt"));
Expand All @@ -2330,7 +2334,7 @@ impl AsInnerMut<fs_imp::DirBuilder> for DirBuilder {
/// [`Path::exists`]: crate::path::Path::exists
// FIXME: stabilization should modify documentation of `exists()` to recommend this method
// instead.
#[unstable(feature = "path_try_exists", issue = "83186")]
#[unstable(feature = "fs_try_exists", issue = "83186")]
#[inline]
pub fn try_exists<P: AsRef<Path>>(path: P) -> io::Result<bool> {
fs_imp::try_exists(path.as_ref())
Expand Down
17 changes: 11 additions & 6 deletions library/std/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2705,6 +2705,9 @@ impl Path {

/// Returns `true` if the path points at an existing entity.
///
/// Warning: this method may be error-prone, consider using [`try_exists()`] instead!
/// It also has a risk of introducing time-of-check to time-of-use (TOCTOU) bugs.
///
/// This function will traverse symbolic links to query information about the
/// destination file.
///
Expand All @@ -2721,7 +2724,9 @@ impl Path {
/// # See Also
///
/// This is a convenience function that coerces errors to false. If you want to
/// check errors, call [`fs::metadata`].
/// check errors, call [`Path::try_exists`].
///
/// [`try_exists()`]: Self::try_exists
#[stable(feature = "path_ext", since = "1.5.0")]
#[must_use]
#[inline]
Expand All @@ -2738,20 +2743,20 @@ impl Path {
/// unrelated to the path not existing. (E.g. it will return `Err(_)` in case of permission
/// denied on some of the parent directories.)
///
/// Note that while this avoids some pitfalls of the `exists()` method, it still can not
/// prevent time-of-check to time-of-use (TOCTOU) bugs. You should only use it in scenarios
/// where those bugs are not an issue.
///
/// # Examples
///
/// ```no_run
/// #![feature(path_try_exists)]
///
/// use std::path::Path;
/// assert!(!Path::new("does_not_exist.txt").try_exists().expect("Can't check existence of file does_not_exist.txt"));
/// assert!(Path::new("/root/secret_file.txt").try_exists().is_err());
/// ```
///
/// [`exists()`]: Self::exists
// FIXME: stabilization should modify documentation of `exists()` to recommend this method
// instead.
#[unstable(feature = "path_try_exists", issue = "83186")]
#[stable(feature = "path_try_exists", since = "1.63.0")]
#[inline]
pub fn try_exists(&self) -> io::Result<bool> {
fs::try_exists(self)
Expand Down

0 comments on commit 5608707

Please sign in to comment.