diff --git a/src/uu/od/src/multifile_reader.rs b/src/uu/od/src/multifile_reader.rs index 7eb10df1548..782a4fcbc42 100644 --- a/src/uu/od/src/multifile_reader.rs +++ b/src/uu/od/src/multifile_reader.rs @@ -5,6 +5,7 @@ // spell-checker:ignore (ToDO) multifile curr fnames fname xfrd fillloop mockstream +use std::ffi::OsStr; use std::fs::File; use std::io; #[cfg(unix)] @@ -16,7 +17,7 @@ use uucore::{display::Quotable, error::strip_errno, show_error, translate}; const SKIP_BUFFER_SIZE: usize = 16 * 1024; pub enum InputSource<'a> { - FileName(&'a str), + FileName(&'a OsStr), Stdin, } @@ -55,7 +56,7 @@ pub struct MultifileReader<'a> { ni: Vec>, curr_file: Option, any_err: bool, - file_name: Option<&'a str>, + file_name: Option<&'a OsStr>, } pub trait HasError { @@ -238,7 +239,10 @@ impl io::Read for MultifileReader<'_> { Err(e) => { show_error!( "{}: {}", - self.file_name.unwrap_or("I/O"), + self.file_name + .unwrap_or(OsStr::new("I/O")) + .maybe_quote() + .external(true), strip_errno(&e) ); self.any_err = true; @@ -275,11 +279,11 @@ mod tests { fn test_multi_file_reader_one_read() { let mut file1 = NamedTempFile::new().unwrap(); file1.write_all(b"abcd").unwrap(); - let path1 = file1.path().to_str().unwrap(); + let path1 = file1.path().as_os_str(); let mut file2 = NamedTempFile::new().unwrap(); file2.write_all(b"ABCD").unwrap(); - let path2 = file2.path().to_str().unwrap(); + let path2 = file2.path().as_os_str(); let inputs = vec![InputSource::FileName(path1), InputSource::FileName(path2)]; let mut v = [0; 10]; @@ -295,11 +299,11 @@ mod tests { fn test_multi_file_reader_two_reads() { let mut file1 = NamedTempFile::new().unwrap(); file1.write_all(b"abcd").unwrap(); - let path1 = file1.path().to_str().unwrap(); + let path1 = file1.path().as_os_str(); let mut file2 = NamedTempFile::new().unwrap(); file2.write_all(b"ABCD").unwrap(); - let path2 = file2.path().to_str().unwrap(); + let path2 = file2.path().as_os_str(); let inputs = vec![InputSource::FileName(path1), InputSource::FileName(path2)]; let mut v = [0; 5]; diff --git a/src/uu/od/src/od.rs b/src/uu/od/src/od.rs index a8d2defb1a6..1e3f2449cf8 100644 --- a/src/uu/od/src/od.rs +++ b/src/uu/od/src/od.rs @@ -70,7 +70,7 @@ struct OdOptions { skip_bytes: u64, read_bytes: Option, label: Option, - input_strings: Vec, + input_strings: Vec, formats: Vec, line_bytes: usize, output_duplicates: bool, @@ -253,12 +253,16 @@ impl OdOptions { /// opens the input and calls `odfunc` to process the input. #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let args = args.collect_ignore(); - let raw_args: Vec = args.iter().map(OsString::from).collect(); + let raw_args: Vec = args.collect(); + // Only used to look up how options were spelled; file operands keep their raw form. + let args: Vec = raw_args + .iter() + .map(|arg| arg.to_string_lossy().into_owned()) + .collect(); let clap_opts = uu_app(); - let clap_matches = uucore::clap_localization::handle_clap_result(clap_opts, &args)?; + let clap_matches = uucore::clap_localization::handle_clap_result(clap_opts, &raw_args)?; // Kept for the caret in SIZE diagnostics, which echoes the command line. let diag_args = uucore::diagnostics::capture(&raw_args); @@ -511,6 +515,7 @@ pub fn uu_app() -> Command { Arg::new(options::FILENAME) .hide(true) .action(ArgAction::Append) + .value_parser(clap::value_parser!(OsString)) .value_hint(clap::ValueHint::FilePath), ) } @@ -597,7 +602,7 @@ where /// Extract and display printable strings from input (od -S option) fn extract_strings_from_input( - input_strings: &[String], + input_strings: &[OsString], skip_bytes: u64, read_bytes: Option, min_length: usize, @@ -788,12 +793,15 @@ fn write_bytes( } /// Helper function to convert input strings to InputSource -fn map_input_strings(input_strings: &[String]) -> Vec> { +fn map_input_strings(input_strings: &[OsString]) -> Vec> { input_strings .iter() - .map(|w| match w as &str { - "-" => InputSource::Stdin, - x => InputSource::FileName(x), + .map(|w| { + if w == "-" { + InputSource::Stdin + } else { + InputSource::FileName(w) + } }) .collect() } @@ -803,7 +811,7 @@ fn map_input_strings(input_strings: &[String]) -> Vec> { /// `skip_bytes` is the number of bytes skipped from the input /// `read_bytes` is an optional limit to the number of bytes to read fn open_input_peek_reader( - input_strings: &[String], + input_strings: &[OsString], skip_bytes: u64, read_bytes: Option, ) -> UResult>>>> { diff --git a/src/uu/od/src/parse_inputs.rs b/src/uu/od/src/parse_inputs.rs index 99e633c2f29..1b96b2f1d01 100644 --- a/src/uu/od/src/parse_inputs.rs +++ b/src/uu/od/src/parse_inputs.rs @@ -5,21 +5,22 @@ use super::options; use clap::ArgMatches; +use std::ffi::{OsStr, OsString}; use uucore::translate; /// Abstraction for getopts pub trait CommandLineOpts { /// returns all command line parameters which do not belong to an option. - fn inputs(&self) -> Vec<&str>; + fn inputs(&self) -> Vec<&OsStr>; /// tests if any of the specified options is present. fn opts_present(&self, _: &[&str]) -> bool; } /// Implementation for `getopts` impl CommandLineOpts for ArgMatches { - fn inputs(&self) -> Vec<&str> { - self.get_many::(options::FILENAME) - .map(|values| values.map(String::as_str).collect()) + fn inputs(&self) -> Vec<&OsStr> { + self.get_many::(options::FILENAME) + .map(|values| values.map(OsString::as_os_str).collect()) .unwrap_or_default() } @@ -38,8 +39,8 @@ impl CommandLineOpts for ArgMatches { /// but it might be 0. #[derive(PartialEq, Eq, Debug)] pub enum CommandLineInputs { - FileNames(Vec), - FileAndOffset((String, u64, Option)), + FileNames(Vec), + FileAndOffset((OsString, u64, Option)), } /// Interprets the command line inputs of od. @@ -69,17 +70,17 @@ pub fn parse_inputs(matches: &dyn CommandLineOpts) -> Result { if let [first] = input_strings[..] - && first.starts_with('+') + && first.as_encoded_bytes().starts_with(b"+") { - return Ok(CommandLineInputs::FileAndOffset(("-".to_string(), n, None))); + return Ok(CommandLineInputs::FileAndOffset(("-".into(), n, None))); } if let [first, _] = input_strings[..] { return Ok(CommandLineInputs::FileAndOffset(( - first.to_string(), + first.to_os_string(), n, None, ))); @@ -93,7 +94,10 @@ pub fn parse_inputs(matches: &dyn CommandLineOpts) -> Result Result Result Result { +pub fn parse_inputs_traditional(input_strings: &[&OsStr]) -> Result { match input_strings.len() { - 0 => Ok(CommandLineInputs::FileNames(vec!["-".to_string()])), + 0 => Ok(CommandLineInputs::FileNames(vec!["-".into()])), 1 => { - let offset0 = parse_offset_operand(input_strings[0]); + let offset0 = parse_offset_operand_os(input_strings[0]); Ok(match offset0 { - Ok(n) => CommandLineInputs::FileAndOffset(("-".to_string(), n, None)), + Ok(n) => CommandLineInputs::FileAndOffset(("-".into(), n, None)), _ => CommandLineInputs::FileNames( - input_strings.iter().map(|&s| s.to_string()).collect(), + input_strings.iter().map(|&s| s.to_os_string()).collect(), ), }) } 2 => { - let offset0 = parse_offset_operand(input_strings[0]); - let offset1 = parse_offset_operand(input_strings[1]); + let offset0 = parse_offset_operand_os(input_strings[0]); + let offset1 = parse_offset_operand_os(input_strings[1]); match (offset0, offset1) { - (Ok(n), Ok(m)) => Ok(CommandLineInputs::FileAndOffset(( - "-".to_string(), - n, - Some(m), - ))), + (Ok(n), Ok(m)) => Ok(CommandLineInputs::FileAndOffset(("-".into(), n, Some(m)))), (_, Ok(m)) => Ok(CommandLineInputs::FileAndOffset(( - input_strings[0].to_string(), + input_strings[0].to_os_string(), m, None, ))), - (_, Err(e)) => Err(format!("{}: {e}", input_strings[1])), + (_, Err(e)) => Err(format!("{}: {e}", input_strings[1].to_string_lossy())), } } 3 => { - let offset = parse_offset_operand(input_strings[1]); - let label = parse_offset_operand(input_strings[2]); + let offset = parse_offset_operand_os(input_strings[1]); + let label = parse_offset_operand_os(input_strings[2]); match (offset, label) { (Ok(n), Ok(m)) => Ok(CommandLineInputs::FileAndOffset(( - input_strings[0].to_string(), + input_strings[0].to_os_string(), n, Some(m), ))), - (Err(e), _) => Err(format!("{}: {e}", input_strings[1])), - (_, Err(e)) => Err(format!("{}: {e}", input_strings[2])), + (Err(e), _) => Err(format!("{}: {e}", input_strings[1].to_string_lossy())), + (_, Err(e)) => Err(format!("{}: {e}", input_strings[2].to_string_lossy())), } } - _ => Err(translate!("od-error-too-many-inputs", "input" => input_strings[3])), + _ => Err( + translate!("od-error-too-many-inputs", "input" => input_strings[3].to_string_lossy()), + ), } } +/// Like [`parse_offset_operand`], for an operand that may not be valid UTF-8 +/// (and is then a file name, never an offset). +fn parse_offset_operand_os(s: &OsStr) -> Result { + s.to_str().map_or_else( + || Err(translate!("od-error-parse-failed")), + parse_offset_operand, + ) +} + /// parses format used by offset and label on the command line pub fn parse_offset_operand(s: &str) -> Result { if s.is_empty() { @@ -243,31 +254,27 @@ mod tests { #[test] fn test_parse_inputs_normal() { assert_eq!( - CommandLineInputs::FileNames(vec!["-".to_string()]), + CommandLineInputs::FileNames(vec!["-".into()]), parse_inputs(&uu_app().get_matches_from(vec!["od"])).unwrap() ); assert_eq!( - CommandLineInputs::FileNames(vec!["-".to_string()]), + CommandLineInputs::FileNames(vec!["-".into()]), parse_inputs(&uu_app().get_matches_from(vec!["od", "-"])).unwrap() ); assert_eq!( - CommandLineInputs::FileNames(vec!["file1".to_string()]), + CommandLineInputs::FileNames(vec!["file1".into()]), parse_inputs(&uu_app().get_matches_from(vec!["od", "file1"])).unwrap() ); assert_eq!( - CommandLineInputs::FileNames(vec!["file1".to_string(), "file2".to_string()]), + CommandLineInputs::FileNames(vec!["file1".into(), "file2".into()]), parse_inputs(&uu_app().get_matches_from(vec!["od", "file1", "file2"])).unwrap() ); assert_eq!( - CommandLineInputs::FileNames(vec![ - "-".to_string(), - "file1".to_string(), - "file2".to_string(), - ]), + CommandLineInputs::FileNames(vec!["-".into(), "file1".into(), "file2".into()]), parse_inputs(&uu_app().get_matches_from(vec!["od", "-", "file1", "file2"])).unwrap() ); } @@ -277,58 +284,58 @@ mod tests { fn test_parse_inputs_with_offset() { // offset is found without filename, so stdin will be used. assert_eq!( - CommandLineInputs::FileAndOffset(("-".to_string(), 8, None)), + CommandLineInputs::FileAndOffset(("-".into(), 8, None)), parse_inputs(&uu_app().get_matches_from(vec!["od", "+10"])).unwrap() ); // offset must start with "+" if no input is specified. assert_eq!( - CommandLineInputs::FileNames(vec!["10".to_string()]), + CommandLineInputs::FileNames(vec!["10".into()]), parse_inputs(&uu_app().get_matches_from(vec!["od", "10"])).unwrap() ); // offset is not valid, so it is considered a filename. assert_eq!( - CommandLineInputs::FileNames(vec!["+10a".to_string()]), + CommandLineInputs::FileNames(vec!["+10a".into()]), parse_inputs(&uu_app().get_matches_from(vec!["od", "+10a"])).unwrap() ); // if -j is included in the command line, there cannot be an offset. assert_eq!( - CommandLineInputs::FileNames(vec!["+10".to_string()]), + CommandLineInputs::FileNames(vec!["+10".into()]), parse_inputs(&uu_app().get_matches_from(vec!["od", "-j10", "+10"])).unwrap() ); // if -v is included in the command line, there cannot be an offset. assert_eq!( - CommandLineInputs::FileNames(vec!["+10".to_string()]), + CommandLineInputs::FileNames(vec!["+10".into()]), parse_inputs(&uu_app().get_matches_from(vec!["od", "-o", "-v", "+10"])).unwrap() ); assert_eq!( - CommandLineInputs::FileAndOffset(("file1".to_string(), 8, None)), + CommandLineInputs::FileAndOffset(("file1".into(), 8, None)), parse_inputs(&uu_app().get_matches_from(vec!["od", "file1", "+10"])).unwrap() ); // offset does not need to start with "+" if a filename is included. assert_eq!( - CommandLineInputs::FileAndOffset(("file1".to_string(), 8, None)), + CommandLineInputs::FileAndOffset(("file1".into(), 8, None)), parse_inputs(&uu_app().get_matches_from(vec!["od", "file1", "10"])).unwrap() ); assert_eq!( - CommandLineInputs::FileNames(vec!["file1".to_string(), "+10a".to_string()]), + CommandLineInputs::FileNames(vec!["file1".into(), "+10a".into()]), parse_inputs(&uu_app().get_matches_from(vec!["od", "file1", "+10a"])).unwrap() ); assert_eq!( - CommandLineInputs::FileNames(vec!["file1".to_string(), "+10".to_string()]), + CommandLineInputs::FileNames(vec!["file1".into(), "+10".into()]), parse_inputs(&uu_app().get_matches_from(vec!["od", "-j10", "file1", "+10"])).unwrap() ); // offset must be last on the command line assert_eq!( - CommandLineInputs::FileNames(vec!["+10".to_string(), "file1".to_string()]), + CommandLineInputs::FileNames(vec!["+10".into(), "file1".into()]), parse_inputs(&uu_app().get_matches_from(vec!["od", "+10", "file1"])).unwrap() ); } @@ -337,30 +344,30 @@ mod tests { fn test_parse_inputs_traditional() { // it should not return FileAndOffset to signal no offset was entered on the command line. assert_eq!( - CommandLineInputs::FileNames(vec!["-".to_string()]), + CommandLineInputs::FileNames(vec!["-".into()]), parse_inputs(&uu_app().get_matches_from(vec!["od", "--traditional"])).unwrap() ); assert_eq!( - CommandLineInputs::FileNames(vec!["file1".to_string()]), + CommandLineInputs::FileNames(vec!["file1".into()]), parse_inputs(&uu_app().get_matches_from(vec!["od", "--traditional", "file1"])).unwrap() ); // offset does not need to start with a + assert_eq!( - CommandLineInputs::FileAndOffset(("-".to_string(), 8, None)), + CommandLineInputs::FileAndOffset(("-".into(), 8, None)), parse_inputs(&uu_app().get_matches_from(vec!["od", "--traditional", "10"])).unwrap() ); // valid offset and valid label assert_eq!( - CommandLineInputs::FileAndOffset(("-".to_string(), 8, Some(8))), + CommandLineInputs::FileAndOffset(("-".into(), 8, Some(8))), parse_inputs(&uu_app().get_matches_from(vec!["od", "--traditional", "10", "10"])) .unwrap() ); assert_eq!( - CommandLineInputs::FileAndOffset(("file1".to_string(), 8, None)), + CommandLineInputs::FileAndOffset(("file1".into(), 8, None)), parse_inputs(&uu_app().get_matches_from(vec!["od", "--traditional", "file1", "10"])) .unwrap() ); @@ -370,7 +377,7 @@ mod tests { .unwrap_err(); assert_eq!( - CommandLineInputs::FileAndOffset(("file1".to_string(), 8, Some(8))), + CommandLineInputs::FileAndOffset(("file1".into(), 8, Some(8))), parse_inputs(&uu_app().get_matches_from(vec![ "od", "--traditional", diff --git a/tests/by-util/test_od.rs b/tests/by-util/test_od.rs index 8868bca7caa..23e0f37a452 100644 --- a/tests/by-util/test_od.rs +++ b/tests/by-util/test_od.rs @@ -93,6 +93,63 @@ fn test_non_existing_file() { new_ucmd!().arg("non_existing_file").fails(); } +// A file operand that is not valid UTF-8 must not be dropped, which made od read stdin instead. +#[test] +#[cfg(unix)] +#[cfg_attr(wasi_runner, ignore = "WASI: argv/filenames must be valid UTF-8")] +fn test_non_utf8_non_existing_file() { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + + new_ucmd!() + .arg(OsStr::from_bytes(b"\xff")) + .pipe_in("abc") + .fails_with_code(1) + .no_stdout() + .stderr_contains("No such file or directory"); +} + +#[test] +#[cfg(target_os = "linux")] +#[cfg_attr(wasi_runner, ignore = "WASI: argv/filenames must be valid UTF-8")] +fn test_non_utf8_file() { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + + let (at, mut ucmd) = at_and_ucmd!(); + let name = OsStr::from_bytes(b"\xff"); + std::fs::write(at.plus(name), "abcdefghijklmnopqrstuvwxyz\n").unwrap(); + ucmd.arg("--endian=little") + .arg(name) + .pipe_in("not read") + .succeeds() + .stdout_only(unindent(ALPHA_OUT)); +} + +#[test] +#[cfg(target_os = "linux")] +#[cfg_attr(wasi_runner, ignore = "WASI: argv/filenames must be valid UTF-8")] +fn test_traditional_non_utf8_file_with_offset() { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + + let (at, mut ucmd) = at_and_ucmd!(); + let name = OsStr::from_bytes(b"\xff"); + std::fs::write(at.plus(name), "abcdefghijklmnopq").unwrap(); + ucmd.arg("--traditional") + .arg("-c") + .arg(name) + .arg("10") + .pipe_in("not read") + .succeeds() + .stdout_only(unindent( + " + 0000010 i j k l m n o p q + 0000021 + ", + )); +} + // Test that od reads from stdin instead of a file #[test] fn test_from_stdin() {