Skip to content
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

Fix fs::remove_dir_all #31944

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 37 additions & 1 deletion src/libstd/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1426,7 +1426,8 @@ impl Iterator for WalkDir {
/// # Platform-specific behavior
///
/// This function currently corresponds to the `chmod` function on Unix
/// and the `SetFileAttributes` function on Windows.
/// and the `CreateFile`, `GetFileInformationByHandle` and
/// `SetFileInformationByHandle` function on Windows.
/// Note that, this [may change in the future][changes].
/// [changes]: ../io/index.html#platform-specific-behavior
///
Expand Down Expand Up @@ -1980,6 +1981,41 @@ mod tests {
}
}

#[test]
fn recursive_rmdir_tricky() {
let tmpdir = tmpdir();
let dir = tmpdir.join("dir");
check!(fs::create_dir(&dir));
// filenames that can only be accessed with `/??/`-paths
let fullpath = check!(dir.canonicalize());
check!(File::create(fullpath.join("morse .. .")));
check!(File::create(fullpath.join("con")));
// read-only file
let readonly = dir.join("readonly");
check!(File::create(&readonly));
let mut perms = check!(readonly.metadata()).permissions();
perms.set_readonly(true);
check!(fs::set_permissions(&readonly, perms));
// hardlink outside this directory should not lose its read-only flag
check!(fs::hard_link(&readonly, tmpdir.join("canary_ro")));
// read-only dir
let readonly_dir = dir.join("readonly_dir");
check!(fs::create_dir(&readonly_dir));
check!(File::create(readonly_dir.join("file")));
let mut perms = check!(readonly_dir.metadata()).permissions();
perms.set_readonly(true);
check!(fs::set_permissions(&readonly_dir, perms));
// open file
let mut opts = fs::OpenOptions::new();
let mut file_open = check!(opts.write(true).create(true)
.open(dir.join("remains_open")));

check!(fs::remove_dir_all(&dir));
assert!(check!(tmpdir.join("canary_ro").metadata())
.permissions().readonly());
check!(file_open.write("something".as_bytes()));
}

#[test]
fn unicode_path_is_dir() {
assert!(Path::new(".").is_dir());
Expand Down
18 changes: 16 additions & 2 deletions src/libstd/sys/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -639,10 +639,17 @@ pub fn rmdir(p: &Path) -> io::Result<()> {
}

pub fn remove_dir_all(path: &Path) -> io::Result<()> {
let filetype = try!(lstat(path)).file_type();
if filetype.is_symlink() {
let metadata = try!(lstat(path));
if metadata.file_type().is_symlink() {
unlink(path)
} else {
let mut mode = metadata.perm();
if mode.readonly() {
// we can only remove the contents of a directory if it is not
// readonly
mode.set_readonly(false);
try!(set_perm(&path, mode));
}
remove_dir_all_recursive(path)
}
}
Expand All @@ -651,6 +658,13 @@ fn remove_dir_all_recursive(path: &Path) -> io::Result<()> {
for child in try!(readdir(path)) {
let child = try!(child);
if try!(child.file_type()).is_dir() {
let mut mode = try!(child.metadata()).perm();
if mode.readonly() {
// we can only remove the contents of a directory if it is not
// readonly
mode.set_readonly(false);
try!(set_perm(&child.path(), mode));
}
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps this function could take &Metadata as an argument and perform this logic at the start? That way it could prevent duplication between here/above.

try!(remove_dir_all_recursive(&child.path()));
} else {
try!(unlink(&child.path()));
Expand Down
24 changes: 22 additions & 2 deletions src/libstd/sys/windows/c.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ pub const TRUE: BOOL = 1;
pub const FALSE: BOOL = 0;

pub const FILE_ATTRIBUTE_READONLY: DWORD = 0x1;
#[cfg(test)]
pub const FILE_ATTRIBUTE_SYSTEM: DWORD = 0x4;
pub const FILE_ATTRIBUTE_DIRECTORY: DWORD = 0x10;
pub const FILE_ATTRIBUTE_REPARSE_POINT: DWORD = 0x400;

Expand All @@ -98,7 +100,9 @@ pub const TRUNCATE_EXISTING: DWORD = 5;
pub const FILE_WRITE_DATA: DWORD = 0x00000002;
pub const FILE_APPEND_DATA: DWORD = 0x00000004;
pub const FILE_WRITE_EA: DWORD = 0x00000010;
pub const FILE_READ_ATTRIBUTES: DWORD = 0x00000080;
pub const FILE_WRITE_ATTRIBUTES: DWORD = 0x00000100;
pub const DELETE: DWORD = 0x00010000;
pub const READ_CONTROL: DWORD = 0x00020000;
pub const SYNCHRONIZE: DWORD = 0x00100000;
pub const GENERIC_READ: DWORD = 0x80000000;
Expand All @@ -112,6 +116,7 @@ pub const FILE_GENERIC_WRITE: DWORD = STANDARD_RIGHTS_WRITE | FILE_WRITE_DATA |

pub const FILE_FLAG_OPEN_REPARSE_POINT: DWORD = 0x00200000;
pub const FILE_FLAG_BACKUP_SEMANTICS: DWORD = 0x02000000;
pub const FILE_FLAG_DELETE_ON_CLOSE: DWORD = 0x04000000;
pub const SECURITY_SQOS_PRESENT: DWORD = 0x00100000;

#[repr(C)]
Expand Down Expand Up @@ -364,6 +369,23 @@ pub enum FILE_INFO_BY_HANDLE_CLASS {
MaximumFileInfoByHandlesClass
}

#[repr(C)]
pub struct FILE_BASIC_INFO {
pub CreationTime: LARGE_INTEGER,
pub LastAccessTime: LARGE_INTEGER,
pub LastWriteTime: LARGE_INTEGER,
pub ChangeTime: LARGE_INTEGER,
pub FileAttributes: DWORD,
}

#[repr(C)]
pub struct FILE_RENAME_INFO {
pub ReplaceIfExists: BOOL, // for true use -1, not TRUE
pub RootDirectory: HANDLE, // NULL, or obtained with NtOpenDirectoryObject
pub FileNameLength: DWORD,
pub FileName: [WCHAR; 0],
}

#[repr(C)]
pub struct FILE_END_OF_FILE_INFO {
pub EndOfFile: LARGE_INTEGER,
Expand Down Expand Up @@ -855,8 +877,6 @@ extern "system" {
pub fn GetConsoleMode(hConsoleHandle: HANDLE,
lpMode: LPDWORD) -> BOOL;
pub fn RemoveDirectoryW(lpPathName: LPCWSTR) -> BOOL;
pub fn SetFileAttributesW(lpFileName: LPCWSTR,
dwFileAttributes: DWORD) -> BOOL;
pub fn GetFileInformationByHandle(hFile: HANDLE,
lpFileInformation: LPBY_HANDLE_FILE_INFORMATION)
-> BOOL;
Expand Down
Loading