From 13bad942927563f4437957d384b7bc882ff37a66 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Sun, 28 Feb 2016 08:49:39 +0100 Subject: [PATCH 1/5] Small fixes to setting Windows file permissions On Windows files can have a simple read-only permission flag, and the more complex access rights and access control lists. The read-only flag is one bit of the file attributes. To make sure only the read-only flag is toggled and not the other attributes, read the files attributes and write them back with only this bit changed. By reading and setting the attributes from the handle the file does not have to be opened twice. Also directories can not be read-only, here the flag has a different meaning. So we should no longer report directories as read-only, and also not allow making them read-only. --- src/libstd/fs.rs | 3 +- src/libstd/sys/windows/c.rs | 12 +++++- src/libstd/sys/windows/fs.rs | 77 ++++++++++++++++++++++++++---------- 3 files changed, 69 insertions(+), 23 deletions(-) diff --git a/src/libstd/fs.rs b/src/libstd/fs.rs index badbba21d55cc..14b916513a711 100644 --- a/src/libstd/fs.rs +++ b/src/libstd/fs.rs @@ -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 /// diff --git a/src/libstd/sys/windows/c.rs b/src/libstd/sys/windows/c.rs index 5cbfec01bedaa..f1714ffc48fa1 100644 --- a/src/libstd/sys/windows/c.rs +++ b/src/libstd/sys/windows/c.rs @@ -98,6 +98,7 @@ 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 READ_CONTROL: DWORD = 0x00020000; pub const SYNCHRONIZE: DWORD = 0x00100000; @@ -364,6 +365,15 @@ 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_END_OF_FILE_INFO { pub EndOfFile: LARGE_INTEGER, @@ -855,8 +865,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; diff --git a/src/libstd/sys/windows/fs.rs b/src/libstd/sys/windows/fs.rs index 95fb1e7c60052..f935d135840fb 100644 --- a/src/libstd/sys/windows/fs.rs +++ b/src/libstd/sys/windows/fs.rs @@ -77,8 +77,8 @@ pub struct OpenOptions { security_attributes: usize, // FIXME: should be a reference } -#[derive(Clone, PartialEq, Eq, Debug)] -pub struct FilePermissions { attrs: c::DWORD } +#[derive(Clone, PartialEq, Eq, Debug, Default)] +pub struct FilePermissions { readonly: bool } pub struct DirBuilder; @@ -333,6 +333,46 @@ impl File { Ok(newpos as u64) } + pub fn set_attributes(&self, attr: c::DWORD) -> io::Result<()> { + let mut info = c::FILE_BASIC_INFO { + CreationTime: 0, // do not change + LastAccessTime: 0, // do not change + LastWriteTime: 0, // do not change + ChangeTime: 0, // do not change + FileAttributes: attr, + }; + let size = mem::size_of_val(&info); + try!(cvt(unsafe { + c::SetFileInformationByHandle(self.handle.raw(), + c::FileBasicInfo, + &mut info as *mut _ as *mut _, + size as c::DWORD) + })); + Ok(()) + } + + pub fn set_perm(&self, perm: FilePermissions) -> io::Result<()> { + let attr = try!(self.file_attr()).attributes; + if attr & c::FILE_ATTRIBUTE_DIRECTORY != 0 { + // this matches directories, dir symlinks, and junctions. + if perm.readonly { + Err(io::Error::new(io::ErrorKind::PermissionDenied, + "directories can not be read-only")) + } else { + Ok(()) // no reason to fail, as the result is what is expected. + // (the directory will not be read-only) + } + } else { + if perm.readonly == (attr & c::FILE_ATTRIBUTE_READONLY != 0) { + Ok(()) + } else if perm.readonly { + self.set_attributes(attr | c::FILE_ATTRIBUTE_READONLY) + } else { + self.set_attributes(attr & !c::FILE_ATTRIBUTE_READONLY) + } + } + } + pub fn duplicate(&self) -> io::Result { Ok(File { handle: try!(self.handle.duplicate(0, true, c::DUPLICATE_SAME_ACCESS)), @@ -422,7 +462,12 @@ impl FileAttr { } pub fn perm(&self) -> FilePermissions { - FilePermissions { attrs: self.attributes } + FilePermissions { + // Only files can be read-only. If the flag is set on a directory this means the + // directory its view is customized by Windows (with a Desktop.ini file) + readonly: self.attributes & c::FILE_ATTRIBUTE_READONLY != 0 && + self.attributes & c::FILE_ATTRIBUTE_DIRECTORY == 0 + } } pub fn attrs(&self) -> u32 { self.attributes as u32 } @@ -465,17 +510,9 @@ fn to_u64(ft: &c::FILETIME) -> u64 { } impl FilePermissions { - pub fn readonly(&self) -> bool { - self.attrs & c::FILE_ATTRIBUTE_READONLY != 0 - } - - pub fn set_readonly(&mut self, readonly: bool) { - if readonly { - self.attrs |= c::FILE_ATTRIBUTE_READONLY; - } else { - self.attrs &= !c::FILE_ATTRIBUTE_READONLY; - } - } + pub fn new() -> FilePermissions { Default::default() } + pub fn readonly(&self) -> bool { self.readonly } + pub fn set_readonly(&mut self, readonly: bool) { self.readonly = readonly } } impl FileType { @@ -640,12 +677,12 @@ pub fn lstat(path: &Path) -> io::Result { file.file_attr() } -pub fn set_perm(p: &Path, perm: FilePermissions) -> io::Result<()> { - let p = try!(to_u16s(p)); - unsafe { - try!(cvt(c::SetFileAttributesW(p.as_ptr(), perm.attrs))); - Ok(()) - } +pub fn set_perm(path: &Path, perm: FilePermissions) -> io::Result<()> { + let mut opts = OpenOptions::new(); + opts.access_mode(c::FILE_READ_ATTRIBUTES | c::FILE_WRITE_ATTRIBUTES); + opts.custom_flags(c::FILE_FLAG_BACKUP_SEMANTICS); + let file = try!(File::open(path, &opts)); + file.set_perm(perm) } fn get_path(f: &File) -> io::Result { From 3d65bb24036adc22ee43a24aea99a41ce0d93bbe Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Sun, 28 Feb 2016 08:50:49 +0100 Subject: [PATCH 2/5] Fix `fs::remove_dir_all` This fixes the case where `remove_dir_all` is unable to remove large directories on Windows due to race conditions. It is now also able to remove read-only files, and when the paths become longer that MAX_PATH. --- src/libstd/fs.rs | 32 +++++++++ src/libstd/sys/windows/c.rs | 15 +++++ src/libstd/sys/windows/fs.rs | 127 ++++++++++++++++++++++++++++++++--- 3 files changed, 163 insertions(+), 11 deletions(-) diff --git a/src/libstd/fs.rs b/src/libstd/fs.rs index 14b916513a711..d87831eff5832 100644 --- a/src/libstd/fs.rs +++ b/src/libstd/fs.rs @@ -1981,6 +1981,38 @@ mod tests { } } + #[test] + #[cfg(windows)] // only tricky on Windows + fn recursive_rmdir_tricky() { + use os::windows::fs::OpenOptionsExt; + 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 mut opts = fs::OpenOptions::new(); + opts.write(true); + opts.create(true); + opts.attributes(0x1); // FILE_ATTRIBUTE_READONLY + let _ = check!(opts.open(dir.join("readonly"))); + } + // hardlink outside this directory should not lose its read-only flag + check!(fs::hard_link(dir.join("readonly"), tmpdir.join("canary_ro"))); + // 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()); diff --git a/src/libstd/sys/windows/c.rs b/src/libstd/sys/windows/c.rs index f1714ffc48fa1..bee585a39c212 100644 --- a/src/libstd/sys/windows/c.rs +++ b/src/libstd/sys/windows/c.rs @@ -100,6 +100,7 @@ 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; @@ -113,6 +114,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)] @@ -374,6 +376,19 @@ pub struct FILE_BASIC_INFO { 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_DISPOSITION_INFO { + pub DeleteFile: BOOL, // for true use -1, not TRUE +} + #[repr(C)] pub struct FILE_END_OF_FILE_INFO { pub EndOfFile: LARGE_INTEGER, diff --git a/src/libstd/sys/windows/fs.rs b/src/libstd/sys/windows/fs.rs index f935d135840fb..19635045fdb0e 100644 --- a/src/libstd/sys/windows/fs.rs +++ b/src/libstd/sys/windows/fs.rs @@ -11,7 +11,7 @@ use io::prelude::*; use os::windows::prelude::*; -use ffi::OsString; +use ffi::{OsString, OsStr}; use fmt; use io::{self, Error, SeekFrom}; use mem; @@ -23,6 +23,7 @@ use sys::handle::Handle; use sys::time::SystemTime; use sys::{c, cvt}; use sys_common::FromInner; +use vec::Vec; use super::to_u16s; @@ -333,6 +334,51 @@ impl File { Ok(newpos as u64) } + pub fn remove(&self) -> io::Result<()> { + let mut info = c::FILE_DISPOSITION_INFO { + DeleteFile: -1, + }; + let size = mem::size_of_val(&info); + try!(cvt(unsafe { + c::SetFileInformationByHandle(self.handle.raw(), + c::FileDispositionInfo, + &mut info as *mut _ as *mut _, + size as c::DWORD) + })); + Ok(()) + } + + pub fn rename(&self, new: &Path, replace: bool) -> io::Result<()> { + // &self must be opened with DELETE permission + + #[cfg(target_arch = "x86")] + const STRUCT_SIZE: usize = 12; + #[cfg(target_arch = "x86_64")] + const STRUCT_SIZE: usize = 20; + + // FIXME: check for internal NULs in 'new' + let reserved = OsStr::new(&"\0\0\0\0\0\0\0\0\0\0"[..STRUCT_SIZE/2]); + let mut data: Vec = reserved.encode_wide() + .chain(new.as_os_str().encode_wide()) + .collect(); + data.push(0); + let size = data.len() * 2; + + unsafe { + // Thanks to alignment guarantees on Windows this works + // (8 for 32-bit and 16 for 64-bit) + let mut info = data.as_mut_ptr() as *mut c::FILE_RENAME_INFO; + (*info).ReplaceIfExists = if replace { -1 } else { c::FALSE }; + (*info).RootDirectory = ptr::null_mut(); + (*info).FileNameLength = (size - STRUCT_SIZE) as c::DWORD; + try!(cvt(c::SetFileInformationByHandle(self.handle().raw(), + c::FileRenameInfo, + data.as_mut_ptr() as *mut _ as *mut _, + size as c::DWORD))); + Ok(()) + } + } + pub fn set_attributes(&self, attr: c::DWORD) -> io::Result<()> { let mut info = c::FILE_BASIC_INFO { CreationTime: 0, // do not change @@ -539,9 +585,6 @@ impl FileType { *self == FileType::SymlinkDir || *self == FileType::MountPoint } - pub fn is_symlink_dir(&self) -> bool { - *self == FileType::SymlinkDir || *self == FileType::MountPoint - } } impl DirBuilder { @@ -604,23 +647,85 @@ pub fn remove_dir_all(path: &Path) -> io::Result<()> { // rmdir only deletes dir symlinks and junctions, not file symlinks. rmdir(path) } else { - remove_dir_all_recursive(path) + // canonicalize to get a `//?/`-path, which can handle files like `CON` + // and `morse .. .`, and when a directory structure is so deep it needs + // long path names + let path = try!(path.canonicalize()); + let base_dir = match path.parent() { + Some(dir) => dir, + None => return Err(io::Error::new(io::ErrorKind::PermissionDenied, + "can't delete root directory")) + }; + try!(remove_dir_all_recursive(path.as_ref(), base_dir.as_ref(), 0)); + Ok(()) } } -fn remove_dir_all_recursive(path: &Path) -> io::Result<()> { +fn remove_dir_all_recursive(path: &Path, base_dir: &Path, mut counter: u64) + -> io::Result { + // 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 scheduled. So just before or after flagging a + // file for deletion, we move it to some `base_dir` to avoid races. + + fn move_item(file: &File, base_dir: &Path, mut counter: u64) + -> io::Result { + let mut tmpname = base_dir.join(format!{"rm-{}", counter}); + counter += 1; + // Try to rename the file. If it already exists, just retry with an other filename. + while let Err(err) = file.rename(tmpname.as_ref(), false) { + if err.kind() != io::ErrorKind::AlreadyExists { return Err(err) }; + tmpname = base_dir.join(format!{"rm-{}", counter}); + counter += 1; + } + Ok(counter) + } + + fn remove_item(path: &Path, + base_dir: &Path, + mut counter: u64, + readonly: bool) -> io::Result { + if !readonly { + let mut opts = OpenOptions::new(); + opts.access_mode(c::DELETE); + opts.custom_flags(c::FILE_FLAG_BACKUP_SEMANTICS | // delete directory + c::FILE_FLAG_OPEN_REPARSE_POINT | // delete symlink + c::FILE_FLAG_DELETE_ON_CLOSE); + let file = try!(File::open(path, &opts)); + counter = try!(move_item(&file, base_dir, counter)); + } else { + let mut opts = OpenOptions::new(); + opts.access_mode(c::DELETE | c::FILE_WRITE_ATTRIBUTES); + opts.custom_flags(c::FILE_FLAG_OPEN_REPARSE_POINT); + let file = try!(File::open(path, &opts)); + // remove read-only permision + try!(file.set_perm(FilePermissions::new())); + counter = try!(move_item(&file, base_dir, counter)); + try!(file.remove()); + // restore read-only flag just in case there are other hard links + let mut perm = FilePermissions::new(); + perm.set_readonly(true); + try!(file.set_perm(perm)); + }; + Ok(counter) + } + for child in try!(readdir(path)) { let child = try!(child); let child_type = try!(child.file_type()); if child_type.is_dir() { - try!(remove_dir_all_recursive(&child.path())); - } else if child_type.is_symlink_dir() { - try!(rmdir(&child.path())); + counter = try!(remove_dir_all_recursive(&child.path(), + base_dir, + counter)); } else { - try!(unlink(&child.path())); + counter = try!(remove_item(&child.path().as_ref(), + base_dir, + counter, + try!(child.metadata()).perm().readonly())); } } - rmdir(path) + counter = try!(remove_item(path, base_dir, counter, false)); + Ok(counter) } pub fn readlink(path: &Path) -> io::Result { From 5f9ca8849877800d6b09b0bc3060f564096039b9 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Sun, 28 Feb 2016 20:52:13 +0100 Subject: [PATCH 3/5] Nicer iterator --- src/libstd/sys/windows/fs.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libstd/sys/windows/fs.rs b/src/libstd/sys/windows/fs.rs index 19635045fdb0e..8677b4afd097d 100644 --- a/src/libstd/sys/windows/fs.rs +++ b/src/libstd/sys/windows/fs.rs @@ -11,7 +11,7 @@ use io::prelude::*; use os::windows::prelude::*; -use ffi::{OsString, OsStr}; +use ffi::OsString; use fmt; use io::{self, Error, SeekFrom}; use mem; @@ -350,15 +350,14 @@ impl File { pub fn rename(&self, new: &Path, replace: bool) -> io::Result<()> { // &self must be opened with DELETE permission - + use iter; #[cfg(target_arch = "x86")] const STRUCT_SIZE: usize = 12; #[cfg(target_arch = "x86_64")] const STRUCT_SIZE: usize = 20; // FIXME: check for internal NULs in 'new' - let reserved = OsStr::new(&"\0\0\0\0\0\0\0\0\0\0"[..STRUCT_SIZE/2]); - let mut data: Vec = reserved.encode_wide() + let mut data: Vec = iter::repeat(0u16).take(STRUCT_SIZE/2) .chain(new.as_os_str().encode_wide()) .collect(); data.push(0); From 9bb385ec4118eca87133df56778749efafd9f1f5 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Sat, 5 Mar 2016 15:04:41 +0100 Subject: [PATCH 4/5] Address comments --- src/libstd/fs.rs | 23 +-- src/libstd/sys/windows/c.rs | 7 +- src/libstd/sys/windows/fs.rs | 321 +++++++++++++++++++++++------------ 3 files changed, 225 insertions(+), 126 deletions(-) diff --git a/src/libstd/fs.rs b/src/libstd/fs.rs index d87831eff5832..47bc83656df05 100644 --- a/src/libstd/fs.rs +++ b/src/libstd/fs.rs @@ -1982,9 +1982,7 @@ mod tests { } #[test] - #[cfg(windows)] // only tricky on Windows fn recursive_rmdir_tricky() { - use os::windows::fs::OpenOptionsExt; let tmpdir = tmpdir(); let dir = tmpdir.join("dir"); check!(fs::create_dir(&dir)); @@ -1993,15 +1991,20 @@ mod tests { check!(File::create(fullpath.join("morse .. ."))); check!(File::create(fullpath.join("con"))); // read-only file - { - let mut opts = fs::OpenOptions::new(); - opts.write(true); - opts.create(true); - opts.attributes(0x1); // FILE_ATTRIBUTE_READONLY - let _ = check!(opts.open(dir.join("readonly"))); - } + 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(dir.join("readonly"), tmpdir.join("canary_ro"))); + 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) diff --git a/src/libstd/sys/windows/c.rs b/src/libstd/sys/windows/c.rs index bee585a39c212..e7e87d638b0d1 100644 --- a/src/libstd/sys/windows/c.rs +++ b/src/libstd/sys/windows/c.rs @@ -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; @@ -384,11 +386,6 @@ pub struct FILE_RENAME_INFO { pub FileName: [WCHAR; 0], } -#[repr(C)] -pub struct FILE_DISPOSITION_INFO { - pub DeleteFile: BOOL, // for true use -1, not TRUE -} - #[repr(C)] pub struct FILE_END_OF_FILE_INFO { pub EndOfFile: LARGE_INTEGER, diff --git a/src/libstd/sys/windows/fs.rs b/src/libstd/sys/windows/fs.rs index 8677b4afd097d..9e58f2b5ef17b 100644 --- a/src/libstd/sys/windows/fs.rs +++ b/src/libstd/sys/windows/fs.rs @@ -334,20 +334,6 @@ impl File { Ok(newpos as u64) } - pub fn remove(&self) -> io::Result<()> { - let mut info = c::FILE_DISPOSITION_INFO { - DeleteFile: -1, - }; - let size = mem::size_of_val(&info); - try!(cvt(unsafe { - c::SetFileInformationByHandle(self.handle.raw(), - c::FileDispositionInfo, - &mut info as *mut _ as *mut _, - size as c::DWORD) - })); - Ok(()) - } - pub fn rename(&self, new: &Path, replace: bool) -> io::Result<()> { // &self must be opened with DELETE permission use iter; @@ -367,6 +353,8 @@ impl File { // Thanks to alignment guarantees on Windows this works // (8 for 32-bit and 16 for 64-bit) let mut info = data.as_mut_ptr() as *mut c::FILE_RENAME_INFO; + // The type of ReplaceIfExists is BOOL, but it actually expects a + // BOOLEAN. This means true is -1, not c::TRUE. (*info).ReplaceIfExists = if replace { -1 } else { c::FALSE }; (*info).RootDirectory = ptr::null_mut(); (*info).FileNameLength = (size - STRUCT_SIZE) as c::DWORD; @@ -398,23 +386,12 @@ impl File { pub fn set_perm(&self, perm: FilePermissions) -> io::Result<()> { let attr = try!(self.file_attr()).attributes; - if attr & c::FILE_ATTRIBUTE_DIRECTORY != 0 { - // this matches directories, dir symlinks, and junctions. - if perm.readonly { - Err(io::Error::new(io::ErrorKind::PermissionDenied, - "directories can not be read-only")) - } else { - Ok(()) // no reason to fail, as the result is what is expected. - // (the directory will not be read-only) - } + if perm.readonly == (attr & c::FILE_ATTRIBUTE_READONLY != 0) { + Ok(()) + } else if perm.readonly { + self.set_attributes(attr | c::FILE_ATTRIBUTE_READONLY) } else { - if perm.readonly == (attr & c::FILE_ATTRIBUTE_READONLY != 0) { - Ok(()) - } else if perm.readonly { - self.set_attributes(attr | c::FILE_ATTRIBUTE_READONLY) - } else { - self.set_attributes(attr & !c::FILE_ATTRIBUTE_READONLY) - } + self.set_attributes(attr & !c::FILE_ATTRIBUTE_READONLY) } } @@ -508,10 +485,7 @@ impl FileAttr { pub fn perm(&self) -> FilePermissions { FilePermissions { - // Only files can be read-only. If the flag is set on a directory this means the - // directory its view is customized by Windows (with a Desktop.ini file) - readonly: self.attributes & c::FILE_ATTRIBUTE_READONLY != 0 && - self.attributes & c::FILE_ATTRIBUTE_DIRECTORY == 0 + readonly: self.attributes & c::FILE_ATTRIBUTE_READONLY != 0 } } @@ -584,6 +558,9 @@ impl FileType { *self == FileType::SymlinkDir || *self == FileType::MountPoint } + pub fn is_symlink_dir(&self) -> bool { + *self == FileType::SymlinkDir || *self == FileType::MountPoint + } } impl DirBuilder { @@ -640,99 +617,148 @@ 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() { - // On Windows symlinks to files and directories are removed differently. - // rmdir only deletes dir symlinks and junctions, not file symlinks. - rmdir(path) - } else { - // canonicalize to get a `//?/`-path, which can handle files like `CON` - // and `morse .. .`, and when a directory structure is so deep it needs - // long path names - let path = try!(path.canonicalize()); - let base_dir = match path.parent() { + // 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 scheduled. + // To work around this, we move the file or directory to some `base_dir` + // right before deletion to avoid races. + // + // As `base_dir` we choose the parent dir of the directory we want to + // remove. We very probably have permission to create files here, as we + // already need write permission in this dir to delete the directory. And it + // should be on the same volume. + // + // 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 a `//?/`-path with `get_path()`. + // + // To make sure we don't leave a moved file laying around if the process + // crashes before we can delete the file, we do all operations on an file + // handle. By opening a file with `FILE_FLAG_DELETE_ON_CLOSE` Windows will + // always delete the file when the handle closes. + // + // All files are renamed to be in the `base_dir`, and have their name + // changed to "rm-". After every rename the counter is increased. + // Rename should not overwrite possibly existing files in the base dir. So + // if it fails with `AlreadyExists`, we just increase the counter and try + // again. + // + // For read-only files and directories we first have to remove the read-only + // attribute before we can move or delete them. This also removes the + // attribute from possible hardlinks to the file, so just before closing we + // restore the read-only attribute. + // + // If 'path' points to a directory symlink or junction we should not + // recursively remove the target of the link, but only the link itself. + // + // Moving and deleting is guaranteed to succeed if we are able to open the + // file with `DELETE` permission. If others have the file open we only have + // `DELETE` permission if they have specified `FILE_SHARE_DELETE`. We can + // also delete the file now, but it will not disappear until all others have + // closed the file. But no-one can open the file after we have flagged it + // for deletion. + + // Open the path once to get the canonical path, file type and attributes. + let (path, metadata) = { + let mut opts = OpenOptions::new(); + opts.access_mode(c::FILE_READ_ATTRIBUTES); + opts.custom_flags(c::FILE_FLAG_BACKUP_SEMANTICS | + c::FILE_FLAG_OPEN_REPARSE_POINT); + let file = try!(File::open(path, &opts)); + (try!(get_path(&file)), try!(file.file_attr())) + }; + + let mut ctx = RmdirContext { + base_dir: match path.parent() { Some(dir) => dir, None => return Err(io::Error::new(io::ErrorKind::PermissionDenied, "can't delete root directory")) - }; - try!(remove_dir_all_recursive(path.as_ref(), base_dir.as_ref(), 0)); - Ok(()) + }, + readonly: metadata.perm().readonly(), + counter: 0, + }; + + let filetype = metadata.file_type(); + if filetype.is_dir() { + remove_dir_all_recursive(path.as_ref(), &mut ctx) + } else if filetype.is_symlink_dir() { + remove_item(path.as_ref(), &mut ctx) + } else { + Err(io::Error::new(io::ErrorKind::PermissionDenied, "Not a directory")) } } -fn remove_dir_all_recursive(path: &Path, base_dir: &Path, mut counter: u64) - -> io::Result { - // 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 scheduled. So just before or after flagging a - // file for deletion, we move it to some `base_dir` to avoid races. - - fn move_item(file: &File, base_dir: &Path, mut counter: u64) - -> io::Result { - let mut tmpname = base_dir.join(format!{"rm-{}", counter}); - counter += 1; - // Try to rename the file. If it already exists, just retry with an other filename. - while let Err(err) = file.rename(tmpname.as_ref(), false) { - if err.kind() != io::ErrorKind::AlreadyExists { return Err(err) }; - tmpname = base_dir.join(format!{"rm-{}", counter}); - counter += 1; - } - Ok(counter) - } - - fn remove_item(path: &Path, - base_dir: &Path, - mut counter: u64, - readonly: bool) -> io::Result { - if !readonly { - let mut opts = OpenOptions::new(); - opts.access_mode(c::DELETE); - opts.custom_flags(c::FILE_FLAG_BACKUP_SEMANTICS | // delete directory - c::FILE_FLAG_OPEN_REPARSE_POINT | // delete symlink - c::FILE_FLAG_DELETE_ON_CLOSE); - let file = try!(File::open(path, &opts)); - counter = try!(move_item(&file, base_dir, counter)); - } else { - let mut opts = OpenOptions::new(); - opts.access_mode(c::DELETE | c::FILE_WRITE_ATTRIBUTES); - opts.custom_flags(c::FILE_FLAG_OPEN_REPARSE_POINT); - let file = try!(File::open(path, &opts)); - // remove read-only permision - try!(file.set_perm(FilePermissions::new())); - counter = try!(move_item(&file, base_dir, counter)); - try!(file.remove()); - // restore read-only flag just in case there are other hard links - let mut perm = FilePermissions::new(); - perm.set_readonly(true); - try!(file.set_perm(perm)); - }; - Ok(counter) - } +struct RmdirContext<'a> { + base_dir: &'a Path, + readonly: bool, + counter: u64, +} +fn remove_dir_all_recursive(path: &Path, ctx: &mut RmdirContext) + -> io::Result<()> { + let dir_readonly = ctx.readonly; for child in try!(readdir(path)) { let child = try!(child); let child_type = try!(child.file_type()); + ctx.readonly = try!(child.metadata()).perm().readonly(); if child_type.is_dir() { - counter = try!(remove_dir_all_recursive(&child.path(), - base_dir, - counter)); + try!(remove_dir_all_recursive(&child.path(), ctx)); } else { - counter = try!(remove_item(&child.path().as_ref(), - base_dir, - counter, - try!(child.metadata()).perm().readonly())); + try!(remove_item(&child.path().as_ref(), ctx)); } } - counter = try!(remove_item(path, base_dir, counter, false)); - Ok(counter) + ctx.readonly = dir_readonly; + remove_item(path, ctx) +} + +fn remove_item(path: &Path, ctx: &mut RmdirContext) -> io::Result<()> { + if !ctx.readonly { + let mut opts = OpenOptions::new(); + opts.access_mode(c::DELETE); + opts.custom_flags(c::FILE_FLAG_BACKUP_SEMANTICS | // delete directory + c::FILE_FLAG_OPEN_REPARSE_POINT | // delete symlink + c::FILE_FLAG_DELETE_ON_CLOSE); + let file = try!(File::open(path, &opts)); + move_item(&file, ctx) + } else { + // remove read-only permision + try!(set_perm(&path, FilePermissions::new())); + // move and delete file, similar to !readonly. + // only the access mode is different. + let mut opts = OpenOptions::new(); + opts.access_mode(c::DELETE | c::FILE_WRITE_ATTRIBUTES); + opts.custom_flags(c::FILE_FLAG_BACKUP_SEMANTICS | + c::FILE_FLAG_OPEN_REPARSE_POINT | + c::FILE_FLAG_DELETE_ON_CLOSE); + let file = try!(File::open(path, &opts)); + try!(move_item(&file, ctx)); + // restore read-only flag just in case there are other hard links + let mut perm = FilePermissions::new(); + perm.set_readonly(true); + let _ = file.set_perm(perm); // ignore if this fails + Ok(()) + } +} + +fn move_item(file: &File, ctx: &mut RmdirContext) -> io::Result<()> { + let mut tmpname = ctx.base_dir.join(format!{"rm-{}", ctx.counter}); + ctx.counter += 1; + // Try to rename the file. If it already exists, just retry with an other + // filename. + while let Err(err) = file.rename(tmpname.as_ref(), false) { + if err.kind() != io::ErrorKind::AlreadyExists { return Err(err) }; + tmpname = ctx.base_dir.join(format!("rm-{}", ctx.counter)); + ctx.counter += 1; + } + Ok(()) } pub fn readlink(path: &Path) -> io::Result { - // Open the link with no access mode, instead of generic read. + // Open the link with as few permissions as possible, instead of generic read. // By default FILE_LIST_DIRECTORY is denied for the junction "C:\Documents and Settings", so // this is needed for a common case. let mut opts = OpenOptions::new(); - opts.access_mode(0); + opts.access_mode(c::FILE_READ_ATTRIBUTES); opts.custom_flags(c::FILE_FLAG_OPEN_REPARSE_POINT | c::FILE_FLAG_BACKUP_SEMANTICS); let file = try!(File::open(&path, &opts)); @@ -764,8 +790,7 @@ pub fn link(src: &Path, dst: &Path) -> io::Result<()> { pub fn stat(path: &Path) -> io::Result { let mut opts = OpenOptions::new(); - // No read or write permissions are necessary - opts.access_mode(0); + opts.access_mode(c::FILE_READ_ATTRIBUTES); // This flag is so we can open directories too opts.custom_flags(c::FILE_FLAG_BACKUP_SEMANTICS); let file = try!(File::open(path, &opts)); @@ -774,8 +799,7 @@ pub fn stat(path: &Path) -> io::Result { pub fn lstat(path: &Path) -> io::Result { let mut opts = OpenOptions::new(); - // No read or write permissions are necessary - opts.access_mode(0); + opts.access_mode(c::FILE_READ_ATTRIBUTES); opts.custom_flags(c::FILE_FLAG_BACKUP_SEMANTICS | c::FILE_FLAG_OPEN_REPARSE_POINT); let file = try!(File::open(path, &opts)); file.file_attr() @@ -886,3 +910,78 @@ fn symlink_junction_inner(target: &Path, junction: &Path) -> io::Result<()> { ptr::null_mut())).map(|_| ()) } } + +#[cfg(test)] +mod tests { + use prelude::v1::*; + use env; + use fs::{self, File}; + use path::PathBuf; + use rand::{self, Rng}; + use sys::c; + + macro_rules! check { ($e:expr) => ( + match $e { + Ok(t) => t, + Err(e) => panic!("{} failed with: {}", stringify!($e), e), + } + ) } + + macro_rules! error { ($e:expr, $s:expr) => ( + match $e { + Ok(_) => panic!("Unexpected success. Should've been: {:?}", $s), + Err(ref err) => assert!(err.to_string().contains($s), + format!("`{}` did not contain `{}`", err, $s)) + } + ) } + + pub struct TempDir(PathBuf); + + impl TempDir { + fn join(&self, path: &str) -> PathBuf { + let TempDir(ref p) = *self; + p.join(path) + } + } + + impl Drop for TempDir { + fn drop(&mut self) { + // Gee, seeing how we're testing the fs module I sure hope that we + // at least implement this correctly! + let TempDir(ref p) = *self; + check!(fs::remove_dir_all(p)); + } + } + + pub fn tmpdir() -> TempDir { + let p = env::temp_dir(); + let mut r = rand::thread_rng(); + let ret = p.join(&format!("rust-{}", r.next_u32())); + check!(fs::create_dir(&ret)); + TempDir(ret) + } + + #[test] + fn set_perm_preserves_attr() { + let tmpdir = tmpdir(); + let path = tmpdir.join("file"); + check!(File::create(&path)); + + let mut opts = super::OpenOptions::new(); + opts.read(true); + opts.write(true); + let file = check!(super::File::open(&path, &opts)); + + let metadata = check!(file.file_attr()); + let attr = metadata.attributes; + check!(file.set_attributes(attr | c::FILE_ATTRIBUTE_SYSTEM)); + + let mut perm = metadata.perm(); + perm.set_readonly(true); + check!(file.set_perm(perm)); + + let new_metadata = check!(file.file_attr()); + assert!(new_metadata.attributes & c::FILE_ATTRIBUTE_SYSTEM != 0); + assert!(new_metadata.perm().readonly()); + } +} From 89489ece46281e863f4b4662b65885c2ea0d0260 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Sat, 5 Mar 2016 16:44:44 +0100 Subject: [PATCH 5/5] Fix removing the contents of a readonly dir on Unix --- src/libstd/sys/unix/fs.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/libstd/sys/unix/fs.rs b/src/libstd/sys/unix/fs.rs index 250b1b015a069..a10fb556207fa 100644 --- a/src/libstd/sys/unix/fs.rs +++ b/src/libstd/sys/unix/fs.rs @@ -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) } } @@ -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)); + } try!(remove_dir_all_recursive(&child.path())); } else { try!(unlink(&child.path()));