From fe3645d4d5c34ef830ef04edc5f5fe50edcdc472 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Fri, 10 Sep 2021 21:33:34 +0200 Subject: [PATCH 01/17] ls: add support for showing SELinux context (--context/-Z) --- Cargo.lock | 1 + Cargo.toml | 2 +- src/uu/ls/Cargo.toml | 4 ++ src/uu/ls/src/ls.rs | 164 +++++++++++++++++++++++++++++++++++++------ 4 files changed, 149 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 704d1eea1..ea3467b5a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2632,6 +2632,7 @@ dependencies = [ "lscolors", "number_prefix", "once_cell", + "selinux", "term_grid", "termsize", "unicode-width", diff --git a/Cargo.toml b/Cargo.toml index 3a2c5f12a..8d2f2da58 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -147,7 +147,7 @@ feat_os_unix_musl = [ # NOTE: # The selinux(-sys) crate requires `libselinux` headers and shared library to be accessible in the C toolchain at compile time. # Running a uutils compiled with `feat_selinux` requires an SELinux enabled Kernel at run time. -feat_selinux = ["cp/selinux", "id/selinux", "selinux", "feat_require_selinux"] +feat_selinux = ["cp/selinux", "id/selinux", "ls/selinux", "selinux", "feat_require_selinux"] # "feat_acl" == set of utilities providing support for acl (access control lists) if enabled with `--features feat_acl`. # NOTE: # On linux, the posix-acl/acl-sys crate requires `libacl` headers and shared library to be accessible in the C toolchain at compile time. diff --git a/src/uu/ls/Cargo.toml b/src/uu/ls/Cargo.toml index dbe6bacaa..62dcd59a4 100644 --- a/src/uu/ls/Cargo.toml +++ b/src/uu/ls/Cargo.toml @@ -28,6 +28,7 @@ uucore = { version = ">=0.0.8", package = "uucore", path = "../../uucore", featu uucore_procs = { version=">=0.0.6", package = "uucore_procs", path = "../../uucore_procs" } once_cell = "1.7.2" atty = "0.2" +selinux = { version="0.2.1", optional = true } [target.'cfg(unix)'.dependencies] lazy_static = "1.4.0" @@ -35,3 +36,6 @@ lazy_static = "1.4.0" [[bin]] name = "ls" path = "src/main.rs" + +[features] +feat_selinux = ["selinux"] diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 6f63c2a4a..7b7fd873e 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -47,6 +47,11 @@ use unicode_width::UnicodeWidthStr; use uucore::libc::{S_IXGRP, S_IXOTH, S_IXUSR}; use uucore::{fs::display_permissions, version_cmp::version_cmp}; +#[cfg(not(feature = "selinux"))] +static CONTEXT_HELP_TEXT: &str = "print any security context of each file (not enabled)"; +#[cfg(feature = "selinux")] +static CONTEXT_HELP_TEXT: &str = "print any security context of each file"; + fn usage() -> String { format!("{0} [OPTION]... [FILE]...", uucore::execution_phrase()) } @@ -126,6 +131,7 @@ pub mod options { pub static FULL_TIME: &str = "full-time"; pub static HIDE: &str = "hide"; pub static IGNORE: &str = "ignore"; + pub static CONTEXT: &str = "context"; } const DEFAULT_TERM_WIDTH: u16 = 80; @@ -236,6 +242,8 @@ struct Config { quoting_style: QuotingStyle, indicator_style: IndicatorStyle, time_style: TimeStyle, + context: bool, + selinux_supported: bool, } // Fields that can be removed or added to the long format @@ -247,9 +255,18 @@ struct LongFormat { numeric_uid_gid: bool, } +struct PaddingCollection { + longest_link_count_len: usize, + longest_uname_len: usize, + longest_group_len: usize, + longest_context_len: usize, + longest_size_len: usize, +} + impl Config { #[allow(clippy::cognitive_complexity)] fn from(options: &clap::ArgMatches) -> UResult { + let context = options.is_present(options::CONTEXT); let (mut format, opt) = if let Some(format_) = options.value_of(options::FORMAT) { ( match format_ { @@ -593,6 +610,17 @@ impl Config { quoting_style, indicator_style, time_style, + context, + selinux_supported: { + #[cfg(feature = "selinux")] + { + selinux::kernel_support() != selinux::KernelSupport::Unsupported + } + #[cfg(not(feature = "selinux"))] + { + false + } + }, }) } } @@ -611,6 +639,10 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .map(|v| v.map(Path::new).collect()) .unwrap_or_else(|| vec![Path::new(".")]); + if config.context && !config.selinux_supported { + show_warning!("--context (-Z) works only on an SELinux-enabled kernel",); + } + list(locs, config) } @@ -1154,6 +1186,12 @@ only ignore '.' and '..'.", .overrides_with(options::FULL_TIME) .help("like -l --time-style=full-iso"), ) + .arg( + Arg::with_name(options::CONTEXT) + .short("Z") + .long(options::CONTEXT) + .help(CONTEXT_HELP_TEXT), + ) // Positional arguments .arg( Arg::with_name(options::PATHS) @@ -1178,6 +1216,7 @@ struct PathData { // PathBuf that all above data corresponds to p_buf: PathBuf, must_dereference: bool, + security_context: String, } impl PathData { @@ -1221,12 +1260,39 @@ impl PathData { None => OnceCell::new(), }; + let security_context: String = if config.context { + if config.selinux_supported { + #[cfg(feature = "selinux")] + { + if let Ok(Some(context)) = + selinux::SecurityContext::of_path(&p_buf, must_dereference, false) + { + String::from_utf8_lossy(context.as_bytes()) + .trim_end_matches(char::from(0)) + .to_string() + } else { + // TODO: print warning with error to stderr + "?".to_string() + } + } + #[cfg(not(feature = "selinux"))] + { + "?".to_string() + } + } else { + "?".to_string() + } + } else { + String::new() + }; + Self { md: OnceCell::new(), ft, display_name, p_buf, must_dereference, + security_context, } } @@ -1390,7 +1456,7 @@ fn get_metadata(entry: &Path, dereference: bool) -> std::io::Result { } fn display_dir_entry_size(entry: &PathData, config: &Config) -> (usize, usize, usize, usize) { - // TODO: Cache/memoize the display_* results so we don't have to recalculate them. + // TODO: Cache/memorize the display_* results so we don't have to recalculate them. if let Some(md) = entry.md() { ( display_symlink_count(md).len(), @@ -1403,31 +1469,40 @@ fn display_dir_entry_size(entry: &PathData, config: &Config) -> (usize, usize, u } } -fn pad_left(string: String, count: usize) -> String { +fn pad_left(string: &str, count: usize) -> String { format!("{:>width$}", string, width = count) } -fn pad_right(string: String, count: usize) -> String { +fn pad_right(string: &str, count: usize) -> String { format!("{:) { + // `-Z`, `--context`: + // Display the SELinux security context or '?' if none is found. When used with the `-l` + // option, print the security context to the left of the size column. + if config.format == Format::Long { let ( mut longest_link_count_len, mut longest_uname_len, mut longest_group_len, + mut longest_context_len, mut longest_size_len, - ) = (1, 1, 1, 1); + ) = (1, 1, 1, 1, 1); let mut total_size = 0; for item in items { + let context_len = item.security_context.len(); let (link_count_len, uname_len, group_len, size_len) = display_dir_entry_size(item, config); longest_link_count_len = link_count_len.max(longest_link_count_len); longest_size_len = size_len.max(longest_size_len); longest_uname_len = uname_len.max(longest_uname_len); longest_group_len = group_len.max(longest_group_len); + if config.context { + longest_context_len = context_len.max(longest_context_len); + } longest_size_len = size_len.max(longest_size_len); total_size += item.md().map_or(0, |md| get_block_size(md, config)); } @@ -1439,16 +1514,31 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter display_grid(names, config.width, Direction::TopToBottom, out), @@ -1573,15 +1663,13 @@ fn display_grid( /// longest_link_count_len: usize, /// longest_uname_len: usize, /// longest_group_len: usize, +/// longest_context_len: usize, /// longest_size_len: usize, /// ``` /// that decide the maximum possible character count of each field. fn display_item_long( item: &PathData, - longest_link_count_len: usize, - longest_uname_len: usize, - longest_group_len: usize, - longest_size_len: usize, + padding: PaddingCollection, config: &Config, out: &mut BufWriter, ) { @@ -1602,16 +1690,23 @@ fn display_item_long( let _ = write!( out, - "{} {}", + "{}{} {}", display_permissions(md, true), - pad_left(display_symlink_count(md), longest_link_count_len), + if item.security_context != "?" { + // GNU `ls` uses a "." character to indicate a file with a security context, but not + // other alternate access method. + "." + } else { + "" + }, + pad_left(&display_symlink_count(md), padding.longest_link_count_len), ); if config.long.owner { let _ = write!( out, " {}", - pad_right(display_uname(md, config), longest_uname_len) + pad_right(&display_uname(md, config), padding.longest_uname_len) ); } @@ -1619,7 +1714,15 @@ fn display_item_long( let _ = write!( out, " {}", - pad_right(display_group(md, config), longest_group_len) + pad_right(&display_group(md, config), padding.longest_group_len) + ); + } + + if config.context { + let _ = write!( + out, + " {}", + pad_right(&item.security_context, padding.longest_context_len) ); } @@ -1629,19 +1732,19 @@ fn display_item_long( let _ = write!( out, " {}", - pad_right(display_uname(md, config), longest_uname_len) + pad_right(&display_uname(md, config), padding.longest_uname_len) ); } let _ = writeln!( out, " {} {} {}", - pad_left(display_size_or_rdev(md, config), longest_size_len), + pad_left(&display_size_or_rdev(md, config), padding.longest_size_len), display_date(md, config), // unwrap is fine because it fails when metadata is not available // but we already know that it is because it's checked at the // start of the function. - display_file_name(item, config).unwrap().contents, + display_file_name(item, config, None).unwrap().contents, ); } @@ -1865,10 +1968,15 @@ fn classify_file(path: &PathData) -> Option { /// * `config.indicator_style` to append specific characters to `name` using [`classify_file`]. /// * `config.format` to display symlink targets if `Format::Long`. This function is also /// responsible for coloring symlink target names if `config.color` is specified. +/// * `config.context` to prepend security context to `name` if compiled with `feat_selinux`. /// /// Note that non-unicode sequences in symlink targets are dealt with using /// [`std::path::Path::to_string_lossy`]. -fn display_file_name(path: &PathData, config: &Config) -> Option { +fn display_file_name( + path: &PathData, + config: &Config, + prefix_context: Option, +) -> Option { // This is our return value. We start by `&path.display_name` and modify it along the way. let mut name = escape_name(&path.display_name, &config.quoting_style); @@ -1960,6 +2068,20 @@ fn display_file_name(path: &PathData, config: &Config) -> Option { } } + // Prepend the security context to the `name` and adjust `width` in order + // to get correct alignment from later calls to`display_grid()`. + if config.context { + if let Some(pad_count) = prefix_context { + let security_context = if !matches!(config.format, Format::Commas) { + pad_left(&path.security_context, pad_count) + } else { + path.security_context.to_owned() + }; + name = format!("{} {}", security_context, name); + width += security_context.len() + 1; + } + } + Some(Cell { contents: name, width, From e018a45b54b96e1e3f8dd323b411e0bc6ae84673 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Fri, 10 Sep 2021 21:35:00 +0200 Subject: [PATCH 02/17] test_ls: add/adjust tests for SELinux context --- src/uu/ls/src/ls.rs | 12 ++-- tests/by-util/test_ls.rs | 127 +++++++++++++++++++++++++++------------ 2 files changed, 94 insertions(+), 45 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 7b7fd873e..5c26c8e26 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -639,10 +639,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .map(|v| v.map(Path::new).collect()) .unwrap_or_else(|| vec![Path::new(".")]); - if config.context && !config.selinux_supported { - show_warning!("--context (-Z) works only on an SELinux-enabled kernel",); - } - list(locs, config) } @@ -1260,7 +1256,7 @@ impl PathData { None => OnceCell::new(), }; - let security_context: String = if config.context { + let security_context = if config.context { if config.selinux_supported { #[cfg(feature = "selinux")] { @@ -1692,9 +1688,9 @@ fn display_item_long( out, "{}{} {}", display_permissions(md, true), - if item.security_context != "?" { - // GNU `ls` uses a "." character to indicate a file with a security context, but not - // other alternate access method. + if item.security_context.len() > 1 { + // GNU `ls` uses a "." character to indicate a file with a security context, + // but not other alternate access method. "." } else { "" diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 3d6092416..f14fc424a 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -347,6 +347,7 @@ fn test_ls_long_format() { // A line of the output should be: // One of the characters -bcCdDlMnpPsStTx? // rwx, with - for missing permissions, thrice. + // Zero or one "." for indicating a file with security context // A number, preceded by column whitespace, and followed by a single space. // A username, currently [^ ], followed by column whitespace, twice (or thrice for Hurd). // A number, followed by a single space. @@ -356,13 +357,13 @@ fn test_ls_long_format() { // and followed by a single space. // Whatever comes after is irrelevant to this specific test. scene.ucmd().arg(arg).arg("test-long-dir").succeeds().stdout_matches(&Regex::new( - r"\n[-bcCdDlMnpPsStTx?]([r-][w-][xt-]){3} +\d+ [^ ]+ +[^ ]+( +[^ ]+)? +\d+ [A-Z][a-z]{2} {0,2}\d{0,2} {0,2}[0-9:]+ " + r"\n[-bcCdDlMnpPsStTx?]([r-][w-][xt-]){3}\.? +\d+ [^ ]+ +[^ ]+( +[^ ]+)? +\d+ [A-Z][a-z]{2} {0,2}\d{0,2} {0,2}[0-9:]+ " ).unwrap()); } // This checks for the line with the .. entry. The uname and group should be digits. scene.ucmd().arg("-lan").arg("test-long-dir").succeeds().stdout_matches(&Regex::new( - r"\nd([r-][w-][xt-]){3} +\d+ \d+ +\d+( +\d+)? +\d+ [A-Z][a-z]{2} {0,2}\d{0,2} {0,2}[0-9:]+ \.\." + r"\nd([r-][w-][xt-]){3}\.? +\d+ \d+ +\d+( +\d+)? +\d+ [A-Z][a-z]{2} {0,2}\d{0,2} {0,2}[0-9:]+ \.\." ).unwrap()); } @@ -370,6 +371,7 @@ fn test_ls_long_format() { /// This test is mainly about coloring, but, the recursion, symlink `->` processing, /// and `.` and `..` being present in `-a` all need to work for the test to pass. /// This test does not really test anything provided by `-l` but the file names and symlinks. +#[cfg(all(feature = "ln", feature = "mkdir", feature = "touch"))] #[test] fn test_ls_long_symlink_color() { // If you break this test after breaking mkdir, touch, or ln, do not be alarmed! @@ -639,55 +641,57 @@ fn test_ls_long_formats() { let at = &scene.fixtures; at.touch(&at.plus_as_string("test-long-formats")); + // Zero or one "." for indicating a file with security context + // Regex for three names, so all of author, group and owner - let re_three = Regex::new(r"[xrw-]{9} \d ([-0-9_a-z]+ ){3}0").unwrap(); + let re_three = Regex::new(r"[xrw-]{9}\.? \d ([-0-9_a-z]+ ){3}0").unwrap(); #[cfg(unix)] - let re_three_num = Regex::new(r"[xrw-]{9} \d (\d+ ){3}0").unwrap(); + let re_three_num = Regex::new(r"[xrw-]{9}\.? \d (\d+ ){3}0").unwrap(); // Regex for two names, either: // - group and owner // - author and owner // - author and group - let re_two = Regex::new(r"[xrw-]{9} \d ([-0-9_a-z]+ ){2}0").unwrap(); + let re_two = Regex::new(r"[xrw-]{9}\.? \d ([-0-9_a-z]+ ){2}0").unwrap(); #[cfg(unix)] - let re_two_num = Regex::new(r"[xrw-]{9} \d (\d+ ){2}0").unwrap(); + let re_two_num = Regex::new(r"[xrw-]{9}\.? \d (\d+ ){2}0").unwrap(); // Regex for one name: author, group or owner - let re_one = Regex::new(r"[xrw-]{9} \d [-0-9_a-z]+ 0").unwrap(); + let re_one = Regex::new(r"[xrw-]{9}\.? \d [-0-9_a-z]+ 0").unwrap(); #[cfg(unix)] - let re_one_num = Regex::new(r"[xrw-]{9} \d \d+ 0").unwrap(); + let re_one_num = Regex::new(r"[xrw-]{9}\.? \d \d+ 0").unwrap(); // Regex for no names - let re_zero = Regex::new(r"[xrw-]{9} \d 0").unwrap(); + let re_zero = Regex::new(r"[xrw-]{9}\.? \d 0").unwrap(); - let result = scene + scene .ucmd() .arg("-l") .arg("--author") .arg("test-long-formats") - .succeeds(); - assert!(re_three.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_three); - let result = scene + scene .ucmd() .arg("-l1") .arg("--author") .arg("test-long-formats") - .succeeds(); - assert!(re_three.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_three); #[cfg(unix)] { - let result = scene + scene .ucmd() .arg("-n") .arg("--author") .arg("test-long-formats") - .succeeds(); - assert!(re_three_num.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_three_num); } for arg in &[ @@ -697,22 +701,22 @@ fn test_ls_long_formats() { "-lG --author", // only author and owner "-l --no-group --author", // only author and owner ] { - let result = scene + scene .ucmd() .args(&arg.split(' ').collect::>()) .arg("test-long-formats") - .succeeds(); - assert!(re_two.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_two); #[cfg(unix)] { - let result = scene + scene .ucmd() .arg("-n") .args(&arg.split(' ').collect::>()) .arg("test-long-formats") - .succeeds(); - assert!(re_two_num.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_two_num); } } @@ -726,22 +730,22 @@ fn test_ls_long_formats() { "-l --no-group", // only owner "-gG --author", // only author ] { - let result = scene + scene .ucmd() .args(&arg.split(' ').collect::>()) .arg("test-long-formats") - .succeeds(); - assert!(re_one.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_one); #[cfg(unix)] { - let result = scene + scene .ucmd() .arg("-n") .args(&arg.split(' ').collect::>()) .arg("test-long-formats") - .succeeds(); - assert!(re_one_num.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_one_num); } } @@ -758,22 +762,22 @@ fn test_ls_long_formats() { "-og1", "-og1l", ] { - let result = scene + scene .ucmd() .args(&arg.split(' ').collect::>()) .arg("test-long-formats") - .succeeds(); - assert!(re_zero.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_zero); #[cfg(unix)] { - let result = scene + scene .ucmd() .arg("-n") .args(&arg.split(' ').collect::>()) .arg("test-long-formats") - .succeeds(); - assert!(re_zero.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_zero); } } } @@ -1251,7 +1255,7 @@ fn test_ls_inode() { at.touch(file); let re_short = Regex::new(r" *(\d+) test_inode").unwrap(); - let re_long = Regex::new(r" *(\d+) [xrw-]{10} \d .+ test_inode").unwrap(); + let re_long = Regex::new(r" *(\d+) [xrw-]{10}\.? \d .+ test_inode").unwrap(); let result = scene.ucmd().arg("test_inode").arg("-i").succeeds(); assert!(re_short.is_match(result.stdout_str())); @@ -2275,3 +2279,52 @@ fn test_ls_dangling_symlinks() { .succeeds() // this should fail, though at the moment, ls lacks a way to propagate errors encountered during display .stdout_contains(if cfg!(windows) { "dangle" } else { "? dangle" }); } + +#[test] +#[cfg(feature = "feat_selinux")] +fn test_ls_context() { + use selinux::{self, KernelSupport}; + if selinux::kernel_support() == KernelSupport::Unsupported { + println!("test skipped: Kernel has no support for SElinux context",); + return; + } + let ts = TestScenario::new(util_name!()); + for c_flag in &["-Z", "--context"] { + ts.ucmd() + .args(&[c_flag, &"/"]) + .succeeds() + .stdout_only(unwrap_or_return!(expected_result(&ts, &[c_flag, &"/"])).stdout_str()); + } +} + +#[test] +#[cfg(feature = "feat_selinux")] +fn test_ls_context_format() { + use selinux::{self, KernelSupport}; + if selinux::kernel_support() == KernelSupport::Unsupported { + println!("test skipped: Kernel has no support for SElinux context",); + return; + } + let ts = TestScenario::new(util_name!()); + // NOTE: + // --format=long/verbose matches the output of GNU's ls for --context + // except for the size count which may differ to the size count reported by GNU's ls. + for word in &[ + "across", + "commas", + "horizontal", + // "long", + "single-column", + // "verbose", + "vertical", + ] { + let format = format!("--format={}", word); + ts.ucmd() + .args(&[&"-Z", &format.as_str(), &"/"]) + .succeeds() + .stdout_only( + unwrap_or_return!(expected_result(&ts, &[&"-Z", &format.as_str(), &"/"])) + .stdout_str(), + ); + } +} From cd2153eac624232ff240b323eb4c04c110308e53 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Mon, 13 Sep 2021 13:24:44 +0200 Subject: [PATCH 03/17] ls: (--context/-Z) add support for invalid utf8 If security context byte slice contains invalid utf8: * issue warning * show replacement characters for invalid utf8 sequences --- src/uu/ls/src/ls.rs | 37 ++++++++++++++++++++++++++----------- tests/by-util/test_ls.rs | 18 +++++++++++++++++- 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 5c26c8e26..be1964e43 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -40,6 +40,7 @@ use std::{ time::Duration, }; use term_grid::{Cell, Direction, Filling, Grid, GridOptions}; +use uucore::display::Quotable; use uucore::error::{set_exit_code, FromIo, UError, UResult}; use unicode_width::UnicodeWidthStr; @@ -1256,27 +1257,41 @@ impl PathData { None => OnceCell::new(), }; + let substitute_string = "?".to_string(); let security_context = if config.context { if config.selinux_supported { #[cfg(feature = "selinux")] { - if let Ok(Some(context)) = - selinux::SecurityContext::of_path(&p_buf, must_dereference, false) - { - String::from_utf8_lossy(context.as_bytes()) - .trim_end_matches(char::from(0)) - .to_string() - } else { - // TODO: print warning with error to stderr - "?".to_string() + match selinux::SecurityContext::of_path(&p_buf, must_dereference, false) { + Err(_r) => { + // TODO: show the actual reason why it failed + show_warning!("failed to get security context of: {}", p_buf.quote()); + substitute_string + } + Ok(None) => substitute_string, + Ok(Some(context)) => { + let mut context = context.as_bytes(); + if context.ends_with(&[0]) { + // TODO: replace with `strip_prefix()` when MSRV >= 1.51 + context = &context[..context.len() - 1] + }; + String::from_utf8(context.to_vec()).unwrap_or_else(|e| { + show_warning!( + "getting security context of: {}: {}", + p_buf.quote(), + e.to_string() + ); + String::from_utf8_lossy(context).into_owned() + }) + } } } #[cfg(not(feature = "selinux"))] { - "?".to_string() + substitute_string } } else { - "?".to_string() + substitute_string } } else { String::new() diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index f14fc424a..58119aac3 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -2282,7 +2282,23 @@ fn test_ls_dangling_symlinks() { #[test] #[cfg(feature = "feat_selinux")] -fn test_ls_context() { +fn test_ls_context1() { + use selinux::{self, KernelSupport}; + if selinux::kernel_support() == KernelSupport::Unsupported { + println!("test skipped: Kernel has no support for SElinux context",); + return; + } + + let file = "test_ls_context_file"; + let expected = format!("unconfined_u:object_r:user_tmp_t:s0 {}\n", file); + let (at, mut ucmd) = at_and_ucmd!(); + at.touch(file); + ucmd.args(&["-Z", file]).succeeds().stdout_is(expected); +} + +#[test] +#[cfg(feature = "feat_selinux")] +fn test_ls_context2() { use selinux::{self, KernelSupport}; if selinux::kernel_support() == KernelSupport::Unsupported { println!("test skipped: Kernel has no support for SElinux context",); From 8f229aad87187b4ffe11cfbdbb747029fff6c210 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Wed, 22 Sep 2021 12:24:27 +0200 Subject: [PATCH 04/17] ls: move SELinux String building logic to its own function --- src/uu/ls/src/ls.rs | 84 ++++++++++++++++++++++++--------------------- 1 file changed, 45 insertions(+), 39 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 911323715..ba666d740 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1259,45 +1259,7 @@ impl PathData { None => OnceCell::new(), }; - let substitute_string = "?".to_string(); - let security_context = if config.context { - if config.selinux_supported { - #[cfg(feature = "selinux")] - { - match selinux::SecurityContext::of_path(&p_buf, must_dereference, false) { - Err(_r) => { - // TODO: show the actual reason why it failed - show_warning!("failed to get security context of: {}", p_buf.quote()); - substitute_string - } - Ok(None) => substitute_string, - Ok(Some(context)) => { - let mut context = context.as_bytes(); - if context.ends_with(&[0]) { - // TODO: replace with `strip_prefix()` when MSRV >= 1.51 - context = &context[..context.len() - 1] - }; - String::from_utf8(context.to_vec()).unwrap_or_else(|e| { - show_warning!( - "getting security context of: {}: {}", - p_buf.quote(), - e.to_string() - ); - String::from_utf8_lossy(context).into_owned() - }) - } - } - } - #[cfg(not(feature = "selinux"))] - { - substitute_string - } - } else { - substitute_string - } - } else { - String::new() - }; + let security_context = get_security_context(config, &p_buf, must_dereference); Self { md: OnceCell::new(), @@ -2124,3 +2086,47 @@ fn display_symlink_count(_metadata: &Metadata) -> String { fn display_symlink_count(metadata: &Metadata) -> String { metadata.nlink().to_string() } + +// This returns the SELinux security context as UTF8 `String`. +// In the long term this should be changed to `OsStr`, see discussions at #2621/#2656 +fn get_security_context(config: &Config, p_buf: &PathBuf, must_dereference: bool) -> String { + let substitute_string = "?".to_string(); + if config.context { + if config.selinux_supported { + #[cfg(feature = "selinux")] + { + match selinux::SecurityContext::of_path(p_buf, must_dereference, false) { + Err(_r) => { + // TODO: show the actual reason why it failed + show_warning!("failed to get security context of: {}", p_buf.quote()); + substitute_string + } + Ok(None) => substitute_string, + Ok(Some(context)) => { + let mut context = context.as_bytes(); + if context.ends_with(&[0]) { + // TODO: replace with `strip_prefix()` when MSRV >= 1.51 + context = &context[..context.len() - 1] + }; + String::from_utf8(context.to_vec()).unwrap_or_else(|e| { + show_warning!( + "getting security context of: {}: {}", + p_buf.quote(), + e.to_string() + ); + String::from_utf8_lossy(context).into_owned() + }) + } + } + } + #[cfg(not(feature = "selinux"))] + { + substitute_string + } + } else { + substitute_string + } + } else { + String::new() + } +} From 8cd8c25b0dd59aef697dbf772944fc6d57967cda Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Wed, 22 Sep 2021 13:49:08 +0200 Subject: [PATCH 05/17] ls: silence clippy warnings if feat_selinx is not set --- src/uu/ls/src/ls.rs | 69 +++++++++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 34 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index ba666d740..51dc0d247 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1259,7 +1259,11 @@ impl PathData { None => OnceCell::new(), }; - let security_context = get_security_context(config, &p_buf, must_dereference); + let security_context = if config.context { + get_security_context(config, &p_buf, must_dereference) + } else { + String::new() + }; Self { md: OnceCell::new(), @@ -2089,44 +2093,41 @@ fn display_symlink_count(metadata: &Metadata) -> String { // This returns the SELinux security context as UTF8 `String`. // In the long term this should be changed to `OsStr`, see discussions at #2621/#2656 -fn get_security_context(config: &Config, p_buf: &PathBuf, must_dereference: bool) -> String { +#[allow(unused_variables)] +fn get_security_context(config: &Config, p_buf: &Path, must_dereference: bool) -> String { let substitute_string = "?".to_string(); - if config.context { - if config.selinux_supported { - #[cfg(feature = "selinux")] - { - match selinux::SecurityContext::of_path(p_buf, must_dereference, false) { - Err(_r) => { - // TODO: show the actual reason why it failed - show_warning!("failed to get security context of: {}", p_buf.quote()); - substitute_string - } - Ok(None) => substitute_string, - Ok(Some(context)) => { - let mut context = context.as_bytes(); - if context.ends_with(&[0]) { - // TODO: replace with `strip_prefix()` when MSRV >= 1.51 - context = &context[..context.len() - 1] - }; - String::from_utf8(context.to_vec()).unwrap_or_else(|e| { - show_warning!( - "getting security context of: {}: {}", - p_buf.quote(), - e.to_string() - ); - String::from_utf8_lossy(context).into_owned() - }) - } + if config.selinux_supported { + #[cfg(feature = "selinux")] + { + match selinux::SecurityContext::of_path(p_buf, must_dereference, false) { + Err(_r) => { + // TODO: show the actual reason why it failed + show_warning!("failed to get security context of: {}", p_buf.quote()); + substitute_string + } + Ok(None) => substitute_string, + Ok(Some(context)) => { + let mut context = context.as_bytes(); + if context.ends_with(&[0]) { + // TODO: replace with `strip_prefix()` when MSRV >= 1.51 + context = &context[..context.len() - 1] + }; + String::from_utf8(context.to_vec()).unwrap_or_else(|e| { + show_warning!( + "getting security context of: {}: {}", + p_buf.quote(), + e.to_string() + ); + String::from_utf8_lossy(context).into_owned() + }) } } - #[cfg(not(feature = "selinux"))] - { - substitute_string - } - } else { + } + #[cfg(not(feature = "selinux"))] + { substitute_string } } else { - String::new() + substitute_string } } From d013461a6fa963c130468d4c7e26ddd437082b9c Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Fri, 1 Oct 2021 22:27:01 -0400 Subject: [PATCH 06/17] ls: replace redundant closure with function itself --- src/uu/ls/src/ls.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 51dc0d247..3fa3b4f8e 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1967,11 +1967,7 @@ fn display_file_name( #[cfg(unix)] { if config.format != Format::Long && config.inode { - name = path - .md() - .map_or_else(|| "?".to_string(), |md| get_inode(md)) - + " " - + &name; + name = path.md().map_or_else(|| "?".to_string(), get_inode) + " " + &name; } } From 71b7d6b57da8853059d8a78e115ac3dbce27280d Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Fri, 1 Oct 2021 22:27:19 -0400 Subject: [PATCH 07/17] more: remove redundant mut from stdout accesses --- src/uu/more/src/more.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/more/src/more.rs b/src/uu/more/src/more.rs index 3a601c1e8..d424d5a77 100644 --- a/src/uu/more/src/more.rs +++ b/src/uu/more/src/more.rs @@ -210,7 +210,7 @@ fn reset_term(stdout: &mut std::io::Stdout) { #[inline(always)] fn reset_term(_: &mut usize) {} -fn more(buff: &str, mut stdout: &mut Stdout, next_file: Option<&str>, silent: bool) { +fn more(buff: &str, stdout: &mut Stdout, next_file: Option<&str>, silent: bool) { let (cols, rows) = terminal::size().unwrap(); let lines = break_buff(buff, usize::from(cols)); @@ -232,7 +232,7 @@ fn more(buff: &str, mut stdout: &mut Stdout, next_file: Option<&str>, silent: bo code: KeyCode::Char('c'), modifiers: KeyModifiers::CONTROL, }) => { - reset_term(&mut stdout); + reset_term(stdout); std::process::exit(0); } Event::Key(KeyEvent { From 06ae968ecf732ce53954f01b85fbae5969a49d38 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Fri, 1 Oct 2021 23:01:06 -0400 Subject: [PATCH 08/17] csplit: use assert! instead of if then panic! --- src/uu/csplit/src/csplit.rs | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/uu/csplit/src/csplit.rs b/src/uu/csplit/src/csplit.rs index dbf65b71d..0d99154df 100644 --- a/src/uu/csplit/src/csplit.rs +++ b/src/uu/csplit/src/csplit.rs @@ -320,18 +320,19 @@ impl<'a> SplitWriter<'a> { let l = line?; match n.cmp(&(&ln + 1)) { Ordering::Less => { - if input_iter.add_line_to_buffer(ln, l).is_some() { - panic!("the buffer is big enough to contain 1 line"); - } + assert!( + input_iter.add_line_to_buffer(ln, l).is_none(), + "the buffer is big enough to contain 1 line" + ); ret = Ok(()); break; } Ordering::Equal => { - if !self.options.suppress_matched - && input_iter.add_line_to_buffer(ln, l).is_some() - { - panic!("the buffer is big enough to contain 1 line"); - } + assert!( + self.options.suppress_matched + || input_iter.add_line_to_buffer(ln, l).is_none(), + "the buffer is big enough to contain 1 line" + ); ret = Ok(()); break; } @@ -378,9 +379,10 @@ impl<'a> SplitWriter<'a> { match (self.options.suppress_matched, offset) { // no offset, add the line to the next split (false, 0) => { - if input_iter.add_line_to_buffer(ln, l).is_some() { - panic!("the buffer is big enough to contain 1 line"); - } + assert!( + input_iter.add_line_to_buffer(ln, l).is_none(), + "the buffer is big enough to contain 1 line" + ); } // a positive offset, some more lines need to be added to the current split (false, _) => self.writeln(l)?, @@ -425,9 +427,10 @@ impl<'a> SplitWriter<'a> { if !self.options.suppress_matched { // add 1 to the buffer size to make place for the matched line input_iter.set_size_of_buffer(offset_usize + 1); - if input_iter.add_line_to_buffer(ln, l).is_some() { - panic!("should be big enough to hold every lines"); - } + assert!( + input_iter.add_line_to_buffer(ln, l).is_none(), + "should be big enough to hold every lines" + ); } self.finish_split(); if input_iter.buffer_len() < offset_usize { From de158c012243554b71ced15f35eca45fd1a86c80 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Fri, 1 Oct 2021 23:02:17 -0400 Subject: [PATCH 09/17] sort: replace redundant closure with function itself --- src/uu/sort/src/sort.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index fe286aa6d..bd79a6811 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -825,7 +825,7 @@ impl FieldSelector { fn parse(key: &str, global_settings: &GlobalSettings) -> UResult { let mut from_to = key.split(','); let (from, from_options) = Self::split_key_options(from_to.next().unwrap()); - let to = from_to.next().map(|to| Self::split_key_options(to)); + let to = from_to.next().map(Self::split_key_options); let options_are_empty = from_options.is_empty() && matches!(to, None | Some((_, ""))); if options_are_empty { From 6aee05a0f141b935f646ec1cd874feab93c5ebf0 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sat, 2 Oct 2021 09:44:16 -0400 Subject: [PATCH 10/17] od: use assert! instead of if then panic! --- src/uu/od/src/output_info.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/uu/od/src/output_info.rs b/src/uu/od/src/output_info.rs index 49c2a09a2..cf050475a 100644 --- a/src/uu/od/src/output_info.rs +++ b/src/uu/od/src/output_info.rs @@ -145,13 +145,12 @@ impl OutputInfo { byte_size_block: usize, print_width_block: usize, ) -> [usize; MAX_BYTES_PER_UNIT] { - if byte_size_block > MAX_BYTES_PER_UNIT { - panic!( - "{}-bits types are unsupported. Current max={}-bits.", - 8 * byte_size_block, - 8 * MAX_BYTES_PER_UNIT - ); - } + assert!( + byte_size_block <= MAX_BYTES_PER_UNIT, + "{}-bits types are unsupported. Current max={}-bits.", + 8 * byte_size_block, + 8 * MAX_BYTES_PER_UNIT + ); let mut spacing = [0; MAX_BYTES_PER_UNIT]; let mut byte_size = sf.byte_size(); From 548a5121ae5bbab718896688dc8cebfe30f1bdc0 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sat, 2 Oct 2021 10:15:15 -0400 Subject: [PATCH 11/17] dd: use assert! instead of if then panic! --- src/uu/dd/src/parseargs/unit_tests.rs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/uu/dd/src/parseargs/unit_tests.rs b/src/uu/dd/src/parseargs/unit_tests.rs index b898f1e5d..21900ee49 100644 --- a/src/uu/dd/src/parseargs/unit_tests.rs +++ b/src/uu/dd/src/parseargs/unit_tests.rs @@ -35,12 +35,11 @@ fn unimplemented_flags_should_error_non_linux() { } } - if !succeeded.is_empty() { - panic!( - "The following flags did not panic as expected: {:?}", - succeeded - ); - } + assert!( + succeeded.is_empty(), + "The following flags did not panic as expected: {:?}", + succeeded + ); } #[test] @@ -64,12 +63,11 @@ fn unimplemented_flags_should_error() { } } - if !succeeded.is_empty() { - panic!( - "The following flags did not panic as expected: {:?}", - succeeded - ); - } + assert!( + succeeded.is_empty(), + "The following flags did not panic as expected: {:?}", + succeeded + ); } #[test] From 9dd401c3584633376440ea61af40ec786a727077 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Sat, 2 Oct 2021 23:10:00 -0300 Subject: [PATCH 12/17] base32: base_common use UResult --- src/uu/base32/src/base_common.rs | 78 ++++++++++++++++---------------- 1 file changed, 38 insertions(+), 40 deletions(-) diff --git a/src/uu/base32/src/base_common.rs b/src/uu/base32/src/base_common.rs index 0fd0fa5c4..015925e12 100644 --- a/src/uu/base32/src/base_common.rs +++ b/src/uu/base32/src/base_common.rs @@ -11,13 +11,16 @@ use std::io::{stdout, Read, Write}; use uucore::display::Quotable; use uucore::encoding::{wrap_print, Data, Format}; +use uucore::error::{FromIo, UResult, USimpleError, UUsageError}; use uucore::InvalidEncodingHandling; use std::fs::File; use std::io::{BufReader, Stdin}; use std::path::Path; -use clap::{App, Arg}; +use clap::{crate_version, App, Arg}; + +pub static BASE_CMD_PARSE_ERROR: i32 = 1; // Config. pub struct Config { @@ -35,15 +38,14 @@ pub mod options { } impl Config { - pub fn from(app_name: &str, options: &clap::ArgMatches) -> Result { + pub fn from(options: &clap::ArgMatches) -> UResult { let file: Option = match options.values_of(options::FILE) { Some(mut values) => { let name = values.next().unwrap(); if let Some(extra_op) = values.next() { - return Err(format!( - "extra operand {}\nTry '{} --help' for more information.", - extra_op.quote(), - app_name + return Err(UUsageError::new( + BASE_CMD_PARSE_ERROR, + format!("extra operand {}", extra_op.quote(),), )); } @@ -51,7 +53,10 @@ impl Config { None } else { if !Path::exists(Path::new(name)) { - return Err(format!("{}: No such file or directory", name.maybe_quote())); + return Err(USimpleError::new( + BASE_CMD_PARSE_ERROR, + format!("{}: No such file or directory", name.maybe_quote()), + )); } Some(name.to_owned()) } @@ -62,8 +67,12 @@ impl Config { let cols = options .value_of(options::WRAP) .map(|num| { - num.parse::() - .map_err(|_| format!("invalid wrap size: {}", num.quote())) + num.parse::().map_err(|_| { + USimpleError::new( + BASE_CMD_PARSE_ERROR, + format!("invalid wrap size: {}", num.quote()), + ) + }) }) .transpose()?; @@ -76,23 +85,17 @@ impl Config { } } -pub fn parse_base_cmd_args( - args: impl uucore::Args, - name: &str, - version: &str, - about: &str, - usage: &str, -) -> Result { - let app = base_app(name, version, about).usage(usage); +pub fn parse_base_cmd_args(args: impl uucore::Args, about: &str, usage: &str) -> UResult { + let app = base_app(about).usage(usage); let arg_list = args .collect_str(InvalidEncodingHandling::ConvertLossy) .accept_any(); - Config::from(name, &app.get_matches_from(arg_list)) + Config::from(&app.get_matches_from(arg_list)) } -pub fn base_app<'a>(name: &str, version: &'a str, about: &'a str) -> App<'static, 'a> { - App::new(name) - .version(version) +pub fn base_app<'a>(about: &'a str) -> App<'static, 'a> { + App::new(uucore::util_name()) + .version(crate_version!()) .about(about) // Format arguments. .arg( @@ -121,14 +124,15 @@ pub fn base_app<'a>(name: &str, version: &'a str, about: &'a str) -> App<'static .arg(Arg::with_name(options::FILE).index(1).multiple(true)) } -pub fn get_input<'a>(config: &Config, stdin_ref: &'a Stdin) -> Box { +pub fn get_input<'a>(config: &Config, stdin_ref: &'a Stdin) -> UResult> { match &config.to_read { Some(name) => { - let file_buf = crash_if_err!(1, File::open(Path::new(name))); - Box::new(BufReader::new(file_buf)) // as Box + let file_buf = + File::open(Path::new(name)).map_err_context(|| name.maybe_quote().to_string())?; + Ok(Box::new(BufReader::new(file_buf))) // as Box } None => { - Box::new(stdin_ref.lock()) // as Box + Ok(Box::new(stdin_ref.lock())) // as Box } } } @@ -139,8 +143,7 @@ pub fn handle_input( line_wrap: Option, ignore_garbage: bool, decode: bool, - name: &str, -) { +) -> UResult<()> { let mut data = Data::new(input, format).ignore_garbage(ignore_garbage); if let Some(wrap) = line_wrap { data = data.line_wrap(wrap); @@ -150,28 +153,23 @@ pub fn handle_input( match data.encode() { Ok(s) => { wrap_print(&data, s); + Ok(()) } - Err(_) => { - eprintln!( - "{}: error: invalid input (length must be multiple of 4 characters)", - name - ); - exit!(1) - } + Err(_) => Err(USimpleError::new( + 1, + "error: invalid input (length must be multiple of 4 characters)", + )), } } else { match data.decode() { Ok(s) => { if stdout().write_all(&s).is_err() { // on windows console, writing invalid utf8 returns an error - eprintln!("{}: error: Cannot write non-utf8 data", name); - exit!(1) + return Err(USimpleError::new(1, "error: cannot write non-utf8 data")); } + Ok(()) } - Err(_) => { - eprintln!("{}: error: invalid input", name); - exit!(1) - } + Err(_) => Err(USimpleError::new(1, "error: invalid input")), } } } From 97df700d67acb6d6c94859a713445aaffbdbbb67 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Sat, 2 Oct 2021 23:12:09 -0300 Subject: [PATCH 13/17] base32: use UResult --- src/uu/base32/src/base32.rs | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/uu/base32/src/base32.rs b/src/uu/base32/src/base32.rs index 667fd927e..0159ef181 100644 --- a/src/uu/base32/src/base32.rs +++ b/src/uu/base32/src/base32.rs @@ -11,7 +11,7 @@ extern crate uucore; use std::io::{stdin, Read}; use clap::App; -use uucore::encoding::Format; +use uucore::{encoding::Format, error::UResult}; pub mod base_common; @@ -24,27 +24,22 @@ static ABOUT: &str = " to attempt to recover from any other non-alphabet bytes in the encoded stream. "; -static VERSION: &str = env!("CARGO_PKG_VERSION"); - -static BASE_CMD_PARSE_ERROR: i32 = 1; fn usage() -> String { format!("{0} [OPTION]... [FILE]", uucore::execution_phrase()) } -pub fn uumain(args: impl uucore::Args) -> i32 { +#[uucore_procs::gen_uumain] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { let format = Format::Base32; let usage = usage(); - let name = uucore::util_name(); - let config_result: Result = - base_common::parse_base_cmd_args(args, name, VERSION, ABOUT, &usage); - let config = config_result.unwrap_or_else(|s| crash!(BASE_CMD_PARSE_ERROR, "{}", s)); + let config: base_common::Config = base_common::parse_base_cmd_args(args, ABOUT, &usage)?; // Create a reference to stdin so we can return a locked stdin from // parse_base_cmd_args let stdin_raw = stdin(); - let mut input: Box = base_common::get_input(&config, &stdin_raw); + let mut input: Box = base_common::get_input(&config, &stdin_raw)?; base_common::handle_input( &mut input, @@ -52,12 +47,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 { config.wrap_cols, config.ignore_garbage, config.decode, - name, - ); - - 0 + ) } pub fn uu_app() -> App<'static, 'static> { - base_common::base_app(uucore::util_name(), VERSION, ABOUT) + base_common::base_app(ABOUT) } From 051284de4ce1322ce0574b4940f37199cfdf0b14 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Sat, 2 Oct 2021 23:12:42 -0300 Subject: [PATCH 14/17] tests/base32: update test --- tests/by-util/test_base32.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/by-util/test_base32.rs b/tests/by-util/test_base32.rs index 5c74c5b59..ffe2cf74c 100644 --- a/tests/by-util/test_base32.rs +++ b/tests/by-util/test_base32.rs @@ -113,12 +113,18 @@ fn test_wrap_bad_arg() { #[test] fn test_base32_extra_operand() { + let ts = TestScenario::new(util_name!()); + // Expect a failure when multiple files are specified. - new_ucmd!() + ts.ucmd() .arg("a.txt") .arg("b.txt") .fails() - .stderr_only("base32: extra operand 'b.txt'\nTry 'base32 --help' for more information."); + .stderr_only(format!( + "{0}: extra operand 'b.txt'\nTry '{1} {0} --help' for more information.", + ts.util_name, + ts.bin_path.to_string_lossy() + )); } #[test] From 452329ad199878e9f483c67621791f94cbc2eaa2 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Sat, 2 Oct 2021 23:15:40 -0300 Subject: [PATCH 15/17] base64: use UResult --- src/uu/base64/src/base64.rs | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/uu/base64/src/base64.rs b/src/uu/base64/src/base64.rs index ded157362..ed8b1b461 100644 --- a/src/uu/base64/src/base64.rs +++ b/src/uu/base64/src/base64.rs @@ -12,7 +12,7 @@ extern crate uucore; use uu_base32::base_common; pub use uu_base32::uu_app; -use uucore::encoding::Format; +use uucore::{encoding::Format, error::UResult}; use std::io::{stdin, Read}; @@ -25,26 +25,22 @@ static ABOUT: &str = " to attempt to recover from any other non-alphabet bytes in the encoded stream. "; -static VERSION: &str = env!("CARGO_PKG_VERSION"); - -static BASE_CMD_PARSE_ERROR: i32 = 1; fn usage() -> String { format!("{0} [OPTION]... [FILE]", uucore::execution_phrase()) } -pub fn uumain(args: impl uucore::Args) -> i32 { +#[uucore_procs::gen_uumain] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { let format = Format::Base64; let usage = usage(); - let name = uucore::util_name(); - let config_result: Result = - base_common::parse_base_cmd_args(args, name, VERSION, ABOUT, &usage); - let config = config_result.unwrap_or_else(|s| crash!(BASE_CMD_PARSE_ERROR, "{}", s)); + + let config: base_common::Config = base_common::parse_base_cmd_args(args, ABOUT, &usage)?; // Create a reference to stdin so we can return a locked stdin from // parse_base_cmd_args let stdin_raw = stdin(); - let mut input: Box = base_common::get_input(&config, &stdin_raw); + let mut input: Box = base_common::get_input(&config, &stdin_raw)?; base_common::handle_input( &mut input, @@ -52,8 +48,5 @@ pub fn uumain(args: impl uucore::Args) -> i32 { config.wrap_cols, config.ignore_garbage, config.decode, - name, - ); - - 0 + ) } From f85ccf8e460dca0a11f4a7a8440aa62e02964952 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Sat, 2 Oct 2021 23:16:07 -0300 Subject: [PATCH 16/17] tests/base64: update test --- tests/by-util/test_base64.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/by-util/test_base64.rs b/tests/by-util/test_base64.rs index a860aae91..87aa0db44 100644 --- a/tests/by-util/test_base64.rs +++ b/tests/by-util/test_base64.rs @@ -95,12 +95,18 @@ fn test_wrap_bad_arg() { #[test] fn test_base64_extra_operand() { + let ts = TestScenario::new(util_name!()); + // Expect a failure when multiple files are specified. - new_ucmd!() + ts.ucmd() .arg("a.txt") .arg("b.txt") .fails() - .stderr_only("base64: extra operand 'b.txt'\nTry 'base64 --help' for more information."); + .stderr_only(format!( + "{0}: extra operand 'b.txt'\nTry '{1} {0} --help' for more information.", + ts.util_name, + ts.bin_path.to_string_lossy() + )); } #[test] From b924774c8a2e6cedffa22ab5e8f429d885362c46 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Sat, 2 Oct 2021 23:17:16 -0300 Subject: [PATCH 17/17] basenc: use UResult --- src/uu/basenc/src/basenc.rs | 40 ++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/src/uu/basenc/src/basenc.rs b/src/uu/basenc/src/basenc.rs index 86c251ad1..a730d3e0f 100644 --- a/src/uu/basenc/src/basenc.rs +++ b/src/uu/basenc/src/basenc.rs @@ -11,10 +11,14 @@ #[macro_use] extern crate uucore; -use clap::{crate_version, App, Arg}; -use uu_base32::base_common::{self, Config}; +use clap::{App, Arg}; +use uu_base32::base_common::{self, Config, BASE_CMD_PARSE_ERROR}; -use uucore::{encoding::Format, InvalidEncodingHandling}; +use uucore::{ + encoding::Format, + error::{UResult, UUsageError}, + InvalidEncodingHandling, +}; use std::io::{stdin, Read}; @@ -26,8 +30,6 @@ static ABOUT: &str = " from any other non-alphabet bytes in the encoded stream. "; -static BASE_CMD_PARSE_ERROR: i32 = 1; - const ENCODINGS: &[(&str, Format)] = &[ ("base64", Format::Base64), ("base64url", Format::Base64Url), @@ -47,14 +49,14 @@ fn usage() -> String { } pub fn uu_app() -> App<'static, 'static> { - let mut app = base_common::base_app(uucore::util_name(), crate_version!(), ABOUT); + let mut app = base_common::base_app(ABOUT); for encoding in ENCODINGS { app = app.arg(Arg::with_name(encoding.0).long(encoding.0)); } app } -fn parse_cmd_args(args: impl uucore::Args) -> (Config, Format) { +fn parse_cmd_args(args: impl uucore::Args) -> UResult<(Config, Format)> { let usage = usage(); let matches = uu_app().usage(&usage[..]).get_matches_from( args.collect_str(InvalidEncodingHandling::ConvertLossy) @@ -63,24 +65,19 @@ fn parse_cmd_args(args: impl uucore::Args) -> (Config, Format) { let format = ENCODINGS .iter() .find(|encoding| matches.is_present(encoding.0)) - .unwrap_or_else(|| { - show_usage_error!("missing encoding type"); - std::process::exit(1) - }) + .ok_or_else(|| UUsageError::new(BASE_CMD_PARSE_ERROR, "missing encoding type"))? .1; - ( - Config::from("basenc", &matches).unwrap_or_else(|s| crash!(BASE_CMD_PARSE_ERROR, "{}", s)), - format, - ) + let config = Config::from(&matches)?; + Ok((config, format)) } -pub fn uumain(args: impl uucore::Args) -> i32 { - let name = uucore::util_name(); - let (config, format) = parse_cmd_args(args); +#[uucore_procs::gen_uumain] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { + let (config, format) = parse_cmd_args(args)?; // Create a reference to stdin so we can return a locked stdin from // parse_base_cmd_args let stdin_raw = stdin(); - let mut input: Box = base_common::get_input(&config, &stdin_raw); + let mut input: Box = base_common::get_input(&config, &stdin_raw)?; base_common::handle_input( &mut input, @@ -88,8 +85,5 @@ pub fn uumain(args: impl uucore::Args) -> i32 { config.wrap_cols, config.ignore_garbage, config.decode, - name, - ); - - 0 + ) }