diff --git a/src/uu/basename/src/basename.rs b/src/uu/basename/src/basename.rs index dd53522c5b8..b9faf5851b1 100644 --- a/src/uu/basename/src/basename.rs +++ b/src/uu/basename/src/basename.rs @@ -5,8 +5,11 @@ // spell-checker:ignore (ToDO) fullname +use clap::builder::ValueParser; use clap::{Arg, ArgAction, Command}; use std::collections::HashMap; +use std::ffi::OsString; +use std::io::{Write, stdout}; use std::path::PathBuf; use uucore::display::Quotable; use uucore::error::{UResult, UUsageError}; @@ -24,8 +27,6 @@ pub mod options { #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let args = args.collect_lossy(); - // // Argument parsing // @@ -34,7 +35,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let line_ending = LineEnding::from_zero_flag(matches.get_flag(options::ZERO)); let mut name_args = matches - .get_many::(options::NAME) + .get_many::(options::NAME) .unwrap_or_default() .collect::>(); if name_args.is_empty() { @@ -43,18 +44,18 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { get_message("basename-error-missing-operand"), )); } - let multiple_paths = - matches.get_one::(options::SUFFIX).is_some() || matches.get_flag(options::MULTIPLE); + let multiple_paths = matches.get_one::(options::SUFFIX).is_some() + || matches.get_flag(options::MULTIPLE); let suffix = if multiple_paths { matches - .get_one::(options::SUFFIX) + .get_one::(options::SUFFIX) .cloned() .unwrap_or_default() } else { // "simple format" match name_args.len() { 0 => panic!("already checked"), - 1 => String::default(), + 1 => OsString::default(), 2 => name_args.pop().unwrap().clone(), _ => { return Err(UUsageError::new( @@ -73,7 +74,8 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // for path in name_args { - print!("{}{line_ending}", basename(path, &suffix)); + stdout().write_all(&basename(path, &suffix)?)?; + print!("{line_ending}"); } Ok(()) @@ -96,6 +98,7 @@ pub fn uu_app() -> Command { .arg( Arg::new(options::NAME) .action(ArgAction::Append) + .value_parser(ValueParser::os_string()) .value_hint(clap::ValueHint::AnyPath) .hide(true) .trailing_var_arg(true), @@ -105,6 +108,7 @@ pub fn uu_app() -> Command { .short('s') .long(options::SUFFIX) .value_name("SUFFIX") + .value_parser(ValueParser::os_string()) .help(get_message("basename-help-suffix")) .overrides_with(options::SUFFIX), ) @@ -118,21 +122,30 @@ pub fn uu_app() -> Command { ) } -fn basename(fullname: &str, suffix: &str) -> String { +// We return a Vec. Returning a seemingly more proper `OsString` would +// require back and forth conversions as we need a &[u8] for printing anyway. +fn basename(fullname: &OsString, suffix: &OsString) -> UResult> { + let fullname_bytes = uucore::os_str_as_bytes(fullname)?; + // Handle special case where path ends with /. - if fullname.ends_with("/.") { - return ".".to_string(); + if fullname_bytes.ends_with(b"/.") { + return Ok(b".".into()); } // Convert to path buffer and get last path component let pb = PathBuf::from(fullname); - pb.components().next_back().map_or_else(String::new, |c| { - let name = c.as_os_str().to_str().unwrap(); + pb.components().next_back().map_or(Ok([].into()), |c| { + let name = c.as_os_str(); + let name_bytes = uucore::os_str_as_bytes(name)?; if name == suffix { - name.to_string() + Ok(name_bytes.into()) } else { - name.strip_suffix(suffix).unwrap_or(name).to_string() + let suffix_bytes = uucore::os_str_as_bytes(suffix)?; + Ok(name_bytes + .strip_suffix(suffix_bytes) + .unwrap_or(name_bytes) + .into()) } }) } diff --git a/tests/by-util/test_basename.rs b/tests/by-util/test_basename.rs index 7a18d3ee14f..ecbfe6c5dfe 100644 --- a/tests/by-util/test_basename.rs +++ b/tests/by-util/test_basename.rs @@ -4,8 +4,6 @@ // file that was distributed with this source code. // spell-checker:ignore (words) reallylongexecutable nbaz -#[cfg(any(unix, target_os = "redox"))] -use std::ffi::OsStr; use uutests::new_ucmd; #[test] @@ -138,20 +136,25 @@ fn test_too_many_args_output() { .usage_error("extra operand 'c'"); } -#[cfg(any(unix, target_os = "redox"))] -fn test_invalid_utf8_args(os_str: &OsStr) { - let test_vec = vec![os_str.to_os_string()]; - new_ucmd!().args(&test_vec).succeeds().stdout_is("fo�o\n"); -} - #[cfg(any(unix, target_os = "redox"))] #[test] -fn invalid_utf8_args_unix() { - use std::os::unix::ffi::OsStrExt; +fn test_invalid_utf8_args() { + let param = uucore::os_str_from_bytes(b"/tmp/some-\xc0-file.k\xf3") + .expect("Only unix platforms can test non-unicode names"); + + new_ucmd!() + .arg(¶m) + .succeeds() + .stdout_is_bytes(b"some-\xc0-file.k\xf3\n"); + + let suffix = uucore::os_str_from_bytes(b".k\xf3") + .expect("Only unix platforms can test non-unicode names"); - let source = [0x66, 0x6f, 0x80, 0x6f]; - let os_str = OsStr::from_bytes(&source[..]); - test_invalid_utf8_args(os_str); + new_ucmd!() + .arg(¶m) + .arg(&suffix) + .succeeds() + .stdout_is_bytes(b"some-\xc0-file\n"); } #[test]