diff --git a/src/chgrp/chgrp.rs b/src/chgrp/chgrp.rs index 2455592de..ff7ed6377 100644 --- a/src/chgrp/chgrp.rs +++ b/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 { diff --git a/src/chown/chown.rs b/src/chown/chown.rs index 8b1fabbad..421ef21e6 100644 --- a/src/chown/chown.rs +++ b/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 { diff --git a/src/uucore/fs.rs b/src/uucore/fs.rs index e2a34d814..d02895c8d 100644 --- a/src/uucore/fs.rs +++ b/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), - _ => unreachable!(), + Component::RootDir | + Component::Normal(_) | + Component::Prefix(_) => result.push(comp.as_os_str()), } } result.into() diff --git a/tests/test_chgrp.rs b/tests/test_chgrp.rs index 06875215c..948046bc6 100644 --- a/tests/test_chgrp.rs +++ b/tests/test_chgrp.rs @@ -43,12 +43,52 @@ fn test_fail_silently() { #[test] fn test_preserve_root() { - new_ucmd!() - .arg("--preserve-root") - .arg("-R") - .arg("bin").arg("/") + // 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(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]