From 7247a8b6ee59fc99bbb69ca6b3ca4bfd8c809ead Mon Sep 17 00:00:00 2001 From: Robert Collins Date: Fri, 24 Feb 2023 11:09:55 +0100 Subject: [PATCH] Merge pull request from GHSA-mc8h-8q98-g5hr * Fix TOCTOU races This involves a new safe API that works from a file descriptor rather than a path. The existing APIs are preserved but no longer recommended. * Bump version to indicate API breaks The crate features have changed, in particular parallel is now not defaulted-on. * Rearrange changelog --- CHANGELOG.md | 67 +++++++- Cargo.toml | 39 +++-- src/_impl.rs | 157 ++++++++++++++++++ src/_impl/io.rs | 27 +++ src/_impl/path_components.rs | 22 +++ src/_impl/unix.rs | 53 ++++++ src/_impl/win.rs | 190 +++++++++++++++++++++ src/fs.rs | 165 ------------------ src/lib.rs | 312 +++++++++++++++++++++++++++++++---- src/portable.rs | 133 --------------- src/unix.rs | 21 --- tests/interface.rs | 232 ++++++++++++++++++++++++++ tests/windows.rs | 100 ----------- 13 files changed, 1052 insertions(+), 466 deletions(-) create mode 100644 src/_impl.rs create mode 100644 src/_impl/io.rs create mode 100644 src/_impl/path_components.rs create mode 100644 src/_impl/unix.rs create mode 100644 src/_impl/win.rs delete mode 100644 src/fs.rs delete mode 100644 src/portable.rs delete mode 100644 src/unix.rs create mode 100644 tests/interface.rs delete mode 100644 tests/windows.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index ed3fe2c..c2bed6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,68 @@ -# unreleased +# 0.8.0 -- Added feature to enable use of rayon. -- Migrated from winapi to windows-sys +## Security changes + +- Fix TOCTOU race conditions both inside the implementation of functions and the + contract: functions now only operate on directories. Callers wanting to + process the contents of a symlink (e.g. for remove_dir_contents) should + resolve the symlink themselves. This is an API break from 0.7.0, but the previous behaviour was insecure. + + This is due to the same code pattern as caused CVE-2022-21658 in Rust itself: + it was possible to trick a privileged process doing a recursive delete in an + attacker controlled directory into deleting privileged files, on all operating + systems. + + For instance, consider deleting a tree called 'etc' in a parent directory + called 'p'. Between calling `remove_dir_all("a")` and remove_dir_all("a") + actually starting its work, the attacker can move 'p' to 'p-prime', and + replace 'p' with a symlink to '/'. Then the privileged process deletes 'p/etc' + which is actually /etc, and now your system is broken. There are some + mitigations for this exact scenario, such as CWD relative file lookup, but + they are not guaranteed - any code using absolute paths will not have that + protection in place. + + The same attack could be performed at any point in the directory tree being + deleted: if 'a' contains a child directory called 'etc', attacking the + deletion by replacing 'a' with a link is possible. + + The new code in this release mitigates the attack within the directory tree + being deleted by using file-handle relative operations: to open 'a/etc', the + path 'etc' relative to 'a' is opened, where 'a' is represented by a file + descriptor (Unix) or handle (Windows). With the exception of the entry points + into the directory deletion logic, this is robust against manipulation of the + directory hierarchy, and remove_dir_all will only delete files and directories + contained in the tree it is deleting. + + The entry path however is a challenge - as described above, there are some + potential mitigations, but since using them must be done by the calling code, + it is hard to be confident about the security properties of the path based + interface. + + The new extension trait `RemoveDir` provides an interface where it is much + harder to get it wrong. + + `somedir.remove_dir_contents("name-of-child")`. + + Callers can then make their own security evaluation about how to securely get + a directory handle. That is still not particularly obvious, and we're going to + follow up with a helper of some sort (probably in the `fs_at` crate). Once + that is available, the path based entry points will get deprecated. + + In the interim, processes that might run with elevated privileges should + figure out how to securely identify the directory they are going to delete, to + avoid the initial race. Pragmatically, other processes should be fine with the + path based entry points : this is the same interface `std::fs::remove_dir_all` + offers, and an unprivileged process running in an attacker controlled + directory can't do anything that the attacker can't already do. + + tl;dr: state shared with threat actors makes things dangerous; library + functions cannot assume anything about the particular threat model of a + program and must err on the side of caution. + +## Other changes + +- Made feature to control use of rayon off-by-default for easier integration by + other crates. # 0.5.2 diff --git a/Cargo.toml b/Cargo.toml index 9a6b89d..78c43d7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,48 +1,59 @@ [package] authors = [ - "Erin P. ", - "Robert C. ", + "Erin P. ", + "Robert C. ", ] categories = ["filesystem"] description = "A safe, reliable implementation of remove_dir_all for Windows" edition = "2018" include = [ - "Cargo.toml", - "LICENCE-APACHE", - "LICENCE-MIT", - "src/**/*", - "README.md", + "Cargo.toml", + "LICENCE-APACHE", + "LICENCE-MIT", + "src/**/*", + "README.md", ] keywords = ["utility", "filesystem", "remove_dir", "windows"] license = "MIT OR Apache-2.0" name = "remove_dir_all" readme = "README.md" repository = "https://github.com/XAMPPRocky/remove_dir_all.git" -version = "0.7.1-alpha.0" +version = "0.8.0-alpha.0" [features] -default = ["parallel"] -parallel = ["rayon"] +default = [] +log = ["dep:log"] +parallel = ["dep:rayon"] [dependencies] cfg-if = "1.0.0" +fs_at = { version = "0.1" } +lazy_static = "1.4.0" +log = { version = "0.4.11", optional = true } +normpath = "1.0.1" +rayon = { version = "1.4", optional = true } [target.'cfg(windows)'.dependencies] -log = "0.4.11" -rayon = { version = "1.4", optional = true } +aligned = "0.4.1" [target.'cfg(windows)'.dependencies.windows-sys] -version = "0.45.0" features = [ "Win32_Foundation", "Win32_Storage_FileSystem", + "Win32_System_IO", + "Win32_System_Ioctl", "Win32_System_SystemServices", + "Win32_System_Threading", ] +version = "0.45.0" [target.'cfg(not(windows))'.dependencies] libc = "0.2" +cvt = "0.1.1" [dev-dependencies] doc-comment = "0.3" -env_logger = "0.9.0" +env_logger = "0.10.0" tempfile = "3.1" +test-log = "0.2" +log = "0.4.11" \ No newline at end of file diff --git a/src/_impl.rs b/src/_impl.rs new file mode 100644 index 0000000..8d83a67 --- /dev/null +++ b/src/_impl.rs @@ -0,0 +1,157 @@ +use std::{ + ffi::OsStr, + fs::File, + io::{ErrorKind, Result}, + path::Path, +}; + +#[cfg(feature = "parallel")] +use rayon::prelude::*; + +mod io; +mod path_components; + +cfg_if::cfg_if! { + if #[cfg(windows)] { + mod win; + pub(crate) use win::WindowsIo as OsIo; + } else { + mod unix; + pub(crate) use unix::UnixIo as OsIo; + } +} + +impl super::RemoveDir for std::fs::File { + fn remove_dir_contents(&mut self, debug_root: Option<&Path>) -> Result<()> { + // thunk over to the free version adding in the os-specific IO trait impl + let debug_root = match debug_root { + None => PathComponents::Path(Path::new("")), + Some(debug_root) => PathComponents::Path(debug_root), + }; + _remove_dir_contents::(self, &debug_root) + } +} + +/// Entry point for deprecated function +pub(crate) fn _ensure_empty_dir_path>(path: P) -> Result<()> { + // This is as TOCTOU safe as we can make it. Attacks via link replacements + // in interior components of the path is still possible. if the create + // succeeds, mission accomplished. if the create fails, open the dir + // (subject to races again), and then proceed to delete the contents via the + // descriptor. + match std::fs::create_dir(&path) { + Err(e) if e.kind() == ErrorKind::AlreadyExists => { + // Exists and is a dir. Open it + let mut existing_dir = I::open_dir(path.as_ref())?; + existing_dir.remove_dir_contents(Some(path.as_ref())) + } + otherwise => otherwise, + } +} + +// Deprecated entry point +pub(crate) fn _remove_dir_contents_path>(path: P) -> Result<()> { + let mut d = I::open_dir(path.as_ref())?; + _remove_dir_contents::(&mut d, &PathComponents::Path(path.as_ref())) +} + +/// exterior lifetime interface to dir removal +fn _remove_dir_contents(d: &mut File, debug_root: &PathComponents<'_>) -> Result<()> { + let owned_handle = I::duplicate_fd(d)?; + remove_dir_contents_recursive::(owned_handle, debug_root) +} + +/// deprecated interface +pub(crate) fn remove_dir_all_path>(path: P) -> Result<()> { + let p = path.as_ref(); + // Opportunity 1 for races + let d = I::open_dir(p)?; + let debug_root = PathComponents::Path(if p.has_root() { p } else { Path::new(".") }); + remove_dir_contents_recursive::(d, &debug_root)?; + // Opportunity 2 for races + std::fs::remove_dir(&path) +} + +use crate::RemoveDir; + +use self::path_components::PathComponents; + +// Core workhorse, heading towards this being able to be tasks. +#[allow(clippy::map_identity)] +fn remove_dir_contents_recursive( + mut d: File, + debug_root: &PathComponents<'_>, +) -> Result<()> { + #[cfg(feature = "log")] + log::trace!("scanning {}", &debug_root); + // We take a os level clone of the FD so that there are no lifetime + // concerns. It would *not* be ok to do readdir on one file twice + // concurrently because of shared kernel state. + let dirfd = I::duplicate_fd(&mut d)?; + cfg_if::cfg_if! { + if #[cfg(feature = "parallel")] { + let iter = fs_at::read_dir(&mut d)?; + let iter = iter.par_bridge(); + } else { + let mut iter = fs_at::read_dir(&mut d)?; + } + } + + iter.try_for_each(|dir_entry| -> Result<()> { + let dir_entry = dir_entry?; + let name = dir_entry.name(); + if name == OsStr::new(".") || name == OsStr::new("..") { + return Ok(()); + } + let dir_path = Path::new(name); + let dir_debug_root = PathComponents::Component(debug_root, dir_path); + // Windows optimised: open everything always, which is not bad for + // linux, and portable to OS's and FS's that don't expose inode type in + // the readdir entries. + + let mut opts = fs_at::OpenOptions::default(); + opts.read(true) + .write(fs_at::OpenOptionsWriteMode::Write) + .follow(false); + + let child_result = opts.open_dir_at(&dirfd, name); + let is_dir = match child_result { + Err(e) if !I::is_eloop(&e) => return Err(e), + Err(_) => false, + Ok(child_file) => { + let metadata = child_file.metadata()?; + let is_dir = metadata.is_dir(); + I::clear_readonly(&child_file, &dir_debug_root, &metadata)?; + + if is_dir { + remove_dir_contents_recursive::(child_file, &dir_debug_root)?; + #[cfg(feature = "log")] + log::trace!("rmdir: {}", &dir_debug_root); + let opts = fs_at::OpenOptions::default(); + opts.rmdir_at(&dirfd, name).map_err(|e| { + #[cfg(feature = "log")] + log::debug!("error removing {}", dir_debug_root); + e + })?; + } + is_dir + } + }; + if !is_dir { + #[cfg(feature = "log")] + log::trace!("unlink: {}", &dir_debug_root); + opts.unlink_at(&dirfd, name).map_err(|e| { + #[cfg(feature = "log")] + log::debug!("error removing {}", dir_debug_root); + e + })?; + } + + #[cfg(feature = "log")] + log::trace!("removed {}", dir_debug_root); + Ok(()) + })?; + #[cfg(feature = "log")] + log::trace!("scanned {}", &debug_root); + Ok(()) +} diff --git a/src/_impl/io.rs b/src/_impl/io.rs new file mode 100644 index 0000000..b8b1cfb --- /dev/null +++ b/src/_impl/io.rs @@ -0,0 +1,27 @@ +//! Private trait to deal with OS variance + +use std::{ + fmt::Debug, + fs::{File, Metadata}, + io, + path::Path, +}; + +use super::path_components::PathComponents; + +pub(crate) trait Io { + type UniqueIdentifier: PartialEq + Debug; + + fn duplicate_fd(f: &mut File) -> io::Result; + + fn open_dir(p: &Path) -> io::Result; + fn unique_identifier(d: &File) -> io::Result; + + fn clear_readonly( + f: &File, + dir_debug_root: &'_ PathComponents<'_>, + metadata: &Metadata, + ) -> io::Result<()>; + + fn is_eloop(e: &io::Error) -> bool; +} diff --git a/src/_impl/path_components.rs b/src/_impl/path_components.rs new file mode 100644 index 0000000..33450c9 --- /dev/null +++ b/src/_impl/path_components.rs @@ -0,0 +1,22 @@ +use std::{fmt::Display, path::Path}; + +/// Print a path that is broken into segments. +// explicitly typed to avoid type recursion. 'a is the smallest lifetime present +// : that of the child. +pub(crate) enum PathComponents<'a> { + Path(&'a Path), + Component(&'a PathComponents<'a>, &'a Path), +} + +impl<'p> Display for PathComponents<'p> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + PathComponents::Path(p) => p.display().fmt(f), + PathComponents::Component(p, c) => { + p.fmt(f)?; + f.write_str("/")?; + c.display().fmt(f) + } + } + } +} diff --git a/src/_impl/unix.rs b/src/_impl/unix.rs new file mode 100644 index 0000000..ca47eb4 --- /dev/null +++ b/src/_impl/unix.rs @@ -0,0 +1,53 @@ +use std::fs::{File, OpenOptions}; +use std::io; +use std::os::unix::fs::OpenOptionsExt; +use std::os::unix::prelude::FromRawFd; +use std::path::Path; +use std::{fs, os::unix::prelude::AsRawFd}; + +use cvt::cvt; +use libc::{self, fcntl, F_DUPFD_CLOEXEC}; + +use super::io::Io; + +pub(crate) struct UnixIo; + +impl Io for UnixIo { + type UniqueIdentifier = (); + + fn duplicate_fd(f: &mut fs::File) -> io::Result { + let source_fd = f.as_raw_fd(); + // F_DUPFD_CLOEXEC seems to be quite portable, but we should be prepared + // to add in more codepaths here. + let fd = cvt(unsafe { fcntl(source_fd, F_DUPFD_CLOEXEC) })?; + Ok(unsafe { File::from_raw_fd(fd) }) + } + + fn open_dir(p: &Path) -> io::Result { + let mut options = OpenOptions::new(); + options.read(true); + options.custom_flags(libc::O_NOFOLLOW); + options.open(p) + } + + fn unique_identifier(_d: &fs::File) -> io::Result { + todo!() + } + + fn clear_readonly( + _f: &fs::File, + _dir_debug_root: &'_ super::path_components::PathComponents<'_>, + _metadata: &fs::Metadata, + ) -> io::Result<()> { + // can't delete contents of a directory without 'w' on the directory, so + // you might expect to see logic here to check a directory. that said, + // remove_dir_all doesn't concern itself with permissions; it does + // concern itself with the readonly attribute on windows - but that is + // not a file permission. + Ok(()) + } + + fn is_eloop(e: &io::Error) -> bool { + e.raw_os_error() == Some(libc::ELOOP) + } +} diff --git a/src/_impl/win.rs b/src/_impl/win.rs new file mode 100644 index 0000000..8d01aa6 --- /dev/null +++ b/src/_impl/win.rs @@ -0,0 +1,190 @@ +use std::{ + ffi::c_void, + fs::{File, Metadata, OpenOptions}, + io::{self, Result}, + mem::{size_of, MaybeUninit}, + os::windows::fs::OpenOptionsExt, + os::windows::prelude::AsRawHandle, + os::windows::prelude::*, + path::Path, +}; + +use aligned::{Aligned, A8}; +use windows_sys::Win32::{ + Foundation::{ + DuplicateHandle, DUPLICATE_SAME_ACCESS, ERROR_CANT_RESOLVE_FILENAME, + ERROR_NOT_A_REPARSE_POINT, HANDLE, + }, + Storage::FileSystem::{ + FileBasicInfo, FileIdInfo, GetFileInformationByHandleEx, SetFileInformationByHandle, + FILE_ATTRIBUTE_NORMAL, FILE_BASIC_INFO, FILE_FLAG_BACKUP_SEMANTICS, + FILE_FLAG_OPEN_REPARSE_POINT, FILE_ID_INFO, MAXIMUM_REPARSE_DATA_BUFFER_SIZE, + REPARSE_GUID_DATA_BUFFER, + }, + System::{ + Ioctl::FSCTL_GET_REPARSE_POINT, + SystemServices::{IO_REPARSE_TAG_MOUNT_POINT, IO_REPARSE_TAG_SYMLINK}, + Threading::GetCurrentProcess, + IO::DeviceIoControl, + }, +}; + +use super::{io::Io, path_components::PathComponents}; + +pub(crate) struct WindowsIo; + +// basically FILE_ID_INFO but declared primitives to permit derives. +#[derive(Debug, PartialEq)] +pub(crate) struct VSNFileId { + vsn: u64, + file_id: [u8; 16], +} + +impl Io for WindowsIo { + fn duplicate_fd(f: &mut File) -> io::Result { + let mut new_handle: MaybeUninit<*mut c_void> = MaybeUninit::uninit(); + + let result = unsafe { + DuplicateHandle( + GetCurrentProcess(), + f.as_raw_handle() as HANDLE, + GetCurrentProcess(), + new_handle.as_mut_ptr() as *mut HANDLE, + 0, + false as i32, + DUPLICATE_SAME_ACCESS, + ) + }; + if result == 0 { + return Err(std::io::Error::last_os_error()); + } + + let new_handle = unsafe { new_handle.assume_init() }; + Ok(unsafe { File::from_raw_handle(new_handle) }) + } + + fn open_dir(p: &Path) -> Result { + let mut options = OpenOptions::new(); + options.read(true); + options.custom_flags(FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT); + let maybe_dir = options.open(p)?; + if is_symlink(maybe_dir.as_raw_handle() as HANDLE)? { + return Err(io::Error::new( + io::ErrorKind::Other, + "Path is a directory link, not directory", + )); + } + Ok(maybe_dir) + } + + type UniqueIdentifier = VSNFileId; + + fn unique_identifier(d: &File) -> io::Result { + let mut info = MaybeUninit::::uninit(); + let bool_result = unsafe { + GetFileInformationByHandleEx( + d.as_raw_handle() as HANDLE, + FileIdInfo, + info.as_mut_ptr() as *mut c_void, + size_of::() as u32, + ) + }; + if bool_result == 0 { + return Err(io::Error::last_os_error()); + } + let info = unsafe { info.assume_init() }; + Ok(VSNFileId { + vsn: info.VolumeSerialNumber, + file_id: info.FileId.Identifier, + }) + } + + fn clear_readonly( + f: &File, + _dir_debug_root: &'_ PathComponents<'_>, + metadata: &Metadata, + ) -> io::Result<()> { + if metadata.permissions().readonly() { + // TODO use the FileDispositionEx interface to avoid resetting at all. + #[cfg(feature = "log")] + log::trace!("clearing permissions: {}", &_dir_debug_root); + // set the readonly bit off. TODO: could read the times from the + // directory listing iterator metadata. But as we've been asked to + // delete, who cares? + let mut info = FILE_BASIC_INFO { + FileAttributes: FILE_ATTRIBUTE_NORMAL, + CreationTime: 0, + LastAccessTime: 0, + LastWriteTime: 0, + ChangeTime: 0, + }; + use std::ffi::c_void; + let result = unsafe { + SetFileInformationByHandle( + f.as_raw_handle() as HANDLE, + FileBasicInfo, + &mut info as *mut FILE_BASIC_INFO as *mut c_void, + size_of::() as u32, + ) + }; + if result == 0 { + Err(std::io::Error::last_os_error()) + } else { + Ok(()) + } + } else { + Ok(()) + } + } + + fn is_eloop(e: &io::Error) -> bool { + e.raw_os_error() == Some(ERROR_CANT_RESOLVE_FILENAME as i32) + } +} + +fn is_symlink(handle: HANDLE) -> Result { + let mut reparse_buffer: Aligned< + A8, + [MaybeUninit; MAXIMUM_REPARSE_DATA_BUFFER_SIZE as usize], + > = Aligned([MaybeUninit::::uninit(); MAXIMUM_REPARSE_DATA_BUFFER_SIZE as usize]); + let mut out_size = 0; + let bool_result = unsafe { + DeviceIoControl( + handle, + FSCTL_GET_REPARSE_POINT, + std::ptr::null(), + 0, + // output buffer + reparse_buffer.as_mut_ptr().cast(), + // size of output buffer + MAXIMUM_REPARSE_DATA_BUFFER_SIZE, + // number of bytes returned + &mut out_size, + // OVERLAPPED structure + std::ptr::null_mut(), + ) + }; + if bool_result == 0 { + let e = io::Error::last_os_error(); + if e.raw_os_error() != Some(ERROR_NOT_A_REPARSE_POINT as i32) { + return Err(e); + } else { + return Ok(false); + } + } + // Might be a non-link reparse point + if out_size < size_of::() as u32 { + // Success but not enough data to read the tag + return Err(io::Error::new( + io::ErrorKind::Other, + "Insufficient data from DeviceIOControl", + )); + } + let reparse_buffer = reparse_buffer.as_ptr().cast::(); + Ok(unsafe { + matches!( + (*reparse_buffer).ReparseTag, + IO_REPARSE_TAG_SYMLINK | IO_REPARSE_TAG_MOUNT_POINT + ) + }) +} diff --git a/src/fs.rs b/src/fs.rs deleted file mode 100644 index 219ebf1..0000000 --- a/src/fs.rs +++ /dev/null @@ -1,165 +0,0 @@ -use std::{ - fs::{self, OpenOptions}, - io, - ffi::c_void, - mem::size_of, - os::windows::prelude::*, - path::{Path, PathBuf}, -}; - -#[cfg(feature = "parallel")] -use rayon::prelude::*; -use windows_sys::Win32::Storage::FileSystem::*; -use windows_sys::Win32::Foundation::HANDLE; - -/// Reliably removes a directory and all of its children. -/// -/// ```rust -/// use std::fs; -/// use remove_dir_all::*; -/// -/// fs::create_dir("./temp/").unwrap(); -//// remove_dir_all("./temp/").unwrap(); -/// ``` -/// -/// On Windows it is not enough to just recursively remove the contents of a -/// directory and then the directory itself. Deleting does not happen -/// instantaneously, but is delayed by IO being completed in the fs stack and -/// then the last copy of the directory handle being closed. -/// -/// Further, typical Windows machines can handle many more concurrent IOs than a -/// single threaded application is capable of submitting: the overlapped (async) -/// calls available do not cover the operations needed to perform directory -/// removal efficiently. -/// -/// The `parallel` feature enables the use of a work stealing scheduler to -/// mitigate this limitation: that permits submitting deletions concurrently with -/// directory scanning, and delete sibling directories in parallel. This allows -/// the slight latency of STATUS_DELETE_PENDING to only have logarithmic effect: -/// a very deep tree will pay wall clock time for that overhead per level as the -/// tree traverse completes, but not linearly for every interior not as a simple -/// recursive deletion would result in. -/// -/// Earlier versions of this crate moved the contents of the directory being -/// deleted to become siblings of `base_dir`, which required write access to the -/// parent directory under all circumstances; this is no longer done, even when -/// the parallel feature is disabled -/// - though we could re-instate if in-use files turn out to be handled very -/// poorly with this new threaded implementation, or if many people find -/// threads more concerning that moving files outside of their directory -/// structure as part of deletion! -/// - As a result in-use file deletion now leaves files in-situ and blocks the -/// removal, when previously it would leave the file with a nonsense name -/// outside the dir - but not block the removal. Generally speaking -/// applications can choose when to close files, and they should arrange to do -/// so before deleting the directory. -/// -/// There is a single small race condition where external side effects may be -/// left: when deleting a hard linked readonly file, the syscalls required are: -/// - open -/// - set rw -/// - unlink (SetFileDispositionDelete) -/// - set ro -/// -/// A crash or power failure could lead to the loss of the readonly bit on the -/// hardlinked inode. -/// -/// To handle files with names like `CON` and `morse .. .`, and when a directory -/// structure is so deep it needs long path names the path is first converted to -/// the Win32 file namespace by calling `canonicalize()`. -pub fn remove_dir_all>(path: P) -> io::Result<()> { - let path = _remove_dir_contents(path)?; - let metadata = path.metadata()?; - if metadata.permissions().readonly() { - delete_readonly(metadata, &path)?; - } else { - log::trace!("removing {}", &path.display()); - fs::remove_dir(&path).map_err(|e| { - log::debug!("error removing {}", &path.display()); - e - })?; - log::trace!("removed {}", &path.display()); - } - Ok(()) -} - -/// Returns the canonicalised path, for one of caller's convenience. -pub fn _remove_dir_contents>(path: P) -> io::Result { - let path = path.as_ref().canonicalize()?; - _delete_dir_contents(&path)?; - Ok(path) -} - -fn _delete_dir_contents(path: &PathBuf) -> io::Result<()> { - log::trace!("scanning {}", &path.display()); - cfg_if::cfg_if! { - if #[cfg(feature = "parallel")] { - let iter = path.read_dir()?.par_bridge(); - } else { - let mut iter = path.read_dir()?; - } - } - iter.try_for_each(|dir_entry| -> io::Result<()> { - let dir_entry = dir_entry?; - let metadata = dir_entry.metadata()?; - let is_dir = dir_entry.file_type()?.is_dir(); - let dir_path = dir_entry.path(); - if is_dir { - _delete_dir_contents(&dir_path)?; - } - log::trace!("removing {}", &dir_path.display()); - if metadata.permissions().readonly() { - delete_readonly(metadata, &dir_path).map_err(|e| { - log::debug!("error removing {}", &dir_path.display()); - e - })?; - } else if is_dir { - fs::remove_dir(&dir_path).map_err(|e| { - log::debug!("error removing {}", &dir_path.display()); - e - })?; - } else { - fs::remove_file(&dir_path).map_err(|e| { - log::debug!("error removing {}", &dir_path.display()); - e - })?; - } - log::trace!("removed {}", &dir_path.display()); - Ok(()) - })?; - log::trace!("scanned {}", &path.display()); - Ok(()) -} - -// Delete a file or directory that is readonly -fn delete_readonly(metadata: fs::Metadata, path: &Path) -> io::Result<()> { - // Open, drop the readonly bit, set delete-on-close, close. - let mut opts = OpenOptions::new(); - opts.access_mode(DELETE | FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES); - opts.custom_flags(FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT); - - let file = opts.open(path)?; - let mut perms = metadata.permissions(); - perms.set_readonly(false); - file.set_permissions(perms)?; - - let mut info = FILE_DISPOSITION_INFO { - DeleteFile: true as u8, - }; - let result = unsafe { - SetFileInformationByHandle( - file.as_raw_handle() as HANDLE, - FileDispositionInfo, - &mut info as *mut FILE_DISPOSITION_INFO as *mut c_void, - size_of::() as u32, - ) - }; - - if result == 0 { - return Err(io::Error::last_os_error()); - } - - file.set_permissions(metadata.permissions())?; - drop(file); - Ok(()) -} diff --git a/src/lib.rs b/src/lib.rs index 9955d95..1393f04 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,32 +1,130 @@ //! Reliably remove a directory and all of its children. //! -//! This library provides a reliable implementation of `remove_dir_all` for -//! Windows. For Unix systems, it re-exports `std::fs::remove_dir_all`. -//! -//! It also provides `remove_dir_contents` and `ensure_empty_dir` for both Unix -//! and Windows. -//! -//! The crate has one feature, enabled by default: "parallel". When this is -//! enabled, deletion of directories proceeds in parallel. And yes, this is a -//! win. On the other hand some users may value build time or code size over -//! tree removal performance. Disable default features and a single threaded -//! implementation is used instead, still with the reliability and contention -//! avoiding features. To support others making this choice, when adding -//! tempfile as a dependency to a library crate, use a feature "parallel" and -//! add it with `default-features = false`. This will permit the user of your -//! library to control the parallel feature inside remove_dir_all : which they -//! may need to do if they also depend on remove_dir_all, and don't want to -//! build things twice with feature-resolver-version-2. +//! This library provides an alternative implementation of +//! `std::fs::remove_dir_all` from the Rust std library. It varies in the +//! following ways: +//! - the `parallel` feature parallelises the deletion. This is useful when high +//! syscall latency is occuring, such as on Windows (deletion IO accrues to +//! the process), or network file systems of any kind. This is off by default. +//! - It tolerates files not being deleted atomically (this is a Windows +//! specific behaviour). +//! - It resets the readonly flag on Windows as needed. +//! +//! Like `remove_dir_all` it assumes both that the caller has permission to +//! delete the files, and that they don't have permission to change permissions +//! to be able to delete the files: no ACL or chmod changes are made during +//! deletion. This is because hardlinks can cause such changes to show up and +//! affect the filesystem outside of the directory tree being deleted. +//! +//! The extension trait `RemoveDir` can be used to invoke `remove_dir_all` on +//! an open `File`, where it will error if the file is not a directory, and +//! otherwise delete the contents. This allows callers to be more confident that +//! what is deleted is what was requested even in the presence of malicious +//! actors changing the filesystem concurrently. +//! +//! The functions `remove_dir_all`, `remove_dir_contents` and `ensure_empty_dir` +//! are intrinsically sensitive to file system races, as the path to the +//! directory to delete can be substituted by an attacker inserting a symlink +//! along that path. Relative paths with one path component are the least +//! fragile, but using `RemoveDir::remove_dir_all` is recommended. +//! +//! ## Features +//! +//! - parallel: When enabled, deletion of directories is parallised. (#parallel)[more details] +//! - log: Include some log messages about the deletion taking place. +//! +//! About the implementation. The implementation prioritises security, then +//! robustness (e.g. low resource situations), and then finally performance. +//! +//! ## Security +//! +//! On all platforms directory related race conditions are avoided by opening +//! paths and then iterating directory contents and deleting names in the +//! directory with _at style syscalls. This does not entirely address possible +//! races on unix style operating systems (but see the funlinkat call on +//! FreeBSD, which could if more widely adopted). It does prevent attackers from +//! replacing intermediary directories with symlinks in order to fool privileged +//! code into traversing outside the intended directory tree. This is the same +//! as the standard library implementation. +//! +//! This function is not designed to succeed in the presence of concurrent +//! actors in the tree being deleted - for instance, adding files to a directory +//! being deleted can prevent the directory being deleted for an arbitrary +//! period by extending the directory iterator indefinitely. +//! +//! Directory traversal only ever happens downwards. In future, to accomodate +//! very large directory trees (greater than file descriptor limits deep) the +//! same path may be traversed multiple times, and the quadratic nature of that +//! will be mitigated by a cache of open directories. See [#future-plans](Future +//! Plans) +//! +//! ## Robustness +//! +//! Every opened file has its type checked through the file handle, and then +//! unlinked or scanned as appropriate. Syscall overheads are minimised by +//! trust-but-verify of the node type metadata returned from directory scanning: +//! only names that appear to be directories get their contents scanned. The +//! consequence is that if an attacker replaces a non-directory with a +//! directory, or vice versa, an error will occur - but the remove_dir_all will +//! not escape from the directory tree. On Windows file deletion requires +//! obtaining a handle to the file, but again the kind metadata from the +//! directory scan is used to avoid re-querying the metadata. Symlinks are +//! detected by a failure to open a path with O_NOFOLLOW, they are unlinked with +//! no further processing. +//! +//! ## Serial deletion +//! +//! Serial deletion occurs recursively - open, read, delete +//! contents-except-for-directories, repeat. +//! +//! Parallel deletion builds on serial deletion by utilising a thread pool for +//! IO which can block: +//! - directory scanning +//! - calls to unlink and fstat +//! - file handle closing (yes, that can block) +//! +//! Parallel is usually a win, but some users may value compile time or size of +//! compiled code more, so the `parallel` feature is opt-in. +//! +//! We suggest permitting the end user to control this choice: when adding +//! remove-dir-all as a dependency to a library crate, expose a feature +//! "parallel" that sets `remove-dir-all/parallel`. This will permit the user of +//! your library to control the parallel feature inside remove_dir_all //! //! e.g. //! //! ```Cargo.toml //! [features] -//! default = ["parallel"] +//! default = [] //! parallel = ["remove_dir_all/parallel"] //! ... //! [dependencies] -//! remove_dir_all = {version = "0.7", default-features = false} +//! remove_dir_all = {version = "0.8"} +//! +//! ## Future Plans +//! Open directory handles are kept in +//! a lg-spaced cache after the first 10 levels: +//! level10/skipped1/level12/skipped2/skipped3/skipped4/level16. If EMFILE is +//! encountered, no more handles are cached, and directories are opened by +//! re-traversing from the closest previously opened handle. Deletion should +//! succeed even only 4 file descriptors are available: one to hold the root, +//! two to iterate individual directories, and one to open-and-delete individual +//! files, though that will be quadratic in the depth of the tree, successfully +//! deleting leaves only on each iteration. +//! +//! IO Prioritisation: +//! 1) directory scanning when few paths are queued for deletion (to avoid +//! ending up accidentally serial) - allowing keeping the other queues full. +//! 4) close/CloseHandle (free up file descriptors) +//! 2) rmdir (free up file descriptors) +//! 3) unlink/SetFileInformationByHandle (to free up directories so they can be +//! rmdir'd) +//! +//! Scanning/unlinking/rmdiring is further biased by depth and lexographic +//! order: this minimises the number of directories being worked on in parallel, +//! so very branchy trees are less likely to exhaust kernel resources or +//! application memory or thrash the open directory cache. + //! ``` #![deny(missing_debug_implementations)] @@ -35,6 +133,10 @@ // See under "known problems" https://rust-lang.github.io/rust-clippy/master/index.html#mutex_atomic #![allow(clippy::mutex_atomic)] +use std::{io::Result, path::Path}; + +use normpath::PathExt; + #[cfg(doctest)] #[macro_use] extern crate doc_comment; @@ -42,19 +144,169 @@ extern crate doc_comment; #[cfg(doctest)] doctest!("../README.md"); -#[cfg(windows)] -mod fs; +mod _impl; + +/// Extension trait adding remove_dir_all support to File. +trait RemoveDir { + /// Remove the contents of the dir. + /// + /// `debug_root`: identifies the directory contents being removed + fn remove_dir_contents(&mut self, debug_root: Option<&Path>) -> Result<()>; +} + +/// Makes `path` an empty directory: if it does not exist, it is created it as +/// an empty directory (as if with `std::fs::create_dir`); if it does exist, its +/// contents are deleted (as if with `remove_dir_contents`). +/// +/// It is an error if `path` exists but is not a directory, including a symlink +/// to a directory. +/// +/// This is subject to file system races: a privileged process could be attacked +/// by replacing parent directories of the supplied path with a link (e.g. to +/// /etc). Consider using `RemoveDir::ensure_empty_dir()` instead. +pub fn ensure_empty_dir>(path: P) -> Result<()> { + _impl::_ensure_empty_dir_path::<_impl::OsIo, _>(path) +} + +/// Deletes the contents of `path`, but not the directory itself. It is an error +/// if `path` is not a directory. +/// +/// This is subject to file system races: a privileged process could be attacked +/// by replacing parent directories of the supplied path with a link (e.g. to +/// /etc). Consider using `RemoveDir::remove_dir_contents()` instead. +pub fn remove_dir_contents>(path: P) -> Result<()> { + _impl::_remove_dir_contents_path::<_impl::OsIo, P>(path) +} + +/// Reliably removes a directory and all of its children. +/// +/// ```rust +/// use std::fs; +/// use remove_dir_all::*; +/// +/// fs::create_dir("./temp/").unwrap(); +/// remove_dir_all("./temp/").unwrap(); +/// ``` +/// +/// Note: calling this on a non-directory (e.g. a symlink to a directory) will +/// error. +/// +/// [remove_dir_all::RemoveDir::remove_dir_all] is somewhat safer and +/// recommended as the path based version is subject to file system races +/// determining what to delete: a privileged process could be attacked by +/// replacing parent directories of the supplied path with a link (e.g. to +/// /etc). Consider using `RemoveDir::remove_dir_all()` instead. +pub fn remove_dir_all>(path: P) -> Result<()> { + let path = path.as_ref().normalize()?; + _impl::remove_dir_all_path::<_impl::OsIo, _>(path) +} + +#[allow(deprecated)] +#[cfg(test)] +mod tests { + //! functional tests for all platforms + //! + //! A note on safety: races are notoriously hard to secure merely via tests: + //! these tests use a dedicated trait to allow sequencing attack operations, + //! much the same as the test clock in Tokio programs. So these 'safe' tests + //! are not actually attempting scheduling races, rather they are showing + //! that the known attacks don't work. A fuzz based heuristic functional + //! test would be a good addition to complement these tests. + use super::Result; + + use std::fs::{self, File}; + use std::io; + use std::path::PathBuf; + + use tempfile::TempDir; + + use crate::ensure_empty_dir; + use crate::remove_dir_all; + use crate::remove_dir_contents; + + cfg_if::cfg_if! { + if #[cfg(windows)] { + const ENOTDIR:i32 = windows_sys::Win32::Foundation::ERROR_DIRECTORY as i32; + const ENOENT:i32 = windows_sys::Win32::Foundation::ERROR_FILE_NOT_FOUND as i32; + const INVALID_INPUT:i32 = windows_sys::Win32::Foundation::ERROR_INVALID_PARAMETER as i32; + } else { + const ENOTDIR:i32 = libc::ENOTDIR; + const ENOENT:i32 = libc::ENOENT; + const INVALID_INPUT:i32 = libc::ENOENT; + } + } + + /// Expect a particular sort of failure + fn expect_failure(n: &[i32], r: io::Result) -> io::Result<()> { + match r { + Err(e) + if n.iter() + .map(|n| Option::Some(*n)) + .any(|n| n == e.raw_os_error()) => + { + Ok(()) + } + Err(e) => { + println!("{e} {:?}, {:?}, {:?}", e.raw_os_error(), e.kind(), n); + Err(e) + } + Ok(_) => Err(io::Error::new( + io::ErrorKind::Other, + "unexpected success".to_string(), + )), + } + } + + struct Prep { + _tmp: TempDir, + ours: PathBuf, + file: PathBuf, + } + + /// Create test setup: t.mkdir/file all in a tempdir. + fn prep() -> Result { + let tmp = TempDir::new()?; + let ours = tmp.path().join("t.mkdir"); + let file = ours.join("file"); + fs::create_dir(&ours)?; + File::create(&file)?; + File::open(&file)?; + Ok(Prep { + _tmp: tmp, + ours, + file, + }) + } + + #[test] + fn mkdir_rm() -> Result<()> { + let p = prep()?; + + expect_failure(&[ENOTDIR, INVALID_INPUT], remove_dir_contents(&p.file))?; + + remove_dir_contents(&p.ours)?; + expect_failure(&[ENOENT], File::open(&p.file))?; + + remove_dir_contents(&p.ours)?; + remove_dir_all(&p.ours)?; + expect_failure(&[ENOENT], remove_dir_contents(&p.ours))?; + Ok(()) + } -#[cfg(not(windows))] -mod unix; + #[test] + fn ensure_rm() -> Result<()> { + let p = prep()?; -mod portable; + expect_failure(&[ENOTDIR, INVALID_INPUT], ensure_empty_dir(&p.file))?; -#[cfg(windows)] -pub use self::fs::remove_dir_all; + ensure_empty_dir(&p.ours)?; + expect_failure(&[ENOENT], File::open(&p.file))?; + ensure_empty_dir(&p.ours)?; -#[cfg(not(windows))] -pub use std::fs::remove_dir_all; + remove_dir_all(&p.ours)?; + ensure_empty_dir(&p.ours)?; + File::create(&p.file)?; -pub use portable::ensure_empty_dir; -pub use portable::remove_dir_contents; + Ok(()) + } +} diff --git a/src/portable.rs b/src/portable.rs deleted file mode 100644 index d4f3443..0000000 --- a/src/portable.rs +++ /dev/null @@ -1,133 +0,0 @@ -use std::io; -use std::path::Path; - -cfg_if::cfg_if! { - if #[cfg(windows)] { - use crate::fs::_remove_dir_contents; - } else { - use crate::unix::_remove_dir_contents; - } -} - -/// Deletes the contents of `path`, but not the directory iteself. -/// -/// If `path` is a symlink to a directory, deletes the contents -/// of that directory. Fails if `path` does not exist. -pub fn remove_dir_contents>(path: P) -> io::Result<()> { - // This wrapper function exists because the core function - // for Windows, in crate::fs, returns a PathBuf, which our - // caller shouldn't see. - _remove_dir_contents(path)?; - Ok(()) -} - -/// Makes `path` an empty directory: if it does not exist, it is -/// created it as an empty directory (as if with -/// `std::fs::create_dir`); if it does exist, its contents are -/// deleted (as if with `remove_dir_contents`). -/// -/// It is an error if `path` exists but is not a directory (or -/// a symlink to one). -pub fn ensure_empty_dir>(path: P) -> io::Result<()> { - match std::fs::create_dir(&path) { - Err(e) if e.kind() == io::ErrorKind::AlreadyExists => remove_dir_contents(path), - otherwise => otherwise, - } -} - -#[cfg(test)] -mod test { - use std::fs::{self, File}; - use std::io; - use std::path::PathBuf; - - use tempfile::TempDir; - - use crate::ensure_empty_dir; - use crate::remove_dir_all; - use crate::remove_dir_contents; - - cfg_if::cfg_if! { - if #[cfg(windows)] { - const ENOTDIR:i32 = windows_sys::Win32::Foundation::ERROR_DIRECTORY as i32; - const ENOENT:i32 = windows_sys::Win32::Foundation::ERROR_FILE_NOT_FOUND as i32; - } else { - const ENOTDIR:i32 = libc::ENOTDIR; - const ENOENT:i32 = libc::ENOENT; - } - } - - /// Expect a particular sort of failure - fn expect_failure(n: &[i32], r: io::Result) -> io::Result<()> { - match r { - Err(e) - if n.iter() - .map(|n| Option::Some(*n)) - .any(|n| n == e.raw_os_error()) => - { - Ok(()) - } - Err(e) => { - println!("{e} {:?}, {:?}, {:?}", e.raw_os_error(), e.kind(), n); - Err(e) - } - Ok(_) => Err(io::Error::new( - io::ErrorKind::Other, - "unexpected success".to_string(), - )), - } - } - - struct Prep { - _tmp: TempDir, - ours: PathBuf, - file: PathBuf, - } - - /// Create test setup: t.mkdir/file all in a tempdir. - fn prep() -> Result { - let tmp = TempDir::new()?; - let ours = tmp.path().join("t.mkdir"); - let file = ours.join("file"); - fs::create_dir(&ours)?; - File::create(&file)?; - File::open(&file)?; - Ok(Prep { - _tmp: tmp, - ours, - file, - }) - } - - #[test] - fn mkdir_rm() -> Result<(), io::Error> { - let p = prep()?; - - expect_failure(&[ENOTDIR], remove_dir_contents(&p.file))?; - - remove_dir_contents(&p.ours)?; - expect_failure(&[ENOENT], File::open(&p.file))?; - - remove_dir_contents(&p.ours)?; - remove_dir_all(&p.ours)?; - expect_failure(&[ENOENT], remove_dir_contents(&p.ours))?; - Ok(()) - } - - #[test] - fn ensure_rm() -> Result<(), io::Error> { - let p = prep()?; - - expect_failure(&[ENOTDIR], ensure_empty_dir(&p.file))?; - - ensure_empty_dir(&p.ours)?; - expect_failure(&[ENOENT], File::open(&p.file))?; - ensure_empty_dir(&p.ours)?; - - remove_dir_all(&p.ours)?; - ensure_empty_dir(&p.ours)?; - File::create(&p.file)?; - - Ok(()) - } -} diff --git a/src/unix.rs b/src/unix.rs deleted file mode 100644 index a60fde7..0000000 --- a/src/unix.rs +++ /dev/null @@ -1,21 +0,0 @@ -use std::fs; -use std::io; -use std::path::Path; - -fn remove_file_or_dir_all>(path: P) -> io::Result<()> { - match fs::remove_file(&path) { - // Unfortunately, there is no ErrorKind for EISDIR - Err(e) if e.raw_os_error() == Some(libc::EISDIR) => - fs::remove_dir_all(&path), - r => r, - } -} - -pub fn _remove_dir_contents>(path: P) -> Result<(), io::Error> { - for entry in fs::read_dir(path)? { - let entry_path = entry?.path(); - remove_file_or_dir_all(&entry_path)?; - } - - Ok(()) -} diff --git a/tests/interface.rs b/tests/interface.rs new file mode 100644 index 0000000..a93860f --- /dev/null +++ b/tests/interface.rs @@ -0,0 +1,232 @@ +//! Tests of the contract behaviour. Not intended to prove freedom from races +//! etc, but rather the interface which if changed will affect callers. + +use std::{ + fs::{self}, + path::Path, +}; + +use tempfile::TempDir; +use test_log::test; + +macro_rules! assert_not_found { + ($path:expr) => {{ + match fs::metadata($path) { + Ok(_) => panic!( + "did not expect to retrieve metadata for {}", + $path.display() + ), + Err(ref err) if err.kind() != ::std::io::ErrorKind::NotFound => { + panic!( + "expected path {} to be NotFound, was {:?}", + $path.display(), + err + ) + } + _ => {} + } + }}; +} + +#[track_caller] +fn assert_empty(path: &Path) { + assert_eq!( + [(); 0], + fs::read_dir(path).unwrap().map(|_e| ()).collect::>()[..] + ); +} + +#[track_caller] +fn assert_exists(path: &Path) { + fs::symlink_metadata(path).unwrap(); +} + +// ensure_dir_empty + +#[allow(deprecated)] +#[test] +fn ensure_empty_dir_missing_dir() { + let tempdir = TempDir::new().unwrap(); + let path = tempdir.path().join("newdir"); + remove_dir_all::ensure_empty_dir(&path).unwrap(); + assert_empty(&path); +} + +#[allow(deprecated)] +#[test] +fn ensure_empty_dir_existing_dir() { + let tempdir = TempDir::new().unwrap(); + let path = tempdir.path().join("newdir"); + fs::create_dir(&path).unwrap(); + remove_dir_all::ensure_empty_dir(&path).unwrap(); + assert_empty(&path); +} + +#[allow(deprecated)] +#[test] +fn ensure_empty_dir_not_empty() { + let tempdir = TempDir::new().unwrap(); + let path = tempdir.path().join("newdir"); + fs::create_dir(&path).unwrap(); + log::trace!("{path:?}"); + fs::write(path.join("child"), b"aa").unwrap(); + remove_dir_all::ensure_empty_dir(&path).unwrap(); + assert_empty(&path); +} + +#[allow(deprecated)] +#[test] +fn ensure_empty_dir_is_file() { + let tempdir = TempDir::new().unwrap(); + let path = tempdir.path().join("newfile"); + fs::write(&path, b"aa").unwrap(); + remove_dir_all::ensure_empty_dir(&path).unwrap_err(); + assert_exists(&path); +} + +#[allow(deprecated)] +#[test] +fn ensure_empty_dir_is_filelink() { + let tempdir = TempDir::new().unwrap(); + let path = tempdir.path().join("newlink"); + #[cfg(windows)] + std::os::windows::fs::symlink_file("target", &path).unwrap(); + #[cfg(not(windows))] + std::os::unix::fs::symlink("target", &path).unwrap(); + remove_dir_all::ensure_empty_dir(&path).unwrap_err(); + assert_exists(&path); +} + +#[allow(deprecated)] +#[cfg(windows)] +#[test] +fn ensure_empty_dir_is_dirlink() { + let tempdir = TempDir::new().unwrap(); + let path = tempdir.path().join("newlink"); + std::os::windows::fs::symlink_dir("target", &path).unwrap(); + remove_dir_all::ensure_empty_dir(&path).unwrap_err(); + assert_exists(&path); +} + +#[allow(deprecated)] +#[test] +fn removes_empty() { + let tempdir = TempDir::new().unwrap(); + let path = tempdir.path().join("empty"); + fs::create_dir_all(&path).unwrap(); + assert!(fs::metadata(&path).unwrap().is_dir()); + + remove_dir_all::remove_dir_all(&path).unwrap(); + assert_not_found!(&path); +} + +#[allow(deprecated)] +#[test] +fn removes_files() { + let tempdir = TempDir::new().unwrap(); + let path = tempdir.path().join("files"); + + fs::create_dir_all(&path).unwrap(); + + for i in 0..5 { + let filename = format!("empty-{}.txt", i); + let filepath = path.join(filename); + + { + let mut _file = fs::File::create(&filepath); + } + + assert!(fs::metadata(&filepath).unwrap().is_file()); + } + + remove_dir_all::remove_dir_all(&path).unwrap(); + assert_not_found!(&path); +} + +#[allow(deprecated)] +#[test] +fn removes_dirs() { + let tempdir = TempDir::new().unwrap(); + let path = tempdir.path().join("dirs"); + + for i in 0..5 { + let subpath = path.join(format!("{i}")).join("subdir"); + + log::trace!("making dir {}", subpath.display()); + fs::create_dir_all(&subpath).unwrap(); + + assert!(fs::metadata(&subpath).unwrap().is_dir()); + } + + remove_dir_all::remove_dir_all(&path).unwrap(); + assert_not_found!(&path); +} + +#[allow(deprecated)] +#[test] +#[cfg(windows)] +fn removes_read_only() { + let tempdir = TempDir::new().unwrap(); + let path = tempdir.path().join("readonly"); + + for i in 0..5 { + let subpath = path.join(format!("{}/subdir", i)); + + fs::create_dir_all(&subpath).unwrap(); + + let file_path = subpath.join("file.txt"); + { + log::trace!("create: {}", file_path.display()); + let file = fs::File::create(&file_path).unwrap(); + + if i % 2 == 0 { + log::trace!("making readonly: {}", file_path.display()); + let metadata = file.metadata().unwrap(); + let mut permissions = metadata.permissions(); + permissions.set_readonly(true); + + fs::set_permissions(&file_path, permissions).unwrap(); + } + } + + assert_eq!( + i % 2 == 0, + fs::metadata(&file_path).unwrap().permissions().readonly() + ); + + if i % 2 == 1 { + log::trace!("making readonly: {}", subpath.display()); + let metadata = fs::metadata(&subpath).unwrap(); + + let mut permissions = metadata.permissions(); + permissions.set_readonly(true); + + fs::set_permissions(&subpath, permissions).unwrap(); + + assert!(fs::metadata(&subpath).unwrap().permissions().readonly()); + } + } + + remove_dir_all::remove_dir_all(&path).unwrap(); + assert_not_found!(&path); +} + +#[allow(deprecated)] +#[test] +fn removes_symlinks() { + let tempdir = TempDir::new().unwrap(); + let root = tempdir.path().join("root"); + let link_name = root.join("some_link"); + let link_target = tempdir.path().join("target"); + fs::File::create(&link_target).unwrap(); + fs::create_dir(&root).unwrap(); + #[cfg(windows)] + std::os::windows::fs::symlink_file(&link_target, &link_name).unwrap(); + #[cfg(not(windows))] + std::os::unix::fs::symlink(&link_target, &link_name).unwrap(); + remove_dir_all::ensure_empty_dir(&root).unwrap(); + assert_exists(&root); + assert_exists(&link_target); +} + +// TODO: Should probably test readonly hard links... diff --git a/tests/windows.rs b/tests/windows.rs deleted file mode 100644 index 264ff0e..0000000 --- a/tests/windows.rs +++ /dev/null @@ -1,100 +0,0 @@ -#![cfg(windows)] -use remove_dir_all::remove_dir_all; -use std::fs::{self, File}; - -macro_rules! assert_not_found { - ($path:expr) => {{ - match fs::metadata($path) { - Ok(_) => panic!("did not expect to retrieve metadata for {}", $path), - Err(ref err) if err.kind() != ::std::io::ErrorKind::NotFound => { - panic!("expected path {} to be NotFound, was {:?}", $path, err) - } - _ => {} - } - }}; -} - -#[test] -fn removes_empty() { - fs::create_dir_all("./empty").unwrap(); - assert!(fs::metadata("./empty").unwrap().is_dir()); - - remove_dir_all("./empty").unwrap(); - assert_not_found!("./empty"); -} - -#[test] -fn removes_files() { - fs::create_dir_all("./files").unwrap(); - - for i in 0..5 { - let path = format!("./files/empty-{}.txt", i); - - { - let mut _file = File::create(&path); - } - - assert!(fs::metadata(&path).unwrap().is_file()); - } - - remove_dir_all("./files").unwrap(); - assert_not_found!("./files"); -} - -#[test] -fn removes_dirs() { - for i in 0..5 { - let path = format!("./dirs/{}/subdir", i); - - fs::create_dir_all(&path).unwrap(); - - assert!(fs::metadata(&path).unwrap().is_dir()); - } - - remove_dir_all("./dirs").unwrap(); - assert_not_found!("./dirs"); -} - -#[test] -fn removes_read_only() { - env_logger::init(); - for i in 0..5 { - let path = format!("./readonly/{}/subdir", i); - - fs::create_dir_all(&path).unwrap(); - - let file_path = format!("{}/file.txt", path); - { - let file = File::create(&file_path).unwrap(); - - if i % 2 == 0 { - let metadata = file.metadata().unwrap(); - let mut permissions = metadata.permissions(); - permissions.set_readonly(true); - - fs::set_permissions(&file_path, permissions).unwrap(); - } - } - - assert_eq!( - i % 2 == 0, - fs::metadata(&file_path).unwrap().permissions().readonly() - ); - - if i % 2 == 1 { - let metadata = fs::metadata(&path).unwrap(); - - let mut permissions = metadata.permissions(); - permissions.set_readonly(true); - - fs::set_permissions(&path, permissions).unwrap(); - - assert!(fs::metadata(&path).unwrap().permissions().readonly()); - } - } - - remove_dir_all("./readonly").unwrap(); - assert_not_found!("./readonly"); -} - -// TODO: Should probably test readonly hard links...