Fix error handling. Add test.

src/uu/echo/src/echo.rs

@@ -355,7 +355,7 @@ fn execute(
  arguments_after_options: ValuesRef<'_, OsString>,
 ) -> UResult<()> {
  for (i, input) in arguments_after_options.enumerate() {
- let bytes = uucore::format::bytes_from_os_str(input)?;
+ let bytes = uucore::format::try_get_bytes_from_os_str(input)?;
 
  if i > 0 {
  stdout_lock.write_all(b" ")?;

src/uu/printf/src/printf.rs

@@ -11,7 +11,9 @@ use std::ffi::OsString;
 use std::io::stdout;
 use std::ops::ControlFlow;
 use uucore::error::{UResult, UUsageError};
-use uucore::format::{bytes_from_os_str, parse_spec_and_escape, FormatArgument, FormatItem};
+use uucore::format::{
+ parse_spec_and_escape, try_get_bytes_from_os_str, FormatArgument, FormatItem,
+};
 use uucore::{format_usage, help_about, help_section, help_usage};
 
 const VERSION: &str = "version";
@@ -33,7 +35,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
  .get_one::<OsString>(options::FORMAT)
  .ok_or_else(|| UUsageError::new(1, "missing operand"))?;
 
- let format_bytes = bytes_from_os_str(format)?;
+ let format_bytes = try_get_bytes_from_os_str(format)?;
 
  let values = match matches.get_many::<OsString>(options::ARGUMENT) {
  Some(os_string) => os_string

src/uucore/src/lib/features/format/argument.rs

@@ -3,14 +3,19 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 
+use super::FormatError;
 use crate::{
- error::{set_exit_code, UResult, USimpleError},
+ error::{set_exit_code, UError},
  features::format::num_parser::{ParseError, ParsedNumber},
  quoting_style::{escape_name, Quotes, QuotingStyle},
- show, show_error, show_warning,
+ show_error, show_warning,
 };
 use os_display::Quotable;
-use std::ffi::{OsStr, OsString};
+use std::{
+ error::Error,
+ ffi::{OsStr, OsString},
+ fmt::Display,
+};
 
 /// An argument for formatting
 ///
@@ -31,70 +36,70 @@ pub enum FormatArgument {
 }
 
 pub trait ArgumentIter<'a>: Iterator<Item = &'a FormatArgument> {
- fn get_char(&mut self) -> u8;
- fn get_i64(&mut self) -> i64;
- fn get_u64(&mut self) -> u64;
- fn get_f64(&mut self) -> f64;
+ fn get_char(&mut self) -> Result<u8, FormatError>;
+ fn get_i64(&mut self) -> Result<i64, FormatError>;
+ fn get_u64(&mut self) -> Result<u64, FormatError>;
+ fn get_f64(&mut self) -> Result<f64, FormatError>;
  fn get_str(&mut self) -> &'a OsStr;
 }
 
 impl<'a, T: Iterator<Item = &'a FormatArgument>> ArgumentIter<'a> for T {
- fn get_char(&mut self) -> u8 {
+ fn get_char(&mut self) -> Result<u8, FormatError> {
  let Some(next) = self.next() else {
- return b'\0';
+ return Ok(b'\0');
  };
  match next {
- FormatArgument::Char(c) => *c as u8,
- FormatArgument::Unparsed(os) => match bytes_from_os_str(os).unwrap().first() {
- Some(&byte) => byte,
- None => b'\0',
+ FormatArgument::Char(c) => Ok(*c as u8),
+ FormatArgument::Unparsed(os) => match try_get_bytes_from_os_str(os)?.first() {
+ Some(&byte) => Ok(byte),
+ None => Ok(b'\0'),
  },
- _ => b'\0',
+ _ => Ok(b'\0'),
  }
  }
 
- fn get_u64(&mut self) -> u64 {
+ fn get_u64(&mut self) -> Result<u64, FormatError> {
  let Some(next) = self.next() else {
- return 0;
+ return Ok(0);
  };
  match next {
- FormatArgument::UnsignedInt(n) => *n,
+ FormatArgument::UnsignedInt(n) => Ok(*n),
  FormatArgument::Unparsed(os) => {
- let str = get_str_or_exit_with_error(os);
+ let str = try_get_str_from_os_str(os)?;
 
- extract_value(ParsedNumber::parse_u64(str), str)
+ Ok(extract_value(ParsedNumber::parse_u64(str), str))
  }
- _ => 0,
+ _ => Ok(0),
  }
  }
 
- fn get_i64(&mut self) -> i64 {
+ fn get_i64(&mut self) -> Result<i64, FormatError> {
  let Some(next) = self.next() else {
- return 0;
+ return Ok(0);
  };
  match next {
- FormatArgument::SignedInt(n) => *n,
+ FormatArgument::SignedInt(n) => Ok(*n),
  FormatArgument::Unparsed(os) => {
- let str = get_str_or_exit_with_error(os);
+ let str = try_get_str_from_os_str(os)?;
 
- extract_value(ParsedNumber::parse_i64(str), str)
+ Ok(extract_value(ParsedNumber::parse_i64(str), str))
  }
- _ => 0,
+ _ => Ok(0),
  }
  }
 
- fn get_f64(&mut self) -> f64 {
+ fn get_f64(&mut self) -> Result<f64, FormatError> {
  let Some(next) = self.next() else {
- return 0.0;
+ return Ok(0.0);
  };
  match next {
- FormatArgument::Float(n) => *n,
+ FormatArgument::Float(n) => Ok(*n),
  FormatArgument::Unparsed(os) => {
- let str = get_str_or_exit_with_error(os);
+ let str = try_get_str_from_os_str(os)?;
 
- extract_value(ParsedNumber::parse_f64(str), str)
+ Ok(extract_value(ParsedNumber::parse_f64(str), str))
  }
- _ => 0.0,
+ _ => Ok(0.0),
  }
  }
 
@@ -135,14 +140,41 @@ fn extract_value<T: Default>(p: Result<T, ParseError<'_, T>>, input: &str) -> T
  } else {
  show_error!("{}: value not completely converted", input.quote());
  }
+
  v
  }
  }
  }
  }
 }
 
-pub fn bytes_from_os_str(input: &OsStr) -> UResult<&[u8]> {
+#[derive(Debug)]
+pub struct NonUtf8OsStr(pub String);
+
+impl Display for NonUtf8OsStr {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ f.write_fmt(format_args!(
+ "invalid (non-UTF-8) string like {} encountered",
+ self.0.quote(),
+ ))
+ }
+}
+
+impl Error for NonUtf8OsStr {}
+impl UError for NonUtf8OsStr {}
+
+pub fn try_get_str_from_os_str(os_str: &OsStr) -> Result<&str, NonUtf8OsStr> {
+ match os_str.to_str() {
+ Some(st) => Ok(st),
+ None => {
+ let cow = os_str.to_string_lossy();
+
+ Err(NonUtf8OsStr(cow.to_string()))
+ }
+ }
+}
+
+pub fn try_get_bytes_from_os_str(input: &OsStr) -> Result<&[u8], NonUtf8OsStr> {
  let result = {
  #[cfg(target_family = "unix")]
  {
@@ -153,38 +185,18 @@ pub fn bytes_from_os_str(input: &OsStr) -> UResult<&[u8]> {
 
  #[cfg(not(target_family = "unix"))]
  {
- use crate::error::USimpleError;
-
  // TODO
  // Verify that this works correctly on these platforms
  match input.to_str().map(|st| st.as_bytes()) {
  Some(sl) => Ok(sl),
- None => Err(USimpleError::new(
- 1,
- "non-UTF-8 string encountered when not allowed",
- )),
+ None => {
+ let cow = input.to_string_lossy();
+
+ Err(NonUtf8OsStr(cow.to_string()))
+ }
  }
  }
  };
 
  result
 }
-
-fn get_str_or_exit_with_error(os_str: &OsStr) -> &str {
- match os_str.to_str() {
- Some(st) => st,
- None => {
- let cow = os_str.to_string_lossy();
-
- let quoted = cow.quote();
-
- let error = format!(
- "argument like {quoted} is not a valid UTF-8 string, and could not be parsed as an integer",
- );
-
- show!(USimpleError::new(1, error.clone()));
-
- panic!("{error}");
- }
- }
-}

src/uucore/src/lib/features/format/mod.rs

@@ -38,6 +38,7 @@ pub mod num_parser;
 mod spec;
 
 pub use argument::*;
+use os_display::Quotable;
 use spec::Spec;
 use std::{
  error::Error,
@@ -63,6 +64,7 @@ pub enum FormatError {
  NeedAtLeastOneSpec(Vec<u8>),
  WrongSpecType,
  InvalidPrecision(String),
+ InvalidEncoding(NonUtf8OsStr),
 }
 
 impl Error for FormatError {}
@@ -74,6 +76,12 @@ impl From<std::io::Error> for FormatError {
  }
 }
 
+impl From<NonUtf8OsStr> for FormatError {
+ fn from(value: NonUtf8OsStr) -> FormatError {
+ FormatError::InvalidEncoding(value)
+ }
+}
+
 impl Display for FormatError {
  fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
  match self {
@@ -98,6 +106,13 @@ impl Display for FormatError {
  Self::IoError(_) => write!(f, "io error"),
  Self::NoMoreArguments => write!(f, "no more arguments"),
  Self::InvalidArgument(_) => write!(f, "invalid argument"),
+ Self::InvalidEncoding(no) => {
+ write!(
+ f,
+ "invalid (non-UTF-8) argument like {} encountered",
+ no.0.quote()
+ )
+ }
  }
  }
 }

src/uucore/src/lib/features/format/spec.rs

@@ -6,12 +6,11 @@
 // spell-checker:ignore (vars) intmax ptrdiff padlen
 
 use super::{
- bytes_from_os_str,
  num_format::{
  self, Case, FloatVariant, ForceDecimal, Formatter, NumberAlignment, PositiveSign, Prefix,
  UnsignedIntVariant,
  },
- parse_escape_only, ArgumentIter, FormatChar, FormatError,
+ parse_escape_only, try_get_bytes_from_os_str, ArgumentIter, FormatChar, FormatError,
 };
 use crate::quoting_style::{escape_name, QuotingStyle};
 use std::{io::Write, ops::ControlFlow};
@@ -315,7 +314,7 @@ impl Spec {
  match self {
  Self::Char { width, align_left } => {
  let width = resolve_asterisk(*width, &mut args)?.unwrap_or(0);
- write_padded(writer, &[args.get_char()], width, *align_left)
+ write_padded(writer, &[args.get_char()?], width, *align_left)
  }
  Self::String {
  width,
@@ -334,7 +333,7 @@ impl Spec {
 
  let os_str = args.get_str();
 
- let bytes = bytes_from_os_str(os_str).unwrap();
+ let bytes = try_get_bytes_from_os_str(os_str).unwrap();
 
  let truncated = match precision {
  Some(p) if p < os_str.len() => &bytes[..p],
@@ -346,7 +345,7 @@ impl Spec {
  Self::EscapedString => {
  let os_str = args.get_str();
 
- let bytes = bytes_from_os_str(os_str).unwrap();
+ let bytes = try_get_bytes_from_os_str(os_str).unwrap();
 
  let mut parsed = Vec::<u8>::new();
 
@@ -386,7 +385,7 @@ impl Spec {
  } => {
  let width = resolve_asterisk(*width, &mut args)?.unwrap_or(0);
  let precision = resolve_asterisk(*precision, &mut args)?.unwrap_or(0);
- let i = args.get_i64();
+ let i = args.get_i64()?;
 
  if precision as u64 > i32::MAX as u64 {
  return Err(FormatError::InvalidPrecision(precision.to_string()));
@@ -409,7 +408,7 @@ impl Spec {
  } => {
  let width = resolve_asterisk(*width, &mut args)?.unwrap_or(0);
  let precision = resolve_asterisk(*precision, &mut args)?.unwrap_or(0);
- let i = args.get_u64();
+ let i = args.get_u64()?;
 
  if precision as u64 > i32::MAX as u64 {
  return Err(FormatError::InvalidPrecision(precision.to_string()));
@@ -435,7 +434,7 @@ impl Spec {
  } => {
  let width = resolve_asterisk(*width, &mut args)?.unwrap_or(0);
  let precision = resolve_asterisk(*precision, &mut args)?.unwrap_or(6);
- let f = args.get_f64();
+ let f = args.get_f64()?;
 
  if precision as u64 > i32::MAX as u64 {
  return Err(FormatError::InvalidPrecision(precision.to_string()));
@@ -463,7 +462,7 @@ fn resolve_asterisk<'a>(
 ) -> Result<Option<usize>, FormatError> {
  Ok(match option {
  None => None,
- Some(CanAsterisk::Asterisk) => Some(usize::try_from(args.get_u64()).ok().unwrap_or(0)),
+ Some(CanAsterisk::Asterisk) => Some(usize::try_from(args.get_u64()?).ok().unwrap_or(0)),
  Some(CanAsterisk::Fixed(w)) => Some(w),
  })
 }

tests/by-util/test_printf.rs

@@ -941,4 +941,11 @@ fn non_utf_8_input() {
  .arg(os_str)
  .succeeds()
  .stdout_only_bytes(INPUT_AND_OUTPUT);
+
+ new_ucmd!()
+ .arg("%d")
+ .arg(os_str)
+ .fails()
+ .stderr_only("printf: invalid (non-UTF-8) argument like 'Swer an rehte g�ete wendet s�n gem�ete, dem volget s�lde und �re.' encountered
+");
 }