Restructure non-UTF-8 input error handling

Author: andrewliebenow

src/uu/echo/src/echo.rs

@@ -10,7 +10,7 @@ use std::ffi::OsString;
 use std::io::{self, StdoutLock, Write};
 use uucore::error::UResult;
 use uucore::format::{EscapedChar, FormatChar, OctalParsing, parse_escape_only};
-use uucore::{format_usage, help_about, help_section, help_usage};
+use uucore::{format_usage, help_about, help_section, help_usage, os_str_as_bytes_verbose};
 
 const ABOUT: &str = help_about!("echo.md");
 const USAGE: &str = help_usage!("echo.md");
@@ -137,8 +137,9 @@ fn execute(
     escaped: bool,
 ) -> UResult<()> {
     for (i, input) in arguments_after_options.into_iter().enumerate() {
-        let bytes = uucore::format::try_get_bytes_from_os_str(&input)?;
+        let bytes = os_str_as_bytes_verbose(&input)?;
 
+        // Don't print a space before the first argument
         if i > 0 {
             stdout_lock.write_all(b" ")?;
         }

src/uu/printf/src/printf.rs

@@ -8,9 +8,11 @@ use std::io::stdout;
 use std::ops::ControlFlow;
 use uucore::error::{UResult, UUsageError};
 use uucore::format::{
-    FormatArgument, FormatArguments, FormatItem, parse_spec_and_escape, try_get_bytes_from_os_str,
+    FormatArgument, FormatArguments, FormatError, FormatItem, parse_spec_and_escape,
+};
+use uucore::{
+    format_usage, help_about, help_section, help_usage, os_str_as_bytes_verbose, show_warning,
 };
-use uucore::{format_usage, help_about, help_section, help_usage, show_warning};
 
 const VERSION: &str = "version";
 const HELP: &str = "help";
@@ -30,7 +32,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let format = matches
         .get_one::<OsString>(options::FORMAT)
         .ok_or_else(|| UUsageError::new(1, "missing operand"))?;
-    let format = try_get_bytes_from_os_str(format)?;
+    let format = os_str_as_bytes_verbose(format).map_err(FormatError::from)?;
 
     let values: Vec<_> = match matches.get_many::<OsString>(options::ARGUMENT) {
         Some(s) => s

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

@@ -6,16 +6,15 @@
 use super::{ExtendedBigDecimal, FormatError};
 use crate::format::spec::ArgumentLocation;
 use crate::{
-    error::{UError, set_exit_code},
+    error::set_exit_code,
+    os_str_as_bytes_verbose, os_str_as_str_verbose,
     parser::num_parser::{ExtendedParser, ExtendedParserError},
     quoting_style::{Quotes, QuotingStyle, escape_name},
     show_error, show_warning,
 };
 use os_display::Quotable;
 use std::{
-    error::Error,
     ffi::{OsStr, OsString},
-    fmt::Display,
     num::NonZero,
 };
 
@@ -77,7 +76,7 @@ impl<'a> FormatArguments<'a> {
     pub fn next_char(&mut self, position: &ArgumentLocation) -> Result<u8, FormatError> {
         match self.next_arg(position) {
             Some(FormatArgument::Char(c)) => Ok(*c as u8),
-            Some(FormatArgument::Unparsed(os)) => match try_get_bytes_from_os_str(os)?.first() {
+            Some(FormatArgument::Unparsed(os)) => match os_str_as_bytes_verbose(os)?.first() {
                 Some(&byte) => Ok(byte),
                 None => Ok(b'\0'),
             },
@@ -96,7 +95,7 @@ impl<'a> FormatArguments<'a> {
         match self.next_arg(position) {
             Some(FormatArgument::SignedInt(n)) => Ok(*n),
             Some(FormatArgument::Unparsed(os)) => {
-                let str = try_get_str_from_os_str(os)?;
+                let str = os_str_as_str_verbose(os)?;
 
                 Ok(extract_value(i64::extended_parse(str), str))
             }
@@ -108,7 +107,7 @@ impl<'a> FormatArguments<'a> {
         match self.next_arg(position) {
             Some(FormatArgument::UnsignedInt(n)) => Ok(*n),
             Some(FormatArgument::Unparsed(os)) => {
-                let s = try_get_str_from_os_str(os)?;
+                let s = os_str_as_str_verbose(os)?;
                 // Check if the string is a character literal enclosed in quotes
                 if s.starts_with(['"', '\'']) {
                     // Extract the content between the quotes safely using chars
@@ -139,7 +138,7 @@ impl<'a> FormatArguments<'a> {
         match self.next_arg(position) {
             Some(FormatArgument::Float(n)) => Ok(n.clone()),
             Some(FormatArgument::Unparsed(os)) => {
-                let s = try_get_str_from_os_str(os)?;
+                let s = os_str_as_str_verbose(os)?;
 
                 Ok(extract_value(ExtendedBigDecimal::extended_parse(s), s))
             }
@@ -207,59 +206,6 @@ fn extract_value<T: Default>(p: Result<T, ExtendedParserError<'_, T>>, input: &s
     }
 }
 
-#[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")]
-        {
-            use std::os::unix::ffi::OsStrExt;
-
-            Ok(input.as_bytes())
-        }
-
-        #[cfg(not(target_family = "unix"))]
-        {
-            // TODO
-            // Verify that this works correctly on these platforms
-            match input.to_str().map(|st| st.as_bytes()) {
-                Some(sl) => Ok(sl),
-                None => {
-                    let cow = input.to_string_lossy();
-
-                    Err(NonUtf8OsStr(cow.to_string()))
-                }
-            }
-        }
-    };
-
-    result
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

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

@@ -37,8 +37,12 @@ pub mod human;
 pub mod num_format;
 mod spec;
 
+pub use self::escape::{EscapedChar, OctalParsing};
 use crate::extendedbigdecimal::ExtendedBigDecimal;
-pub use argument::*;
+pub use argument::{FormatArgument, FormatArguments};
+
+use self::{escape::parse_escape_code, num_format::Formatter};
+use crate::{NonUtf8OsStrError, OsStrConversionType, error::UError};
 pub use spec::Spec;
 use std::{
     error::Error,
@@ -50,13 +54,6 @@ use std::{
 
 use os_display::Quotable;
 
-use crate::error::UError;
-
-pub use self::{
-    escape::{EscapedChar, OctalParsing, parse_escape_code},
-    num_format::Formatter,
-};
-
 #[derive(Debug)]
 pub enum FormatError {
     SpecError(Vec<u8>),
@@ -74,7 +71,7 @@ pub enum FormatError {
     /// The hexadecimal characters represent a code point that cannot represent a
     /// Unicode character (e.g., a surrogate code point)
     InvalidCharacter(char, Vec<u8>),
-    InvalidEncoding(NonUtf8OsStr),
+    InvalidEncoding(NonUtf8OsStrError),
 }
 
 impl Error for FormatError {}
@@ -86,8 +83,8 @@ impl From<std::io::Error> for FormatError {
     }
 }
 
-impl From<NonUtf8OsStr> for FormatError {
-    fn from(value: NonUtf8OsStr) -> FormatError {
+impl From<NonUtf8OsStrError> for FormatError {
+    fn from(value: NonUtf8OsStrError) -> FormatError {
         FormatError::InvalidEncoding(value)
     }
 }
@@ -127,11 +124,18 @@ impl Display for FormatError {
                 String::from_utf8_lossy(digits)
             ),
             Self::InvalidEncoding(no) => {
-                write!(
-                    f,
-                    "invalid (non-UTF-8) argument like {} encountered",
-                    no.0.quote()
-                )
+                use os_display::Quotable;
+
+                let quoted = no.input_lossy_string.quote();
+
+                match no.conversion_type {
+                    OsStrConversionType::ToBytes => f.write_fmt(format_args!(
+                        "invalid (non-UTF-8) argument like {quoted} encountered when converting argument to bytes on a platform that doesn't use UTF-8",
+                    )),
+                    OsStrConversionType::ToString => f.write_fmt(format_args!(
+                        "invalid (non-UTF-8) argument like {quoted} encountered",
+                    )),
+                }
             }
         }
     }

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

@@ -11,10 +11,11 @@ use super::{
         self, Case, FloatVariant, ForceDecimal, Formatter, NumberAlignment, PositiveSign, Prefix,
         UnsignedIntVariant,
     },
-    parse_escape_only, try_get_bytes_from_os_str,
+    parse_escape_only,
 };
 use crate::{
     format::FormatArguments,
+    os_str_as_bytes_verbose,
     quoting_style::{QuotingStyle, escape_name},
 };
 use std::{io::Write, num::NonZero, ops::ControlFlow};
@@ -379,7 +380,7 @@ impl Spec {
 
                 let os_str = args.next_string(position);
 
-                let bytes = try_get_bytes_from_os_str(os_str).unwrap();
+                let bytes = os_str_as_bytes_verbose(os_str)?;
 
                 let truncated = match precision {
                     Some(p) if p < os_str.len() => &bytes[..p],
@@ -390,7 +391,7 @@ impl Spec {
             Self::EscapedString { position } => {
                 let os_str = args.next_string(position);
 
-                let bytes = try_get_bytes_from_os_str(os_str).unwrap();
+                let bytes = os_str_as_bytes_verbose(os_str)?;
 
                 let mut parsed = Vec::<u8>::new();
 
@@ -414,7 +415,7 @@ impl Spec {
                         show_control: false,
                     },
                 );
-                let bytes = try_get_bytes_from_os_str(&s)?;
+                let bytes = os_str_as_bytes_verbose(&s)?;
                 writer.write_all(bytes).map_err(FormatError::IoError)
             }
             Self::SignedInt {