From d47036cbd1e181da6eca7f4f125e092bedeb35e0 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Sun, 7 Feb 2016 19:31:14 +0100 Subject: [PATCH 1/2] Don't let `remove_dir_all` recursively remove a symlink See #29412 --- src/libstd/fs.rs | 34 ++++++++++++++++++++++++++++++++++ src/libstd/sys/unix/fs.rs | 11 ++++++++++- src/libstd/sys/windows/fs.rs | 13 ++++++++++++- 3 files changed, 56 insertions(+), 2 deletions(-) diff --git a/src/libstd/fs.rs b/src/libstd/fs.rs index d1f0fe517e246..e7707d29f7f66 100644 --- a/src/libstd/fs.rs +++ b/src/libstd/fs.rs @@ -1881,12 +1881,46 @@ mod tests { check!(fs::create_dir_all(&d2)); check!(check!(File::create(&canary)).write(b"foo")); check!(symlink_junction(&d2, &dt.join("d2"))); + check!(symlink_file(&canary, &d1.join("canary"))); check!(fs::remove_dir_all(&d1)); assert!(!d1.is_dir()); assert!(canary.exists()); } + #[test] + fn recursive_rmdir_of_symlink() { + // test we do not recursively delete a symlink but only dirs. + let tmpdir = tmpdir(); + let link = tmpdir.join("d1"); + let dir = tmpdir.join("d2"); + let canary = dir.join("do_not_delete"); + check!(fs::create_dir_all(&dir)); + check!(check!(File::create(&canary)).write(b"foo")); + check!(symlink_junction(&dir, &link)); + check!(fs::remove_dir_all(&link)); + + assert!(!link.is_dir()); + assert!(canary.exists()); + } + + #[test] + // only Windows makes a distinction between file and directory symlinks. + #[cfg(windows)] + fn recursive_rmdir_of_file_symlink() { + let tmpdir = tmpdir(); + if !got_symlink_permission(&tmpdir) { return }; + + let f1 = tmpdir.join("f1"); + let f2 = tmpdir.join("f2"); + check!(check!(File::create(&f1)).write(b"foo")); + check!(symlink_file(&f1, &f2)); + match fs::remove_dir_all(&f2) { + Ok(..) => panic!("wanted a failure"), + Err(..) => {} + } + } + #[test] fn unicode_path_is_dir() { assert!(Path::new(".").is_dir()); diff --git a/src/libstd/sys/unix/fs.rs b/src/libstd/sys/unix/fs.rs index a2e6f467b17c0..b9385ebe5c51b 100644 --- a/src/libstd/sys/unix/fs.rs +++ b/src/libstd/sys/unix/fs.rs @@ -511,10 +511,19 @@ 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() { + unlink(path) + } else { + remove_dir_all_recursive(path) + } +} + +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() { - try!(remove_dir_all(&child.path())); + try!(remove_dir_all_recursive(&child.path())); } else { try!(unlink(&child.path())); } diff --git a/src/libstd/sys/windows/fs.rs b/src/libstd/sys/windows/fs.rs index 9f9290b751d84..b0decfa463fd5 100644 --- a/src/libstd/sys/windows/fs.rs +++ b/src/libstd/sys/windows/fs.rs @@ -525,11 +525,22 @@ 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 { + remove_dir_all_recursive(path) + } +} + +fn remove_dir_all_recursive(path: &Path) -> io::Result<()> { 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(&child.path())); + try!(remove_dir_all_recursive(&child.path())); } else if child_type.is_symlink_dir() { try!(rmdir(&child.path())); } else { From d1bfe9bccf8a1632ffb8d29ee97be137a0e71045 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Sun, 7 Feb 2016 21:10:29 +0100 Subject: [PATCH 2/2] Ignore if we can't create a symlink in this test --- src/libstd/fs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstd/fs.rs b/src/libstd/fs.rs index e7707d29f7f66..eb2c6276fb530 100644 --- a/src/libstd/fs.rs +++ b/src/libstd/fs.rs @@ -1881,7 +1881,7 @@ mod tests { check!(fs::create_dir_all(&d2)); check!(check!(File::create(&canary)).write(b"foo")); check!(symlink_junction(&d2, &dt.join("d2"))); - check!(symlink_file(&canary, &d1.join("canary"))); + let _ = symlink_file(&canary, &d1.join("canary")); check!(fs::remove_dir_all(&d1)); assert!(!d1.is_dir());