Merge pull request #983 from knight42/fix-preserve-root

Updated chgrp & chown

src/chgrp/Cargo.toml

@@ -13,7 +13,7 @@ walkdir = "*"
 [dependencies.uucore]
 path = "../uucore"
 default-features = false
-features = ["entries"]
+features = ["entries", "fs"]
 
 [[bin]]
 name = "chgrp"

src/chgrp/chgrp.rs

@@ -12,6 +12,7 @@
 extern crate uucore;
 use uucore::libc::{self, gid_t, lchown};
 pub use uucore::entries;
+use uucore::fs::resolve_relative_path;
 
 extern crate walkdir;
 use walkdir::WalkDir;
@@ -194,12 +195,6 @@ impl Chgrper {
     fn exec(&self) -> i32 {
         let mut ret = 0;
         for f in &self.files {
-            if f == "/" && self.preserve_root && self.recursive {
-                show_info!("it is dangerous to operate recursively on '/'");
-                show_info!("use --no-preserve-root to override this failsafe");
-                ret = 1;
-                continue;
-            }
             ret |= self.traverse(f);
         }
         ret
@@ -230,6 +225,28 @@ impl Chgrper {
             _ => return 1,
         };
 
+        // Prohibit only if:
+        // (--preserve-root and -R present) &&
+        // (
+        //     (argument is not symlink && resolved to be '/') ||
+        //     (argument is symlink && should follow argument && resolved to be '/')
+        // )
+        if self.recursive && self.preserve_root {
+            let may_exist = if follow_arg {
+                path.canonicalize().ok()
+            } else {
+                Some(resolve_relative_path(path).into_owned())
+            };
+
+            if let Some(p) = may_exist {
+                if p.parent().is_none() {
+                    show_info!("it is dangerous to operate recursively on '/'");
+                    show_info!("use --no-preserve-root to override this failsafe");
+                    return 1;
+                }
+            }
+        }
+
         let ret = self.wrap_chgrp(path, &meta, follow_arg);
 
         if !self.recursive {

src/chown/Cargo.toml

@@ -14,7 +14,7 @@ walkdir = "0.1"
 [dependencies.uucore]
 path = "../uucore"
 default-features = false
-features = ["entries"]
+features = ["entries", "fs"]
 
 [dependencies.clippy]
 version = "*"

src/chown/chown.rs

@@ -15,6 +15,7 @@
 extern crate uucore;
 use uucore::libc::{self, uid_t, gid_t, lchown};
 pub use uucore::entries::{self, Locate, Passwd, Group};
+use uucore::fs::resolve_relative_path;
 
 extern crate walkdir;
 use walkdir::WalkDir;
@@ -257,12 +258,6 @@ impl Chowner {
     fn exec(&self) -> i32 {
         let mut ret = 0;
         for f in &self.files {
-            if f == "/" && self.preserve_root && self.recursive {
-                show_info!("it is dangerous to operate recursively on '/'");
-                show_info!("use --no-preserve-root to override this failsafe");
-                ret = 1;
-                continue;
-            }
             ret |= self.traverse(f);
         }
         ret
@@ -293,6 +288,28 @@ impl Chowner {
             _ => return 1,
         };
 
+        // Prohibit only if:
+        // (--preserve-root and -R present) &&
+        // (
+        //     (argument is not symlink && resolved to be '/') ||
+        //     (argument is symlink && should follow argument && resolved to be '/')
+        // )
+        if self.recursive && self.preserve_root {
+            let may_exist = if follow_arg {
+                path.canonicalize().ok()
+            } else {
+                Some(resolve_relative_path(path).into_owned())
+            };
+
+            if let Some(p) = may_exist {
+                if p.parent().is_none() {
+                    show_info!("it is dangerous to operate recursively on '/'");
+                    show_info!("use --no-preserve-root to override this failsafe");
+                    return 1;
+                }
+            }
+        }
+
         let ret = if self.matched(meta.uid(), meta.gid()) {
             self.wrap_chown(path, &meta, follow_arg)
         } else {
@@ -300,10 +317,10 @@ impl Chowner {
         };
 
         if !self.recursive {
-            return ret;
+            ret
+        } else {
+            ret | self.dive_into(&root)
         }
-
-        self.dive_into(&root)
     }
 
     fn dive_into<P: AsRef<Path>>(&self, root: P) -> i32 {
@@ -413,4 +430,3 @@ impl Chowner {
         }
     }
 }
-

src/uucore/fs.rs

@@ -1,18 +1,39 @@
-/*
- * This file is part of the uutils coreutils package.
- *
- * (c) Joseph Crail <jbcrail@gmail.com>
- *
- * For the full copyright and license information, please view the LICENSE
- * file that was distributed with this source code.
- */
+// This file is part of the uutils coreutils package.
+//
+// (c) Joseph Crail <jbcrail@gmail.com>
+// (c) Jian Zeng <anonymousknight96 AT gmail.com>
+//
+// For the full copyright and license information, please view the LICENSE
+// file that was distributed with this source code.
+//
 
 #[cfg(unix)]
 use super::libc;
 use std::env;
 use std::fs;
-use std::io::{Error, ErrorKind, Result};
+use std::io::{Error, ErrorKind};
+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() {
+        return path.into();
+    }
+    let mut result = env::current_dir().unwrap_or(PathBuf::from("/"));
+    for comp in path.components() {
+        match comp {
+            Component::ParentDir => {
+                result.pop();
+            }
+            Component::CurDir => (),
+            Component::Normal(s) => result.push(s),
+            _ => unreachable!(),
+        }
+    }
+    result.into()
+}
 
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
 pub enum CanonicalizeMode {
@@ -22,7 +43,7 @@ pub enum CanonicalizeMode {
     Missing,
 }
 
-fn resolve<P: AsRef<Path>>(original: P) -> Result<PathBuf> {
+fn resolve<P: AsRef<Path>>(original: P) -> IOResult<PathBuf> {
     const MAX_LINKS_FOLLOWED: u32 = 255;
     let mut followed = 0;
     let mut result = original.as_ref().to_path_buf();
@@ -40,7 +61,7 @@ fn resolve<P: AsRef<Path>>(original: P) -> Result<PathBuf> {
                     Ok(path) => {
                         result.pop();
                         result.push(path);
-                    },
+                    }
                     Err(e) => {
                         return Err(e);
                     }
@@ -51,7 +72,7 @@ fn resolve<P: AsRef<Path>>(original: P) -> Result<PathBuf> {
     Ok(result)
 }
 
-pub fn canonicalize<P: AsRef<Path>>(original: P, can_mode: CanonicalizeMode) -> Result<PathBuf> {
+pub fn canonicalize<P: AsRef<Path>>(original: P, can_mode: CanonicalizeMode) -> IOResult<PathBuf> {
     // Create an absolute path
     let original = original.as_ref();
     let original = if original.is_absolute() {
@@ -61,20 +82,21 @@ pub fn canonicalize<P: AsRef<Path>>(original: P, can_mode: CanonicalizeMode) ->
     };
 
     let mut result = PathBuf::new();
-    let mut parts = vec!();
+    let mut parts = vec![];
 
     // Split path by directory separator; add prefix (Windows-only) and root
     // directory to final path buffer; add remaining parts to temporary
     // vector for canonicalization.
     for part in original.components() {
         match part {
-            Component::Prefix(_) | Component::RootDir => {
+            Component::Prefix(_) |
+            Component::RootDir => {
                 result.push(part.as_os_str());
-            },
-            Component::CurDir => {},
+            }
+            Component::CurDir => (),
             Component::ParentDir => {
                 parts.pop();
-            },
+            }
             Component::Normal(_) => {
                 parts.push(part.as_os_str());
             }
@@ -91,10 +113,12 @@ pub fn canonicalize<P: AsRef<Path>>(original: P, can_mode: CanonicalizeMode) ->
             }
 
             match resolve(&result) {
-                Err(e) => match can_mode {
+                Err(e) => {
+                    match can_mode {
                         CanonicalizeMode::Missing => continue,
-                    _ => return Err(e)
-                },
+                        _ => return Err(e),
+                    }
+                }
                 Ok(path) => {
                     result.pop();
                     result.push(path);
@@ -105,7 +129,11 @@ pub fn canonicalize<P: AsRef<Path>>(original: P, can_mode: CanonicalizeMode) ->
         result.push(parts.last().unwrap());
 
         match resolve(&result) {
-            Err(e) => { if can_mode == CanonicalizeMode::Existing { return Err(e); } },
+            Err(e) => {
+                if can_mode == CanonicalizeMode::Existing {
+                    return Err(e);
+                }
+            }
             Ok(path) => {
                 result.pop();
                 result.push(path);