From 080998b6ef82dbb09a8fc470f9f28141756042d2 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Mon, 16 Aug 2021 00:14:50 +0200 Subject: [PATCH 01/11] chmod: pad all file modes to 4 digits --- src/uu/chmod/src/chmod.rs | 4 ++-- tests/by-util/test_chmod.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 5108ec924..bdcafc435 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -323,7 +323,7 @@ impl Chmoder { } if self.verbose { show_error!( - "failed to change mode of file '{}' from {:o} ({}) to {:o} ({})", + "failed to change mode of file '{}' from {:04o} ({}) to {:04o} ({})", file.display(), fperm, display_permissions_unix(fperm as mode_t, false), @@ -335,7 +335,7 @@ impl Chmoder { } else { if self.verbose || self.changes { show_error!( - "mode of '{}' changed from {:o} ({}) to {:o} ({})", + "mode of '{}' changed from {:04o} ({}) to {:04o} ({})", file.display(), fperm, display_permissions_unix(fperm as mode_t, false), diff --git a/tests/by-util/test_chmod.rs b/tests/by-util/test_chmod.rs index 186c645e5..5031105f9 100644 --- a/tests/by-util/test_chmod.rs +++ b/tests/by-util/test_chmod.rs @@ -330,8 +330,8 @@ fn test_chmod_recursive() { .arg("a") .arg("z") .succeeds() - .stderr_contains(&"to 333 (-wx-wx-wx)") - .stderr_contains(&"to 222 (-w--w--w-)"); + .stderr_contains(&"to 0333 (-wx-wx-wx)") + .stderr_contains(&"to 0222 (-w--w--w-)"); assert_eq!(at.metadata("z/y").permissions().mode(), 0o100222); assert_eq!(at.metadata("a/a").permissions().mode(), 0o100222); From 74958794c6619f6cc1cf00f5c0834cea5a253017 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Mon, 16 Aug 2021 00:18:45 +0200 Subject: [PATCH 02/11] chmod: print change message to stdout, not stderr --- src/uu/chmod/src/chmod.rs | 4 ++-- tests/by-util/test_chmod.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index bdcafc435..874c78dea 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -322,7 +322,7 @@ impl Chmoder { show_error!("{}", err); } if self.verbose { - show_error!( + println!( "failed to change mode of file '{}' from {:04o} ({}) to {:04o} ({})", file.display(), fperm, @@ -334,7 +334,7 @@ impl Chmoder { Err(1) } else { if self.verbose || self.changes { - show_error!( + println!( "mode of '{}' changed from {:04o} ({}) to {:04o} ({})", file.display(), fperm, diff --git a/tests/by-util/test_chmod.rs b/tests/by-util/test_chmod.rs index 5031105f9..f0973c712 100644 --- a/tests/by-util/test_chmod.rs +++ b/tests/by-util/test_chmod.rs @@ -330,8 +330,8 @@ fn test_chmod_recursive() { .arg("a") .arg("z") .succeeds() - .stderr_contains(&"to 0333 (-wx-wx-wx)") - .stderr_contains(&"to 0222 (-w--w--w-)"); + .stdout_contains(&"to 0333 (-wx-wx-wx)") + .stdout_contains(&"to 0222 (-w--w--w-)"); assert_eq!(at.metadata("z/y").permissions().mode(), 0o100222); assert_eq!(at.metadata("a/a").permissions().mode(), 0o100222); From 9697c89d174e896f5454a233e570916cbfa9b2ff Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Mon, 16 Aug 2021 00:30:25 +0200 Subject: [PATCH 03/11] uucore/mode: handle mode 0 Trimming all '0' characters results in an invalid string if the string contained only '0's. --- src/uucore/src/lib/features/mode.rs | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/uucore/src/lib/features/mode.rs b/src/uucore/src/lib/features/mode.rs index 8e9f063ff..eb5bfc580 100644 --- a/src/uucore/src/lib/features/mode.rs +++ b/src/uucore/src/lib/features/mode.rs @@ -11,19 +11,21 @@ use libc::{mode_t, S_IRGRP, S_IROTH, S_IRUSR, S_IWGRP, S_IWOTH, S_IWUSR}; pub fn parse_numeric(fperm: u32, mut mode: &str) -> Result { let (op, pos) = parse_op(mode, Some('='))?; - mode = mode[pos..].trim().trim_start_matches('0'); - if mode.len() > 4 { - Err(format!("mode is too large ({} > 7777)", mode)) - } else { - match u32::from_str_radix(mode, 8) { - Ok(change) => Ok(match op { - '+' => fperm | change, - '-' => fperm & !change, - '=' => change, - _ => unreachable!(), - }), - Err(err) => Err(err.to_string()), + mode = mode[pos..].trim(); + match u32::from_str_radix(mode, 8) { + Ok(change) => { + if change > 0o7777 { + Err(format!("mode is too large ({} > 7777", mode)) + } else { + Ok(match op { + '+' => fperm | change, + '-' => fperm & !change, + '=' => change, + _ => unreachable!(), + }) + } } + Err(err) => Err(err.to_string()), } } From 15b40f6aa2d325e1362309f0d82115c18c7dbd83 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Mon, 16 Aug 2021 21:56:09 +0200 Subject: [PATCH 04/11] chmod: implement special handling of directory setuid/setgid --- src/uu/chmod/src/chmod.rs | 2 +- src/uu/install/src/mode.rs | 2 +- src/uucore/src/lib/features/mode.rs | 73 ++++++++++++++++------------- tests/by-util/test_chmod.rs | 25 +++++++++- 4 files changed, 66 insertions(+), 36 deletions(-) diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 874c78dea..65357a576 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -282,7 +282,7 @@ impl Chmoder { // 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) + mode::parse_numeric(fperm, mode, file.is_dir()) } else { mode::parse_symbolic(fperm, mode, file.is_dir()) }; diff --git a/src/uu/install/src/mode.rs b/src/uu/install/src/mode.rs index b8d5cd839..5c3f8f28f 100644 --- a/src/uu/install/src/mode.rs +++ b/src/uu/install/src/mode.rs @@ -9,7 +9,7 @@ pub fn parse(mode_string: &str, considering_dir: bool) -> Result { // Passing 000 as the existing permissions seems to mirror GNU behavior. if mode_string.contains(numbers) { - mode::parse_numeric(0, mode_string) + mode::parse_numeric(0, mode_string, considering_dir) } else { mode::parse_symbolic(0, mode_string, considering_dir) } diff --git a/src/uucore/src/lib/features/mode.rs b/src/uucore/src/lib/features/mode.rs index eb5bfc580..9ddb8540f 100644 --- a/src/uucore/src/lib/features/mode.rs +++ b/src/uucore/src/lib/features/mode.rs @@ -9,23 +9,26 @@ use libc::{mode_t, S_IRGRP, S_IROTH, S_IRUSR, S_IWGRP, S_IWOTH, S_IWUSR}; -pub fn parse_numeric(fperm: u32, mut mode: &str) -> Result { - let (op, pos) = parse_op(mode, Some('='))?; +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)); mode = mode[pos..].trim(); - match u32::from_str_radix(mode, 8) { - Ok(change) => { - if change > 0o7777 { - Err(format!("mode is too large ({} > 7777", mode)) - } else { - Ok(match op { - '+' => fperm | change, - '-' => fperm & !change, - '=' => change, - _ => unreachable!(), - }) - } - } - Err(err) => Err(err.to_string()), + let change = if mode.is_empty() { + 0 + } else { + u32::from_str_radix(mode, 8).map_err(|e| e.to_string())? + }; + if change > 0o7777 { + Err(format!("mode is too large ({} > 7777", change)) + } else { + Ok(match op { + Some('+') => fperm | change, + Some('-') => fperm & !change, + // If this is a directory, we keep the setgid and setuid bits, + // unless the mode contains 5 or more octal digits or the mode is "=" + None if considering_dir && mode.len() < 5 => change | (fperm & (0o4000 | 0o2000)), + None | Some('=') => change, + Some(_) => unreachable!(), + }) } } @@ -45,7 +48,7 @@ pub fn parse_symbolic( let last_umask = unsafe { umask(0) }; mode = &mode[pos..]; while !mode.is_empty() { - let (op, pos) = parse_op(mode, None)?; + let (op, pos) = parse_op(mode)?; mode = &mode[pos..]; let (mut srwx, pos) = parse_change(mode, fperm, considering_dir); if respect_umask { @@ -55,7 +58,13 @@ pub fn parse_symbolic( match op { '+' => fperm |= srwx & mask, '-' => fperm &= !(srwx & mask), - '=' => fperm = (fperm & !mask) | (srwx & mask), + '=' => { + if considering_dir { + // keep the setgid and setuid bits for directories + srwx |= fperm & (0o4000 | 0o2000); + } + fperm = (fperm & !mask) | (srwx & mask) + } _ => unreachable!(), } } @@ -70,9 +79,9 @@ fn parse_levels(mode: &str) -> (u32, usize) { let mut pos = 0; for ch in mode.chars() { mask |= match ch { - 'u' => 0o7700, - 'g' => 0o7070, - 'o' => 0o7007, + 'u' => 0o4700, + 'g' => 0o2070, + 'o' => 0o1007, 'a' => 0o7777, _ => break, }; @@ -84,24 +93,22 @@ fn parse_levels(mode: &str) -> (u32, usize) { (mask, pos) } -fn parse_op(mode: &str, default: Option) -> Result<(char, usize), String> { +fn parse_op(mode: &str) -> Result<(char, usize), String> { let ch = mode .chars() .next() .ok_or_else(|| "unexpected end of mode".to_owned())?; - Ok(match ch { - '+' | '-' | '=' => (ch, 1), - _ => { - let ch = default.ok_or_else(|| { - format!("invalid operator (expected +, -, or =, but found {})", ch) - })?; - (ch, 0) - } - }) + match ch { + '+' | '-' | '=' => Ok((ch, 1)), + _ => Err(format!( + "invalid operator (expected +, -, or =, but found {})", + ch + )), + } } fn parse_change(mode: &str, fperm: u32, considering_dir: bool) -> (u32, usize) { - let mut srwx = fperm & 0o7000; + let mut srwx = 0; let mut pos = 0; for ch in mode.chars() { match ch { @@ -132,7 +139,7 @@ pub fn parse_mode(mode: &str) -> Result { let fperm = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH; let arr: &[char] = &['0', '1', '2', '3', '4', '5', '6', '7', '8', '9']; let result = if mode.contains(arr) { - parse_numeric(fperm as u32, mode) + parse_numeric(fperm as u32, mode, true) } else { parse_symbolic(fperm as u32, mode, true) }; diff --git a/tests/by-util/test_chmod.rs b/tests/by-util/test_chmod.rs index f0973c712..667f7b476 100644 --- a/tests/by-util/test_chmod.rs +++ b/tests/by-util/test_chmod.rs @@ -1,5 +1,5 @@ use crate::common::util::*; -use std::fs::{metadata, set_permissions, OpenOptions}; +use std::fs::{metadata, set_permissions, OpenOptions, Permissions}; use std::os::unix::fs::{OpenOptionsExt, PermissionsExt}; use std::sync::Mutex; @@ -490,3 +490,26 @@ fn test_chmod_strip_minus_from_mode() { assert_eq!(test.1, args.join(" ")); } } + +#[test] +fn test_chmod_keep_setgid() { + for &(from, arg, to) in &[ + (0o7777, "777", 0o46777), + (0o7777, "=777", 0o40777), + (0o7777, "0777", 0o46777), + (0o7777, "=0777", 0o40777), + (0o7777, "00777", 0o40777), + (0o2444, "a+wx", 0o42777), + (0o2444, "a=wx", 0o42333), + (0o1444, "g+s", 0o43444), + (0o4444, "u-s", 0o40444), + (0o7444, "a-s", 0o41444), + ] { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("dir"); + set_permissions(at.plus("dir"), Permissions::from_mode(from)).unwrap(); + let r = ucmd.arg(arg).arg("dir").succeeds(); + println!("{}", r.stderr_str()); + assert_eq!(at.metadata("dir").permissions().mode(), to); + } +} From 945e57ea22642b33ba88b85f9819a8a85d8a9fdd Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Tue, 17 Aug 2021 00:20:44 +0200 Subject: [PATCH 05/11] chmod: show an error if a permission wasn't removed due to umask --- src/uu/chmod/src/chmod.rs | 37 ++++++++++++++++++++++++----- src/uu/install/src/install.rs | 3 ++- src/uu/install/src/mode.rs | 4 ++-- src/uucore/src/lib/features/mode.rs | 20 ++++++++-------- tests/by-util/test_chmod.rs | 20 ++++++++++++---- 5 files changed, 60 insertions(+), 24 deletions(-) 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); } From b841a114211cea27c8f8026695182c885da421c9 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Tue, 17 Aug 2021 15:27:34 +0200 Subject: [PATCH 06/11] chmod: fail if the operand list is empty --- src/uu/chmod/src/chmod.rs | 4 ++++ tests/by-util/test_chmod.rs | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index d307b1c73..6b165d198 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -98,6 +98,10 @@ pub fn uumain(args: impl uucore::Args) -> i32 { Some(cmode) }; + if files.is_empty() { + crash!(1, "missing operand"); + } + let chmoder = Chmoder { changes, quiet, diff --git a/tests/by-util/test_chmod.rs b/tests/by-util/test_chmod.rs index 6ffc86325..5106b4fe6 100644 --- a/tests/by-util/test_chmod.rs +++ b/tests/by-util/test_chmod.rs @@ -523,3 +523,12 @@ fn test_chmod_keep_setgid() { assert_eq!(at.metadata("dir").permissions().mode(), to); } } + +#[test] +fn test_no_operands() { + new_ucmd!() + .arg("777") + .fails() + .code_is(1) + .stderr_is("chmod: missing operand"); +} From 5825889931f29f3583af9c1ff7e8796ae4497b72 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Tue, 17 Aug 2021 15:35:14 +0200 Subject: [PATCH 07/11] chmod: correctly handle modes after -- --- src/uu/chmod/src/chmod.rs | 3 +++ tests/by-util/test_chmod.rs | 14 ++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 6b165d198..60705526b 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -184,6 +184,9 @@ pub fn uu_app() -> App<'static, 'static> { // e.g. "chmod -v -xw -R FILE" -> "chmod -v xw -R FILE" pub fn strip_minus_from_mode(args: &mut Vec) -> bool { for arg in args { + if arg == "--" { + break; + } if arg.starts_with('-') { if let Some(second) = arg.chars().nth(1) { match second { diff --git a/tests/by-util/test_chmod.rs b/tests/by-util/test_chmod.rs index 5106b4fe6..9478c141b 100644 --- a/tests/by-util/test_chmod.rs +++ b/tests/by-util/test_chmod.rs @@ -532,3 +532,17 @@ fn test_no_operands() { .code_is(1) .stderr_is("chmod: missing operand"); } + +#[test] +fn test_mode_after_dash_dash() { + let (at, ucmd) = at_and_ucmd!(); + run_single_test( + &TestCase { + args: vec!["--", "-r", TEST_FILE], + before: 0o100777, + after: 0o100333, + }, + at, + ucmd, + ); +} From 38afdd6ab4ee30a61e5ebcddab9c6b39b6a4b085 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Tue, 17 Aug 2021 15:42:50 +0200 Subject: [PATCH 08/11] uucore/mode: add cast for some platforms --- src/uu/chmod/src/chmod.rs | 4 ++-- src/uucore/src/lib/features/mode.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 60705526b..8502148bb 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -326,8 +326,8 @@ impl Chmoder { show_error!( "{}: new permissions are {}, not {}", file.display(), - display_permissions_unix(new_mode, false), - display_permissions_unix(naively_expected_new_mode, false) + display_permissions_unix(new_mode as mode_t, false), + display_permissions_unix(naively_expected_new_mode as mode_t, false) ); return Err(1); } diff --git a/src/uucore/src/lib/features/mode.rs b/src/uucore/src/lib/features/mode.rs index 4c150ff27..794cda418 100644 --- a/src/uucore/src/lib/features/mode.rs +++ b/src/uucore/src/lib/features/mode.rs @@ -143,7 +143,7 @@ pub fn parse_mode(mode: &str) -> Result { pub fn get_umask() -> u32 { let mask = unsafe { umask(0) }; unsafe { umask(mask) }; - mask + mask as u32 } #[cfg(test)] From 5121e2eec1ddbb6e640252ab41fa0011eba11c92 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Tue, 17 Aug 2021 17:02:15 +0200 Subject: [PATCH 09/11] chmod: add check for quiet --- src/uu/chmod/src/chmod.rs | 2 +- tests/by-util/test_chmod.rs | 13 ++++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index 8502148bb..bf6e81305 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -229,7 +229,7 @@ impl Chmoder { if !self.quiet { show_error!("cannot operate on dangling symlink '{}'", filename); } - } else { + } else if !self.quiet { show_error!("cannot access '{}': No such file or directory", filename); } return Err(1); diff --git a/tests/by-util/test_chmod.rs b/tests/by-util/test_chmod.rs index 9478c141b..1b8983bc3 100644 --- a/tests/by-util/test_chmod.rs +++ b/tests/by-util/test_chmod.rs @@ -360,13 +360,24 @@ fn test_chmod_recursive() { fn test_chmod_non_existing_file() { new_ucmd!() .arg("-R") - .arg("--verbose") .arg("-r,a+w") .arg("does-not-exist") .fails() .stderr_contains(&"cannot access 'does-not-exist': No such file or directory"); } +#[test] +fn test_chmod_non_existing_file_silent() { + new_ucmd!() + .arg("-R") + .arg("--quiet") + .arg("-r,a+w") + .arg("does-not-exist") + .fails() + .no_stderr() + .code_is(1); +} + #[test] fn test_chmod_preserve_root() { new_ucmd!() From 5ab05a92193fc8cc6f44a8b7944cef889543c5ff Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Tue, 17 Aug 2021 17:03:11 +0200 Subject: [PATCH 10/11] chmod: remove redundant cfg redox is unix. --- src/uu/chmod/src/chmod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index bf6e81305..26b3fd85b 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -260,7 +260,7 @@ impl Chmoder { // instead it just sets the readonly attribute on the file Err(0) } - #[cfg(any(unix, target_os = "redox"))] + #[cfg(unix)] fn chmod_file(&self, file: &Path) -> Result<(), i32> { use uucore::mode::get_umask; From 0525d9bf6ae10a6df95a9ef57b8427ea3113c401 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 28 Aug 2021 11:44:34 +0200 Subject: [PATCH 11/11] Remove useless import of USimpleError --- src/uu/install/src/install.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index 548ffec59..b72721d20 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.rs @@ -17,7 +17,7 @@ use file_diff::diff; 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::error::{FromIo, UError, UIoError, UResult}; use uucore::mode::get_umask; use uucore::perms::{wrap_chown, Verbosity, VerbosityLevel};