diff --git a/src/uu/chgrp/src/chgrp.rs b/src/uu/chgrp/src/chgrp.rs index d37da578e..a077ab7a4 100644 --- a/src/uu/chgrp/src/chgrp.rs +++ b/src/uu/chgrp/src/chgrp.rs @@ -11,46 +11,16 @@ extern crate uucore; pub use uucore::entries; use uucore::error::{FromIo, UResult, USimpleError}; -use uucore::perms::{ - ChownExecutor, IfFrom, Verbosity, VerbosityLevel, FTS_COMFOLLOW, FTS_LOGICAL, FTS_PHYSICAL, -}; +use uucore::perms::{chown_base, options, IfFrom}; -use clap::{App, Arg}; +use clap::{App, Arg, ArgMatches}; use std::fs; use std::os::unix::fs::MetadataExt; -use uucore::InvalidEncodingHandling; - static ABOUT: &str = "Change the group of each FILE to GROUP."; static VERSION: &str = env!("CARGO_PKG_VERSION"); -pub mod options { - pub mod verbosity { - pub static CHANGES: &str = "changes"; - pub static QUIET: &str = "quiet"; - pub static SILENT: &str = "silent"; - pub static VERBOSE: &str = "verbose"; - } - pub mod preserve_root { - pub static PRESERVE: &str = "preserve-root"; - pub static NO_PRESERVE: &str = "no-preserve-root"; - } - pub mod dereference { - pub static DEREFERENCE: &str = "dereference"; - pub static NO_DEREFERENCE: &str = "no-dereference"; - } - pub static RECURSIVE: &str = "recursive"; - pub mod traverse { - pub static TRAVERSE: &str = "H"; - pub static NO_TRAVERSE: &str = "P"; - pub static EVERY: &str = "L"; - } - pub static REFERENCE: &str = "reference"; - pub static ARG_GROUP: &str = "GROUP"; - pub static ARG_FILES: &str = "FILE"; -} - fn get_usage() -> String { format!( "{0} [OPTION]... GROUP FILE...\n {0} [OPTION]... --reference=RFILE FILE...", @@ -58,101 +28,7 @@ fn get_usage() -> String { ) } -#[uucore_procs::gen_uumain] -pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let args = args - .collect_str(InvalidEncodingHandling::ConvertLossy) - .accept_any(); - - let usage = get_usage(); - - let mut app = uu_app().usage(&usage[..]); - - // we change the positional args based on whether - // --reference was used. - let mut reference = false; - let mut help = false; - // stop processing options on -- - for arg in args.iter().take_while(|s| *s != "--") { - if arg.starts_with("--reference=") || arg == "--reference" { - reference = true; - } else if arg == "--help" { - // we stop processing once we see --help, - // as it doesn't matter if we've seen reference or not - help = true; - break; - } - } - - if help || !reference { - // add both positional arguments - app = app.arg( - Arg::with_name(options::ARG_GROUP) - .value_name(options::ARG_GROUP) - .required(true) - .takes_value(true) - .multiple(false), - ) - } - app = app.arg( - Arg::with_name(options::ARG_FILES) - .value_name(options::ARG_FILES) - .multiple(true) - .takes_value(true) - .required(true) - .min_values(1), - ); - - let matches = app.get_matches_from(args); - - /* Get the list of files */ - let files: Vec = matches - .values_of(options::ARG_FILES) - .map(|v| v.map(ToString::to_string).collect()) - .unwrap_or_default(); - - let preserve_root = matches.is_present(options::preserve_root::PRESERVE); - - let mut derefer = if matches.is_present(options::dereference::DEREFERENCE) { - 1 - } else if matches.is_present(options::dereference::NO_DEREFERENCE) { - 0 - } else { - -1 - }; - - let mut bit_flag = if matches.is_present(options::traverse::TRAVERSE) { - FTS_COMFOLLOW | FTS_PHYSICAL - } else if matches.is_present(options::traverse::EVERY) { - FTS_LOGICAL - } else { - FTS_PHYSICAL - }; - - let recursive = matches.is_present(options::RECURSIVE); - if recursive { - if bit_flag == FTS_PHYSICAL { - if derefer == 1 { - return Err(USimpleError::new(1, "-R --dereference requires -H or -L")); - } - derefer = 0; - } - } else { - bit_flag = FTS_PHYSICAL; - } - - let verbosity_level = if matches.is_present(options::verbosity::CHANGES) { - VerbosityLevel::Changes - } else if matches.is_present(options::verbosity::SILENT) - || matches.is_present(options::verbosity::QUIET) - { - VerbosityLevel::Silent - } else if matches.is_present(options::verbosity::VERBOSE) { - VerbosityLevel::Verbose - } else { - VerbosityLevel::Normal - }; - +fn parse_gid_and_uid(matches: &ArgMatches) -> UResult<(Option, Option, IfFrom)> { let dest_gid = if let Some(file) = matches.value_of(options::REFERENCE) { fs::metadata(&file) .map(|meta| Some(meta.gid())) @@ -168,22 +44,20 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } } }; + Ok((dest_gid, None, IfFrom::All)) +} - let executor = ChownExecutor { - bit_flag, - dest_gid, - verbosity: Verbosity { - groups_only: true, - level: verbosity_level, - }, - recursive, - dereference: derefer != 0, - preserve_root, - files, - filter: IfFrom::All, - dest_uid: None, - }; - executor.exec() +#[uucore_procs::gen_uumain] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { + let usage = get_usage(); + + chown_base( + uu_app().usage(&usage[..]), + args, + options::ARG_GROUP, + parse_gid_and_uid, + true, + ) } pub fn uu_app() -> App<'static, 'static> { diff --git a/src/uu/chown/src/chown.rs b/src/uu/chown/src/chown.rs index 06f0c6a32..4abb9ac61 100644 --- a/src/uu/chown/src/chown.rs +++ b/src/uu/chown/src/chown.rs @@ -10,49 +10,17 @@ #[macro_use] extern crate uucore; pub use uucore::entries::{self, Group, Locate, Passwd}; -use uucore::perms::{ - ChownExecutor, IfFrom, Verbosity, VerbosityLevel, FTS_COMFOLLOW, FTS_LOGICAL, FTS_PHYSICAL, -}; +use uucore::perms::{chown_base, options, IfFrom}; use uucore::error::{FromIo, UResult, USimpleError}; -use clap::{crate_version, App, Arg}; +use clap::{crate_version, App, Arg, ArgMatches}; use std::fs; use std::os::unix::fs::MetadataExt; -use uucore::InvalidEncodingHandling; - static ABOUT: &str = "change file owner and group"; -pub mod options { - pub mod verbosity { - pub static CHANGES: &str = "changes"; - pub static QUIET: &str = "quiet"; - pub static SILENT: &str = "silent"; - pub static VERBOSE: &str = "verbose"; - } - pub mod preserve_root { - pub static PRESERVE: &str = "preserve-root"; - pub static NO_PRESERVE: &str = "no-preserve-root"; - } - pub mod dereference { - pub static DEREFERENCE: &str = "dereference"; - pub static NO_DEREFERENCE: &str = "no-dereference"; - } - pub static FROM: &str = "from"; - pub static RECURSIVE: &str = "recursive"; - pub mod traverse { - pub static TRAVERSE: &str = "H"; - pub static NO_TRAVERSE: &str = "P"; - pub static EVERY: &str = "L"; - } - pub static REFERENCE: &str = "reference"; -} - -static ARG_OWNER: &str = "owner"; -static ARG_FILES: &str = "files"; - fn get_usage() -> String { format!( "{0} [OPTION]... [OWNER][:[GROUP]] FILE...\n{0} [OPTION]... --reference=RFILE FILE...", @@ -60,65 +28,7 @@ fn get_usage() -> String { ) } -#[uucore_procs::gen_uumain] -pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let args = args - .collect_str(InvalidEncodingHandling::Ignore) - .accept_any(); - - let usage = get_usage(); - - let matches = uu_app().usage(&usage[..]).get_matches_from(args); - - /* First arg is the owner/group */ - let owner = matches.value_of(ARG_OWNER).unwrap(); - - /* Then the list of files */ - let files: Vec = matches - .values_of(ARG_FILES) - .map(|v| v.map(ToString::to_string).collect()) - .unwrap_or_default(); - - let preserve_root = matches.is_present(options::preserve_root::PRESERVE); - - let mut derefer = if matches.is_present(options::dereference::NO_DEREFERENCE) { - 1 - } else { - 0 - }; - - let mut bit_flag = if matches.is_present(options::traverse::TRAVERSE) { - FTS_COMFOLLOW | FTS_PHYSICAL - } else if matches.is_present(options::traverse::EVERY) { - FTS_LOGICAL - } else { - FTS_PHYSICAL - }; - - let recursive = matches.is_present(options::RECURSIVE); - if recursive { - if bit_flag == FTS_PHYSICAL { - if derefer == 1 { - return Err(USimpleError::new(1, "-R --dereference requires -H or -L")); - } - derefer = 0; - } - } else { - bit_flag = FTS_PHYSICAL; - } - - let verbosity = if matches.is_present(options::verbosity::CHANGES) { - VerbosityLevel::Changes - } else if matches.is_present(options::verbosity::SILENT) - || matches.is_present(options::verbosity::QUIET) - { - VerbosityLevel::Silent - } else if matches.is_present(options::verbosity::VERBOSE) { - VerbosityLevel::Verbose - } else { - VerbosityLevel::Normal - }; - +fn parse_gid_uid_and_filter(matches: &ArgMatches) -> UResult<(Option, Option, IfFrom)> { let filter = if let Some(spec) = matches.value_of(options::FROM) { match parse_spec(spec)? { (Some(uid), None) => IfFrom::User(uid), @@ -138,25 +48,24 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { dest_gid = Some(meta.gid()); dest_uid = Some(meta.uid()); } else { - let (u, g) = parse_spec(owner)?; + let (u, g) = parse_spec(matches.value_of(options::ARG_OWNER).unwrap())?; dest_uid = u; dest_gid = g; } - let executor = ChownExecutor { - bit_flag, - dest_uid, - dest_gid, - verbosity: Verbosity { - groups_only: false, - level: verbosity, - }, - recursive, - dereference: derefer != 0, - filter, - preserve_root, - files, - }; - executor.exec() + Ok((dest_gid, dest_uid, filter)) +} + +#[uucore_procs::gen_uumain] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { + let usage = get_usage(); + + chown_base( + uu_app().usage(&usage[..]), + args, + options::ARG_OWNER, + parse_gid_uid_and_filter, + false, + ) } pub fn uu_app() -> App<'static, 'static> { @@ -169,22 +78,31 @@ pub fn uu_app() -> App<'static, 'static> { .long(options::verbosity::CHANGES) .help("like verbose but report only when a change is made"), ) - .arg(Arg::with_name(options::dereference::DEREFERENCE).long(options::dereference::DEREFERENCE).help( - "affect the referent of each symbolic link (this is the default), rather than the symbolic link itself", - )) + .arg( + Arg::with_name(options::dereference::DEREFERENCE) + .long(options::dereference::DEREFERENCE) + .help( + "affect the referent of each symbolic link (this is the default), \ + rather than the symbolic link itself", + ), + ) .arg( Arg::with_name(options::dereference::NO_DEREFERENCE) .short("h") .long(options::dereference::NO_DEREFERENCE) .help( - "affect symbolic links instead of any referenced file (useful only on systems that can change the ownership of a symlink)", + "affect symbolic links instead of any referenced file \ + (useful only on systems that can change the ownership of a symlink)", ), ) .arg( Arg::with_name(options::FROM) .long(options::FROM) .help( - "change the owner and/or group of each file only if its current owner and/or group match those specified here. Either may be omitted, in which case a match is not required for the omitted attribute", + "change the owner and/or group of each file only if its \ + current owner and/or group match those specified here. \ + Either may be omitted, in which case a match is not required \ + for the omitted attribute", ) .value_name("CURRENT_OWNER:CURRENT_GROUP"), ) @@ -216,7 +134,11 @@ pub fn uu_app() -> App<'static, 'static> { .value_name("RFILE") .min_values(1), ) - .arg(Arg::with_name(options::verbosity::SILENT).short("f").long(options::verbosity::SILENT)) + .arg( + Arg::with_name(options::verbosity::SILENT) + .short("f") + .long(options::verbosity::SILENT), + ) .arg( Arg::with_name(options::traverse::TRAVERSE) .short(options::traverse::TRAVERSE) @@ -238,21 +160,9 @@ pub fn uu_app() -> App<'static, 'static> { .arg( Arg::with_name(options::verbosity::VERBOSE) .long(options::verbosity::VERBOSE) + .short("v") .help("output a diagnostic for every file processed"), ) - .arg( - Arg::with_name(ARG_OWNER) - .multiple(false) - .takes_value(true) - .required(true), - ) - .arg( - Arg::with_name(ARG_FILES) - .multiple(true) - .takes_value(true) - .required(true) - .min_values(1), - ) } fn parse_spec(spec: &str) -> UResult<(Option, Option)> { diff --git a/src/uucore/src/lib/features/perms.rs b/src/uucore/src/lib/features/perms.rs index fe1c97a82..b071cedaa 100644 --- a/src/uucore/src/lib/features/perms.rs +++ b/src/uucore/src/lib/features/perms.rs @@ -7,10 +7,14 @@ use crate::error::strip_errno; use crate::error::UResult; +use crate::error::USimpleError; pub use crate::features::entries; use crate::fs::resolve_relative_path; use crate::show_error; -use libc::{self, gid_t, lchown, uid_t}; +use clap::App; +use clap::Arg; +use clap::ArgMatches; +use libc::{self, gid_t, uid_t}; use walkdir::WalkDir; use std::io::Error as IOError; @@ -45,7 +49,7 @@ fn chown>(path: P, uid: uid_t, gid: gid_t, follow: bool) -> IORes if follow { libc::chown(s.as_ptr(), uid, gid) } else { - lchown(s.as_ptr(), uid, gid) + libc::lchown(s.as_ptr(), uid, gid) } }; if ret == 0 { @@ -161,22 +165,26 @@ pub enum IfFrom { UserGroup(u32, u32), } +#[derive(PartialEq, Eq)] +pub enum TraverseSymlinks { + None, + First, + All, +} + pub struct ChownExecutor { pub dest_uid: Option, pub dest_gid: Option, - pub bit_flag: u8, + pub traverse_symlinks: TraverseSymlinks, pub verbosity: Verbosity, pub filter: IfFrom, pub files: Vec, pub recursive: bool, pub preserve_root: bool, + // Must be true if traverse_symlinks is not None pub dereference: bool, } -pub const FTS_COMFOLLOW: u8 = 1; -pub const FTS_PHYSICAL: u8 = 1 << 1; -pub const FTS_LOGICAL: u8 = 1 << 2; - impl ChownExecutor { pub fn exec(&self) -> UResult<()> { let mut ret = 0; @@ -190,9 +198,8 @@ impl ChownExecutor { } fn traverse>(&self, root: P) -> i32 { - let follow_arg = self.dereference || self.bit_flag != FTS_PHYSICAL; let path = root.as_ref(); - let meta = match self.obtain_meta(path, follow_arg) { + let meta = match self.obtain_meta(path, self.dereference) { Some(m) => m, _ => return 1, }; @@ -204,7 +211,7 @@ impl ChownExecutor { // (argument is symlink && should follow argument && resolved to be '/') // ) if self.recursive && self.preserve_root { - let may_exist = if follow_arg { + let may_exist = if self.dereference { path.canonicalize().ok() } else { let real = resolve_relative_path(path); @@ -230,7 +237,7 @@ impl ChownExecutor { &meta, self.dest_uid, self.dest_gid, - follow_arg, + self.dereference, self.verbosity.clone(), ) { Ok(n) => { @@ -258,11 +265,21 @@ impl ChownExecutor { } fn dive_into>(&self, root: P) -> i32 { - let mut ret = 0; let root = root.as_ref(); - let follow = self.dereference || self.bit_flag & FTS_LOGICAL != 0; + + // walkdir always dereferences the root directory, so we have to check it ourselves + // TODO: replace with `root.is_symlink()` once it is stable + if self.traverse_symlinks == TraverseSymlinks::None + && std::fs::symlink_metadata(root) + .map(|m| m.file_type().is_symlink()) + .unwrap_or(false) + { + return 0; + } + + let mut ret = 0; let mut iterator = WalkDir::new(root) - .follow_links(follow) + .follow_links(self.traverse_symlinks == TraverseSymlinks::All) .min_depth(1) .into_iter(); // We can't use a for loop because we need to manipulate the iterator inside the loop. @@ -288,7 +305,7 @@ impl ChownExecutor { Ok(entry) => entry, }; let path = entry.path(); - let meta = match self.obtain_meta(path, follow) { + let meta = match self.obtain_meta(path, self.dereference) { Some(m) => m, _ => { ret = 1; @@ -310,7 +327,7 @@ impl ChownExecutor { &meta, self.dest_uid, self.dest_gid, - follow, + self.dereference, self.verbosity.clone(), ) { Ok(n) => { @@ -341,7 +358,12 @@ impl ChownExecutor { Err(e) => { match self.verbosity.level { VerbosityLevel::Silent => (), - _ => show_error!("cannot access '{}': {}", path.display(), strip_errno(&e)), + _ => show_error!( + "cannot {} '{}': {}", + if follow { "dereference" } else { "access" }, + path.display(), + strip_errno(&e) + ), } None } @@ -359,3 +381,148 @@ impl ChownExecutor { } } } + +pub mod options { + pub mod verbosity { + pub const CHANGES: &str = "changes"; + pub const QUIET: &str = "quiet"; + pub const SILENT: &str = "silent"; + pub const VERBOSE: &str = "verbose"; + } + pub mod preserve_root { + pub const PRESERVE: &str = "preserve-root"; + pub const NO_PRESERVE: &str = "no-preserve-root"; + } + pub mod dereference { + pub const DEREFERENCE: &str = "dereference"; + pub const NO_DEREFERENCE: &str = "no-dereference"; + } + pub const FROM: &str = "from"; + pub const RECURSIVE: &str = "recursive"; + pub mod traverse { + pub const TRAVERSE: &str = "H"; + pub const NO_TRAVERSE: &str = "P"; + pub const EVERY: &str = "L"; + } + pub const REFERENCE: &str = "reference"; + pub const ARG_OWNER: &str = "OWNER"; + pub const ARG_GROUP: &str = "GROUP"; + pub const ARG_FILES: &str = "FILE"; +} + +type GidUidFilterParser<'a> = fn(&ArgMatches<'a>) -> UResult<(Option, Option, IfFrom)>; + +/// Base implementation for `chgrp` and `chown`. +/// +/// An argument called `add_arg_if_not_reference` will be added to `app` if +/// `args` does not contain the `--reference` option. +/// `parse_gid_uid_and_filter` will be called to obtain the target gid and uid, and the filter, +/// from `ArgMatches`. +/// `groups_only` determines whether verbose output will only mention the group. +pub fn chown_base<'a>( + mut app: App<'a, 'a>, + args: impl crate::Args, + add_arg_if_not_reference: &'a str, + parse_gid_uid_and_filter: GidUidFilterParser<'a>, + groups_only: bool, +) -> UResult<()> { + let args: Vec<_> = args.collect(); + let mut reference = false; + let mut help = false; + // stop processing options on -- + for arg in args.iter().take_while(|s| *s != "--") { + if arg.to_string_lossy().starts_with("--reference=") || arg == "--reference" { + reference = true; + } else if arg == "--help" { + // we stop processing once we see --help, + // as it doesn't matter if we've seen reference or not + help = true; + break; + } + } + + if help || !reference { + // add both positional arguments + // arg_group is only required if + app = app.arg( + Arg::with_name(add_arg_if_not_reference) + .value_name(add_arg_if_not_reference) + .required(true) + .takes_value(true) + .multiple(false), + ) + } + app = app.arg( + Arg::with_name(options::ARG_FILES) + .value_name(options::ARG_FILES) + .multiple(true) + .takes_value(true) + .required(true) + .min_values(1), + ); + let matches = app.get_matches_from(args); + + let files: Vec = matches + .values_of(options::ARG_FILES) + .map(|v| v.map(ToString::to_string).collect()) + .unwrap_or_default(); + + let preserve_root = matches.is_present(options::preserve_root::PRESERVE); + + let mut dereference = if matches.is_present(options::dereference::DEREFERENCE) { + Some(true) + } else if matches.is_present(options::dereference::NO_DEREFERENCE) { + Some(false) + } else { + None + }; + + let mut traverse_symlinks = if matches.is_present(options::traverse::TRAVERSE) { + TraverseSymlinks::First + } else if matches.is_present(options::traverse::EVERY) { + TraverseSymlinks::All + } else { + TraverseSymlinks::None + }; + + let recursive = matches.is_present(options::RECURSIVE); + if recursive { + if traverse_symlinks == TraverseSymlinks::None { + if dereference == Some(true) { + return Err(USimpleError::new(1, "-R --dereference requires -H or -L")); + } + dereference = Some(false); + } + } else { + traverse_symlinks = TraverseSymlinks::None; + } + + let verbosity_level = if matches.is_present(options::verbosity::CHANGES) { + VerbosityLevel::Changes + } else if matches.is_present(options::verbosity::SILENT) + || matches.is_present(options::verbosity::QUIET) + { + VerbosityLevel::Silent + } else if matches.is_present(options::verbosity::VERBOSE) { + VerbosityLevel::Verbose + } else { + VerbosityLevel::Normal + }; + let (dest_gid, dest_uid, filter) = parse_gid_uid_and_filter(&matches)?; + + let executor = ChownExecutor { + traverse_symlinks, + dest_gid, + dest_uid, + verbosity: Verbosity { + groups_only, + level: verbosity_level, + }, + recursive, + dereference: dereference.unwrap_or(true), + preserve_root, + files, + filter, + }; + executor.exec() +} diff --git a/tests/by-util/test_chgrp.rs b/tests/by-util/test_chgrp.rs index 0fc73520e..1d047cfe2 100644 --- a/tests/by-util/test_chgrp.rs +++ b/tests/by-util/test_chgrp.rs @@ -288,3 +288,68 @@ fn test_subdir_permission_denied() { .stderr_only("chgrp: cannot access 'dir/subdir': Permission denied"); } } + +#[test] +#[cfg(not(target_vendor = "apple"))] +fn test_traverse_symlinks() { + use std::os::unix::prelude::MetadataExt; + let groups = nix::unistd::getgroups().unwrap(); + if groups.len() < 2 { + return; + } + let (first_group, second_group) = (groups[0], groups[1]); + + for &(args, traverse_first, traverse_second) in &[ + (&[][..] as &[&str], false, false), + (&["-H"][..], true, false), + (&["-P"][..], false, false), + (&["-L"][..], true, true), + ] { + let scenario = TestScenario::new("chgrp"); + + let (at, mut ucmd) = (scenario.fixtures.clone(), scenario.ucmd()); + + at.mkdir("dir"); + at.mkdir("dir2"); + at.touch("dir2/file"); + at.mkdir("dir3"); + at.touch("dir3/file"); + at.symlink_dir("dir2", "dir/dir2_ln"); + at.symlink_dir("dir3", "dir3_ln"); + + scenario + .ccmd("chgrp") + .arg(first_group.to_string()) + .arg("dir2/file") + .arg("dir3/file") + .succeeds(); + + assert!(at.plus("dir2/file").metadata().unwrap().gid() == first_group.as_raw()); + assert!(at.plus("dir3/file").metadata().unwrap().gid() == first_group.as_raw()); + + ucmd.arg("-R") + .args(args) + .arg(second_group.to_string()) + .arg("dir") + .arg("dir3_ln") + .succeeds() + .no_stderr(); + + assert_eq!( + at.plus("dir2/file").metadata().unwrap().gid(), + if traverse_second { + second_group.as_raw() + } else { + first_group.as_raw() + } + ); + assert_eq!( + at.plus("dir3/file").metadata().unwrap().gid(), + if traverse_first { + second_group.as_raw() + } else { + first_group.as_raw() + } + ); + } +}