diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 65357a576..d307b1c73 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -255,7 +255,9 @@ impl Chmoder { } #[cfg(any(unix, target_os = "redox"))] fn chmod_file(&self, file: &Path) -> Result<(), i32> { - let mut fperm = match fs::metadata(file) { + use uucore::mode::get_umask; + + let fperm = match fs::metadata(file) { Ok(meta) => meta.mode() & 0o7777, Err(err) => { if is_symlink(file) { @@ -278,18 +280,30 @@ impl Chmoder { Some(mode) => self.change_file(fperm, mode, file)?, None => { let cmode_unwrapped = self.cmode.clone().unwrap(); + let mut new_mode = fperm; + let mut naively_expected_new_mode = new_mode; for mode in cmode_unwrapped.split(',') { // cmode is guaranteed to be Some in this case let arr: &[char] = &['0', '1', '2', '3', '4', '5', '6', '7', '8', '9']; let result = if mode.contains(arr) { - mode::parse_numeric(fperm, mode, file.is_dir()) + mode::parse_numeric(new_mode, mode, file.is_dir()).map(|v| (v, v)) } else { - mode::parse_symbolic(fperm, mode, file.is_dir()) + mode::parse_symbolic(new_mode, mode, get_umask(), file.is_dir()).map(|m| { + // calculate the new mode as if umask was 0 + let naive_mode = mode::parse_symbolic( + naively_expected_new_mode, + mode, + 0, + file.is_dir(), + ) + .unwrap(); // we know that mode must be valid, so this cannot fail + (m, naive_mode) + }) }; match result { - Ok(mode) => { - self.change_file(fperm, mode, file)?; - fperm = mode; + Ok((mode, naive_mode)) => { + new_mode = mode; + naively_expected_new_mode = naive_mode; } Err(f) => { if !self.quiet { @@ -299,6 +313,17 @@ impl Chmoder { } } } + self.change_file(fperm, new_mode, file)?; + // if a permission would have been removed if umask was 0, but it wasn't because umask was not 0, print an error and fail + if (new_mode & !naively_expected_new_mode) != 0 { + show_error!( + "{}: new permissions are {}, not {}", + file.display(), + display_permissions_unix(new_mode, false), + display_permissions_unix(naively_expected_new_mode, false) + ); + return Err(1); + } } } diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index d5f853de7..72b7ddea8 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.rs @@ -18,6 +18,7 @@ use filetime::{set_file_times, FileTime}; use uucore::backup_control::{self, BackupMode}; use uucore::entries::{grp2gid, usr2uid}; use uucore::error::{FromIo, UError, UIoError, UResult, USimpleError}; +use uucore::mode::get_umask; use uucore::perms::{wrap_chown, Verbosity, VerbosityLevel}; use libc::{getegid, geteuid}; @@ -378,7 +379,7 @@ fn behavior(matches: &ArgMatches) -> UResult { let specified_mode: Option = if matches.is_present(OPT_MODE) { let x = matches.value_of(OPT_MODE).ok_or(1)?; - Some(mode::parse(x, considering_dir).map_err(|err| { + Some(mode::parse(x, considering_dir, get_umask()).map_err(|err| { show_error!("Invalid mode string: {}", err); 1 })?) diff --git a/src/uu/install/src/mode.rs b/src/uu/install/src/mode.rs index 5c3f8f28f..fd4cee50e 100644 --- a/src/uu/install/src/mode.rs +++ b/src/uu/install/src/mode.rs @@ -4,14 +4,14 @@ use std::path::Path; use uucore::mode; /// Takes a user-supplied string and tries to parse to u16 mode bitmask. -pub fn parse(mode_string: &str, considering_dir: bool) -> Result { +pub fn parse(mode_string: &str, considering_dir: bool, umask: u32) -> Result { let numbers: &[char] = &['0', '1', '2', '3', '4', '5', '6', '7', '8', '9']; // Passing 000 as the existing permissions seems to mirror GNU behavior. if mode_string.contains(numbers) { mode::parse_numeric(0, mode_string, considering_dir) } else { - mode::parse_symbolic(0, mode_string, considering_dir) + mode::parse_symbolic(0, mode_string, umask, considering_dir) } } diff --git a/src/uucore/src/lib/features/mode.rs b/src/uucore/src/lib/features/mode.rs index 9ddb8540f..4c150ff27 100644 --- a/src/uucore/src/lib/features/mode.rs +++ b/src/uucore/src/lib/features/mode.rs @@ -7,7 +7,7 @@ // spell-checker:ignore (vars) fperm srwx -use libc::{mode_t, S_IRGRP, S_IROTH, S_IRUSR, S_IWGRP, S_IWOTH, S_IWUSR}; +use libc::{mode_t, umask, S_IRGRP, S_IROTH, S_IRUSR, S_IWGRP, S_IWOTH, S_IWUSR}; pub fn parse_numeric(fperm: u32, mut mode: &str, considering_dir: bool) -> Result { let (op, pos) = parse_op(mode).map_or_else(|_| (None, 0), |(op, pos)| (Some(op), pos)); @@ -35,24 +35,21 @@ pub fn parse_numeric(fperm: u32, mut mode: &str, considering_dir: bool) -> Resul pub fn parse_symbolic( mut fperm: u32, mut mode: &str, + umask: u32, considering_dir: bool, ) -> Result { - #[cfg(unix)] - use libc::umask; - let (mask, pos) = parse_levels(mode); if pos == mode.len() { return Err(format!("invalid mode ({})", mode)); } let respect_umask = pos == 0; - let last_umask = unsafe { umask(0) }; mode = &mode[pos..]; while !mode.is_empty() { let (op, pos) = parse_op(mode)?; mode = &mode[pos..]; let (mut srwx, pos) = parse_change(mode, fperm, considering_dir); if respect_umask { - srwx &= !(last_umask as u32); + srwx &= !(umask as u32); } mode = &mode[pos..]; match op { @@ -68,9 +65,6 @@ pub fn parse_symbolic( _ => unreachable!(), } } - unsafe { - umask(last_umask); - } Ok(fperm) } @@ -141,11 +135,17 @@ pub fn parse_mode(mode: &str) -> Result { let result = if mode.contains(arr) { parse_numeric(fperm as u32, mode, true) } else { - parse_symbolic(fperm as u32, mode, true) + parse_symbolic(fperm as u32, mode, get_umask(), true) }; result.map(|mode| mode as mode_t) } +pub fn get_umask() -> u32 { + let mask = unsafe { umask(0) }; + unsafe { umask(mask) }; + mask +} + #[cfg(test)] mod test { diff --git a/tests/by-util/test_chmod.rs b/tests/by-util/test_chmod.rs index 667f7b476..6ffc86325 100644 --- a/tests/by-util/test_chmod.rs +++ b/tests/by-util/test_chmod.rs @@ -201,11 +201,6 @@ fn test_chmod_ugoa() { before: 0o100000, after: 0o100755, }, - TestCase { - args: vec!["-w", TEST_FILE], - before: 0o100777, - after: 0o100577, - }, TestCase { args: vec!["-x", TEST_FILE], before: 0o100777, @@ -213,6 +208,21 @@ fn test_chmod_ugoa() { }, ]; run_tests(tests); + + // check that we print an error if umask prevents us from removing a permission + let (at, mut ucmd) = at_and_ucmd!(); + at.touch("file"); + set_permissions(at.plus("file"), Permissions::from_mode(0o777)).unwrap(); + ucmd.args(&["-w", "file"]) + .fails() + .code_is(1) + // spell-checker:disable-next-line + .stderr_is("chmod: file: new permissions are r-xrwxrwx, not r-xr-xr-x"); + assert_eq!( + metadata(at.plus("file")).unwrap().permissions().mode(), + 0o100577 + ); + unsafe { umask(last); }