chown,chgrp: fix bug in option --preserve-root

src/chgrp/chgrp.rs

@@ -235,7 +235,12 @@ impl Chgrper {
             let may_exist = if follow_arg {
                 path.canonicalize().ok()
             } else {
-                Some(resolve_relative_path(path).into_owned())
+                let real = resolve_relative_path(path);
+                if real.is_dir() {
+                    Some(real.canonicalize().expect("failed to get real path"))
+                } else {
+                    Some(real.into_owned())
+                }
             };
 
             if let Some(p) = may_exist {

src/chown/chown.rs

@@ -298,7 +298,12 @@ impl Chowner {
             let may_exist = if follow_arg {
                 path.canonicalize().ok()
             } else {
-                Some(resolve_relative_path(path).into_owned())
+                let real = resolve_relative_path(path);
+                if real.is_dir() {
+                    Some(real.canonicalize().expect("failed to get real path"))
+                } else {
+                    Some(real.into_owned())
+                }
             };
 
             if let Some(p) = may_exist {

src/uucore/fs.rs

@@ -16,20 +16,21 @@ use std::io::Result as IOResult;
 use std::path::{Component, Path, PathBuf};
 use std::borrow::Cow;
 
-#[cfg(unix)]
 pub fn resolve_relative_path<'a>(path: &'a Path) -> Cow<'a, Path> {
-    if path.is_absolute() {
+    if path.components().all(|e| e != Component::ParentDir) {
         return path.into();
     }
-    let mut result = env::current_dir().unwrap_or(PathBuf::from("/"));
+    let root = Component::RootDir.as_os_str();
+    let mut result = env::current_dir().unwrap_or(PathBuf::from(root));
     for comp in path.components() {
         match comp {
             Component::ParentDir => {
                 result.pop();
             }
             Component::CurDir => (),
-            Component::Normal(s) => result.push(s),
+            Component::RootDir |
-            _ => unreachable!(),
+            Component::Normal(_) |
+            Component::Prefix(_) => result.push(comp.as_os_str()),
         }
     }
     result.into()

tests/test_chgrp.rs

@@ -43,13 +43,53 @@ fn test_fail_silently() {
 
 #[test]
 fn test_preserve_root() {
+    // It's weird that on OS X, `realpath /etc/..` returns '/private'
+    for d in &["/", "/////tmp///../../../../",
+               "../../../../../../../../../../../../../../",
+               "./../../../../../../../../../../../../../../"] {
         new_ucmd!()
             .arg("--preserve-root")
             .arg("-R")
-        .arg("bin").arg("/")
+            .arg("bin").arg(d)
             .fails()
             .stderr_is("chgrp: it is dangerous to operate recursively on '/'\nchgrp: use --no-preserve-root to override this failsafe");
     }
+}
+
+#[test]
+fn test_preserve_root_symlink() {
+    let file = "test_chgrp_symlink2root";
+    for d in &["/", "////tmp//../../../../",
+               "..//../../..//../..//../../../../../../../../",
+               ".//../../../../../../..//../../../../../../../"] {
+        let (at, mut ucmd) = at_and_ucmd!();
+        at.symlink(d, file);
+        ucmd.arg("--preserve-root")
+            .arg("-HR")
+            .arg("bin").arg(file)
+            .fails()
+            .stderr_is("chgrp: it is dangerous to operate recursively on '/'\nchgrp: use --no-preserve-root to override this failsafe");
+    }
+
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.symlink("///usr", file);
+    ucmd.arg("--preserve-root")
+        .arg("-HR")
+        .arg("bin").arg(format!(".//{}/..//..//../../", file))
+        .fails()
+        .stderr_is("chgrp: it is dangerous to operate recursively on '/'\nchgrp: use --no-preserve-root to override this failsafe");
+
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.symlink("/", "/tmp/__root__");
+    ucmd.arg("--preserve-root")
+        .arg("-R")
+        .arg("bin").arg("/tmp/__root__/.")
+        .fails()
+        .stderr_is("chgrp: it is dangerous to operate recursively on '/'\nchgrp: use --no-preserve-root to override this failsafe");
+
+    use ::std::fs;
+    fs::remove_file("/tmp/__root__").unwrap();
+}
 
 #[test]
 #[cfg(target_os = "linux")]