From 2647a72e9ec77270070b169863e21714e988f4f8 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Mon, 29 Mar 2021 22:07:09 +0200 Subject: [PATCH] chmod: fixed behavior for dangling symlinks (#1775) --- src/uu/chmod/src/chmod.rs | 38 +++++++----- tests/by-util/test_chmod.rs | 120 ++++++++++++++++++++++++++++++------ 2 files changed, 123 insertions(+), 35 deletions(-) diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 819c674a0..d9d8c8cf2 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -206,7 +206,17 @@ impl Chmoder { let filename = &filename[..]; let file = Path::new(filename); if !file.exists() { - show_error!("no such file or directory '{}'", filename); + if is_symlink(file) { + println!( + "failed to change mode of '{}' from 0000 (---------) to 0000 (---------)", + filename + ); + if !self.quiet { + show_error!("cannot operate on dangling symlink '{}'", filename); + } + } else { + show_error!("cannot access '{}': No such file or directory", filename); + } return Err(1); } if self.recursive && self.preserve_root && filename == "/" { @@ -240,18 +250,16 @@ impl Chmoder { let mut fperm = match fs::metadata(file) { Ok(meta) => meta.mode() & 0o7777, Err(err) => { - if !self.quiet { - if is_symlink(file) { - if self.verbose { - show_info!( - "neither symbolic link '{}' nor referent has been changed", - file.display() - ); - } - return Ok(()); - } else { - show_error!("{}: '{}'", err, file.display()); + if is_symlink(file) { + if self.verbose { + println!( + "neither symbolic link '{}' nor referent has been changed", + file.display() + ); } + return Ok(()); + } else { + show_error!("{}: '{}'", err, file.display()); } return Err(1); } @@ -291,11 +299,11 @@ impl Chmoder { fn change_file(&self, fperm: u32, mode: u32, file: &Path) -> Result<(), i32> { if fperm == mode { if self.verbose && !self.changes { - show_info!( - "mode of '{}' retained as {:o} ({})", + println!( + "mode of '{}' retained as {:04o} ({})", file.display(), fperm, - display_permissions_unix(fperm) + display_permissions_unix(fperm), ); } Ok(()) diff --git a/tests/by-util/test_chmod.rs b/tests/by-util/test_chmod.rs index 3a53202fc..78fee462d 100644 --- a/tests/by-util/test_chmod.rs +++ b/tests/by-util/test_chmod.rs @@ -329,10 +329,9 @@ fn test_chmod_non_existing_file() { .arg("-r,a+w") .arg("dont-exist") .fails(); - assert_eq!( - result.stderr, - "chmod: error: no such file or directory 'dont-exist'\n" - ); + assert!(result + .stderr + .contains("cannot access 'dont-exist': No such file or directory")); } #[test] @@ -351,30 +350,111 @@ fn test_chmod_preserve_root() { #[test] fn test_chmod_symlink_non_existing_file() { - let (at, mut ucmd) = at_and_ucmd!(); - at.symlink_file("/non-existing", "test-long.link"); + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; - let _result = ucmd - .arg("-R") + let non_existing = "test_chmod_symlink_non_existing_file"; + let test_symlink = "test_chmod_symlink_non_existing_file_symlink"; + let expected_stdout = &format!( + "failed to change mode of '{}' from 0000 (---------) to 0000 (---------)", + test_symlink + ); + let expected_stderr = &format!("cannot operate on dangling symlink '{}'", test_symlink); + + at.symlink_file(non_existing, test_symlink); + let mut result; + + // this cannot succeed since the symbolic link dangles + result = scene.ucmd().arg("755").arg("-v").arg(test_symlink).fails(); + + println!("stdout = {:?}", result.stdout); + println!("stderr = {:?}", result.stderr); + + assert!(result.stdout.contains(expected_stdout)); + assert!(result.stderr.contains(expected_stderr)); + assert_eq!(result.code, Some(1)); + + // this should be the same than with just '-v' but without stderr + result = scene + .ucmd() .arg("755") .arg("-v") - .arg("test-long.link") + .arg("-f") + .arg(test_symlink) .fails(); + + println!("stdout = {:?}", result.stdout); + println!("stderr = {:?}", result.stderr); + + assert!(result.stdout.contains(expected_stdout)); + assert!(result.stderr.is_empty()); + assert_eq!(result.code, Some(1)); } #[test] -fn test_chmod_symlink_non_existing_recursive() { - let (at, mut ucmd) = at_and_ucmd!(); - at.mkdir("tmp"); - at.symlink_file("/non-existing", "tmp/test-long.link"); +fn test_chmod_symlink_non_existing_file_recursive() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; - let result = ucmd.arg("-R").arg("755").arg("-v").arg("tmp").succeeds(); - // it should be a success - println!("stderr {}", result.stderr); - println!("stdout {}", result.stdout); - assert!(result - .stderr - .contains("neither symbolic link 'tmp/test-long.link' nor referent has been changed")); + let non_existing = "test_chmod_symlink_non_existing_file_recursive"; + let test_symlink = "test_chmod_symlink_non_existing_file_recursive_symlink"; + let test_directory = "test_chmod_symlink_non_existing_file_directory"; + + at.mkdir(test_directory); + at.symlink_file( + non_existing, + &format!("{}/{}", test_directory, test_symlink), + ); + let mut result; + + // this should succeed + result = scene + .ucmd() + .arg("-R") + .arg("755") + .arg(test_directory) + .succeeds(); + assert_eq!(result.code, Some(0)); + assert!(result.stdout.is_empty()); + assert!(result.stderr.is_empty()); + + let expected_stdout = &format!( + "mode of '{}' retained as 0755 (rwxr-xr-x)\nneither symbolic link '{}/{}' nor referent has been changed", + test_directory, test_directory, test_symlink + ); + + // '-v': this should succeed without stderr + result = scene + .ucmd() + .arg("-R") + .arg("-v") + .arg("755") + .arg(test_directory) + .succeeds(); + + println!("stdout = {:?}", result.stdout); + println!("stderr = {:?}", result.stderr); + + assert!(result.stdout.contains(expected_stdout)); + assert!(result.stderr.is_empty()); + assert_eq!(result.code, Some(0)); + + // '-vf': this should be the same than with just '-v' + result = scene + .ucmd() + .arg("-R") + .arg("-v") + .arg("-f") + .arg("755") + .arg(test_directory) + .succeeds(); + + println!("stdout = {:?}", result.stdout); + println!("stderr = {:?}", result.stderr); + + assert!(result.stdout.contains(expected_stdout)); + assert!(result.stderr.is_empty()); + assert_eq!(result.code, Some(0)); } #[test]