Fix fs::remove_dir_all

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
 ///
@@ -1980,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());

src/libstd/sys/windows/c.rs

@@ -98,7 +98,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;
@@ -112,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)]
@@ -364,6 +367,28 @@ 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_DISPOSITION_INFO {
+    pub DeleteFile: BOOL, // for true use -1, not TRUE
+}
+
 #[repr(C)]
 pub struct FILE_END_OF_FILE_INFO {
     pub EndOfFile: LARGE_INTEGER,
@@ -855,8 +880,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;

src/libstd/sys/windows/fs.rs

@@ -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;
 
@@ -77,8 +78,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 +334,90 @@ 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;
+        #[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 mut data: Vec<u16> = iter::repeat(0u16).take(STRUCT_SIZE/2)
+                                 .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
+            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<File> {
         Ok(File {
             handle: try!(self.handle.duplicate(0, true, c::DUPLICATE_SAME_ACCESS)),
@@ -422,7 +507,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 +555,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 {
@@ -502,9 +584,6 @@ impl FileType {
         *self == FileType::SymlinkDir ||
         *self == FileType::MountPoint
     }
-    pub fn is_symlink_dir(&self) -> bool {
-        *self == FileType::SymlinkDir || *self == FileType::MountPoint
-    }
 }
 
 impl DirBuilder {
@@ -567,23 +646,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<u64> {
+    // 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<u64> {
+        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<u64> {
+        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<PathBuf> {
@@ -640,12 +781,12 @@ pub fn lstat(path: &Path) -> io::Result<FileAttr> {
     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<PathBuf> {