Merge pull request #8329 from drinkcat/printf-7209-update

printf: accept non-UTF-8 input in FORMAT and ARGUMENT arguments

src/uu/echo/src/echo.rs

@@ -8,10 +8,9 @@ use clap::{Arg, ArgAction, Command};
 use std::env;
 use std::ffi::{OsStr, OsString};
 use std::io::{self, StdoutLock, Write};
-use uucore::error::{UResult, USimpleError};
+use uucore::error::UResult;
 use uucore::format::{FormatChar, OctalParsing, parse_escape_only};
-use uucore::format_usage;
+use uucore::{format_usage, os_str_as_bytes};
-use uucore::os_str_as_bytes;
 
 use uucore::locale::get_message;
 
@@ -223,9 +222,9 @@ pub fn uu_app() -> Command {
 
 fn execute(stdout: &mut StdoutLock, args: Vec<OsString>, options: Options) -> UResult<()> {
     for (i, arg) in args.into_iter().enumerate() {
-        let bytes = os_str_as_bytes(arg.as_os_str())
+        let bytes = os_str_as_bytes(&arg)?;
-            .map_err(|_| USimpleError::new(1, get_message("echo-error-non-utf8")))?;
 
+        // Don't print a space before the first argument
         if i > 0 {
             stdout.write_all(b" ")?;
         }

src/uu/printf/src/printf.rs

@@ -4,6 +4,7 @@
 // file that was distributed with this source code.
 use clap::{Arg, ArgAction, Command};
 use std::collections::HashMap;
+use std::ffi::OsString;
 use std::io::stdout;
 use std::ops::ControlFlow;
 use uucore::error::{UResult, UUsageError};
@@ -18,21 +19,19 @@ mod options {
     pub const FORMAT: &str = "FORMAT";
     pub const ARGUMENT: &str = "ARGUMENT";
 }
+
 #[uucore::main]
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let matches = uu_app().get_matches_from(args);
 
     let format = matches
-        .get_one::<std::ffi::OsString>(options::FORMAT)
+        .get_one::<OsString>(options::FORMAT)
         .ok_or_else(|| UUsageError::new(1, get_message("printf-error-missing-operand")))?;
     let format = os_str_as_bytes(format)?;
 
-    let values: Vec<_> = match matches.get_many::<std::ffi::OsString>(options::ARGUMENT) {
+    let values: Vec<_> = match matches.get_many::<OsString>(options::ARGUMENT) {
-        // FIXME: use os_str_as_bytes once FormatArgument supports Vec<u8>
         Some(s) => s
-            .map(|os_string| {
+            .map(|os_string| FormatArgument::Unparsed(os_string.to_owned()))
-                FormatArgument::Unparsed(std::ffi::OsStr::to_string_lossy(os_string).to_string())
-            })
             .collect(),
         None => vec![],
     };
@@ -62,7 +61,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
                 "{}",
                 get_message_with_args(
                     "printf-warning-ignoring-excess-arguments",
-                    HashMap::from([("arg".to_string(), arg_str.to_string())])
+                    HashMap::from([("arg".to_string(), arg_str.to_string_lossy().to_string())])
                 )
             );
         }
@@ -103,10 +102,10 @@ pub fn uu_app() -> Command {
                 .help(get_message("printf-help-version"))
                 .action(ArgAction::Version),
         )
-        .arg(Arg::new(options::FORMAT).value_parser(clap::value_parser!(std::ffi::OsString)))
+        .arg(Arg::new(options::FORMAT).value_parser(clap::value_parser!(OsString)))
         .arg(
             Arg::new(options::ARGUMENT)
                 .action(ArgAction::Append)
-                .value_parser(clap::value_parser!(std::ffi::OsString)),
+                .value_parser(clap::value_parser!(OsString)),
         )
 }

src/uucore/src/lib/features/checksum.rs

@@ -968,7 +968,7 @@ fn process_checksum_line(
     cached_line_format: &mut Option<LineFormat>,
     last_algo: &mut Option<String>,
 ) -> Result<(), LineCheckError> {
-    let line_bytes = os_str_as_bytes(line)?;
+    let line_bytes = os_str_as_bytes(line).map_err(|e| LineCheckError::UError(Box::new(e)))?;
 
     // Early return on empty or commented lines.
     if line.is_empty() || line_bytes.starts_with(b"#") {

src/uucore/src/lib/features/extendedbigdecimal.rs

@@ -101,6 +101,18 @@ impl From<f64> for ExtendedBigDecimal {
     }
 }
 
+impl From<u8> for ExtendedBigDecimal {
+    fn from(val: u8) -> Self {
+        Self::BigDecimal(val.into())
+    }
+}
+
+impl From<u32> for ExtendedBigDecimal {
+    fn from(val: u32) -> Self {
+        Self::BigDecimal(val.into())
+    }
+}
+
 impl ExtendedBigDecimal {
     pub fn zero() -> Self {
         Self::BigDecimal(0.into())

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

@@ -7,12 +7,16 @@ use super::ExtendedBigDecimal;
 use crate::format::spec::ArgumentLocation;
 use crate::{
     error::set_exit_code,
+    os_str_as_bytes,
     parser::num_parser::{ExtendedParser, ExtendedParserError},
     quoting_style::{QuotingStyle, locale_aware_escape_name},
     show_error, show_warning,
 };
 use os_display::Quotable;
-use std::{ffi::OsStr, num::NonZero};
+use std::{
+    ffi::{OsStr, OsString},
+    num::NonZero,
+};
 
 /// An argument for formatting
 ///
@@ -24,12 +28,12 @@ use std::{ffi::OsStr, num::NonZero};
 #[derive(Clone, Debug, PartialEq)]
 pub enum FormatArgument {
     Char(char),
-    String(String),
+    String(OsString),
     UnsignedInt(u64),
     SignedInt(i64),
     Float(ExtendedBigDecimal),
     /// Special argument that gets coerced into the other variants
-    Unparsed(String),
+    Unparsed(OsString),
 }
 
 /// A struct that holds a slice of format arguments and provides methods to access them
@@ -72,22 +76,25 @@ impl<'a> FormatArguments<'a> {
     pub fn next_char(&mut self, position: &ArgumentLocation) -> u8 {
         match self.next_arg(position) {
             Some(FormatArgument::Char(c)) => *c as u8,
-            Some(FormatArgument::Unparsed(s)) => s.bytes().next().unwrap_or(b'\0'),
+            Some(FormatArgument::Unparsed(os)) => match os_str_as_bytes(os) {
+                Ok(bytes) => bytes.first().copied().unwrap_or(b'\0'),
+                Err(_) => b'\0',
+            },
             _ => b'\0',
         }
     }
 
-    pub fn next_string(&mut self, position: &ArgumentLocation) -> &'a str {
+    pub fn next_string(&mut self, position: &ArgumentLocation) -> &'a OsStr {
         match self.next_arg(position) {
-            Some(FormatArgument::Unparsed(s) | FormatArgument::String(s)) => s,
+            Some(FormatArgument::Unparsed(os) | FormatArgument::String(os)) => os,
-            _ => "",
+            _ => "".as_ref(),
         }
     }
 
     pub fn next_i64(&mut self, position: &ArgumentLocation) -> i64 {
         match self.next_arg(position) {
             Some(FormatArgument::SignedInt(n)) => *n,
-            Some(FormatArgument::Unparsed(s)) => extract_value(i64::extended_parse(s), s),
+            Some(FormatArgument::Unparsed(os)) => Self::get_num::<i64>(os),
             _ => 0,
         }
     }
@@ -95,25 +102,7 @@ impl<'a> FormatArguments<'a> {
     pub fn next_u64(&mut self, position: &ArgumentLocation) -> u64 {
         match self.next_arg(position) {
             Some(FormatArgument::UnsignedInt(n)) => *n,
-            Some(FormatArgument::Unparsed(s)) => {
+            Some(FormatArgument::Unparsed(os)) => Self::get_num::<u64>(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
-                    let mut chars = s.trim_matches(|c| c == '"' || c == '\'').chars();
-                    if let Some(first_char) = chars.next() {
-                        if chars.clone().count() > 0 {
-                            // Emit a warning if there are additional characters
-                            let remaining: String = chars.collect();
-                            show_warning!(
-                                "{remaining}: character(s) following character constant have been ignored"
-                            );
-                        }
-                        return first_char as u64; // Use only the first character
-                    }
-                    return 0; // Empty quotes
-                }
-                extract_value(u64::extended_parse(s), s)
-            }
             _ => 0,
         }
     }
@@ -121,13 +110,81 @@ impl<'a> FormatArguments<'a> {
     pub fn next_extended_big_decimal(&mut self, position: &ArgumentLocation) -> ExtendedBigDecimal {
         match self.next_arg(position) {
             Some(FormatArgument::Float(n)) => n.clone(),
-            Some(FormatArgument::Unparsed(s)) => {
+            Some(FormatArgument::Unparsed(os)) => Self::get_num::<ExtendedBigDecimal>(os),
-                extract_value(ExtendedBigDecimal::extended_parse(s), s)
-            }
             _ => ExtendedBigDecimal::zero(),
         }
     }
 
+    // Parse an OsStr that we know to start with a '/"
+    fn parse_quote_start<T>(os: &OsStr) -> Result<T, ExtendedParserError<T>>
+    where
+        T: ExtendedParser + From<u8> + From<u32> + Default,
+    {
+        // If this fails (this can only happens on Windows), then just
+        // return NotNumeric.
+        let s = match os_str_as_bytes(os) {
+            Ok(s) => s,
+            Err(_) => return Err(ExtendedParserError::NotNumeric),
+        };
+
+        let bytes = match s.split_first() {
+            Some((b'"', bytes)) | Some((b'\'', bytes)) => bytes,
+            _ => {
+                // This really can't happen, the string we are given must start with '/".
+                debug_assert!(false);
+                return Err(ExtendedParserError::NotNumeric);
+            }
+        };
+
+        if bytes.is_empty() {
+            return Err(ExtendedParserError::NotNumeric);
+        }
+
+        let (val, len) = if let Some(c) = bytes
+            .utf8_chunks()
+            .next()
+            .expect("bytes should not be empty")
+            .valid()
+            .chars()
+            .next()
+        {
+            // Valid UTF-8 character, cast the codepoint to u32 then T
+            // (largest unicode codepoint is only 3 bytes, so this is safe)
+            ((c as u32).into(), c.len_utf8())
+        } else {
+            // Not a valid UTF-8 character, use the first byte
+            (bytes[0].into(), 1)
+        };
+        // Emit a warning if there are additional characters
+        if bytes.len() > len {
+            return Err(ExtendedParserError::PartialMatch(
+                val,
+                String::from_utf8_lossy(&bytes[len..]).into_owned(),
+            ));
+        }
+
+        Ok(val)
+    }
+
+    fn get_num<T>(os: &OsStr) -> T
+    where
+        T: ExtendedParser + From<u8> + From<u32> + Default,
+    {
+        let s = os.to_string_lossy();
+        let first = s.as_bytes().first().copied();
+
+        let quote_start = first == Some(b'"') || first == Some(b'\'');
+        let parsed = if quote_start {
+            // The string begins with a quote
+            Self::parse_quote_start(os)
+        } else {
+            T::extended_parse(&s)
+        };
+
+        // Get the best possible value, even if parsed was an error.
+        extract_value(parsed, &s, quote_start)
+    }
+
     fn get_at_relative_position(&mut self, pos: NonZero<usize>) -> Option<&'a FormatArgument> {
         let pos: usize = pos.into();
         let pos = (pos - 1).saturating_add(self.current_offset);
@@ -147,7 +204,11 @@ impl<'a> FormatArguments<'a> {
     }
 }
 
-fn extract_value<T: Default>(p: Result<T, ExtendedParserError<'_, T>>, input: &str) -> T {
+fn extract_value<T: Default>(
+    p: Result<T, ExtendedParserError<T>>,
+    input: &str,
+    quote_start: bool,
+) -> T {
     match p {
         Ok(v) => v,
         Err(e) => {
@@ -167,14 +228,15 @@ fn extract_value<T: Default>(p: Result<T, ExtendedParserError<'_, T>>, input: &s
                     Default::default()
                 }
                 ExtendedParserError::PartialMatch(v, rest) => {
-                    let bytes = input.as_encoded_bytes();
+                    if quote_start {
-                    if !bytes.is_empty() && (bytes[0] == b'\'' || bytes[0] == b'"') {
+                        set_exit_code(0);
                         show_warning!(
                             "{rest}: character(s) following character constant have been ignored"
                         );
                     } else {
                         show_error!("{}: value not completely converted", input.quote());
                     }
+
                     v
                 }
             }
@@ -249,11 +311,11 @@ mod tests {
         // Test with different method types in sequence
         let args = [
             FormatArgument::Char('a'),
-            FormatArgument::String("hello".to_string()),
+            FormatArgument::String("hello".into()),
-            FormatArgument::Unparsed("123".to_string()),
+            FormatArgument::Unparsed("123".into()),
-            FormatArgument::String("world".to_string()),
+            FormatArgument::String("world".into()),
             FormatArgument::Char('z'),
-            FormatArgument::String("test".to_string()),
+            FormatArgument::String("test".into()),
         ];
         let mut args = FormatArguments::new(&args);
 
@@ -384,10 +446,10 @@ mod tests {
     fn test_unparsed_arguments() {
         // Test with unparsed arguments that get coerced
         let args = [
-            FormatArgument::Unparsed("hello".to_string()),
+            FormatArgument::Unparsed("hello".into()),
-            FormatArgument::Unparsed("123".to_string()),
+            FormatArgument::Unparsed("123".into()),
-            FormatArgument::Unparsed("hello".to_string()),
+            FormatArgument::Unparsed("hello".into()),
-            FormatArgument::Unparsed("456".to_string()),
+            FormatArgument::Unparsed("456".into()),
         ];
         let mut args = FormatArguments::new(&args);
 
@@ -409,10 +471,10 @@ mod tests {
         // Test with mixed types and positional access
         let args = [
             FormatArgument::Char('a'),
-            FormatArgument::String("test".to_string()),
+            FormatArgument::String("test".into()),
             FormatArgument::UnsignedInt(42),
             FormatArgument::Char('b'),
-            FormatArgument::String("more".to_string()),
+            FormatArgument::String("more".into()),
             FormatArgument::UnsignedInt(99),
         ];
         let mut args = FormatArguments::new(&args);

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, 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,6 +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(NonUtf8OsStrError),
 }
 
 impl Error for FormatError {}
@@ -85,6 +83,12 @@ impl From<std::io::Error> for FormatError {
     }
 }
 
+impl From<NonUtf8OsStrError> for FormatError {
+    fn from(value: NonUtf8OsStrError) -> FormatError {
+        FormatError::InvalidEncoding(value)
+    }
+}
+
 impl Display for FormatError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
@@ -118,6 +122,7 @@ impl Display for FormatError {
                 "invalid universal character name \\{escape_char}{}",
                 String::from_utf8_lossy(digits)
             ),
+            Self::InvalidEncoding(no) => no.fmt(f),
         }
     }
 }

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

@@ -5,8 +5,6 @@
 
 // spell-checker:ignore (vars) intmax ptrdiff padlen
 
-use crate::quoting_style::{QuotingStyle, locale_aware_escape_name};
-
 use super::{
     ExtendedBigDecimal, FormatChar, FormatError, OctalParsing,
     num_format::{
@@ -15,7 +13,11 @@ use super::{
     },
     parse_escape_only,
 };
-use crate::format::FormatArguments;
+use crate::{
+    format::FormatArguments,
+    os_str_as_bytes,
+    quoting_style::{QuotingStyle, locale_aware_escape_name},
+};
 use std::{io::Write, num::NonZero, ops::ControlFlow};
 
 /// A parsed specification for formatting a value
@@ -375,22 +377,21 @@ impl Spec {
                 // TODO: We need to not use Rust's formatting for aligning the output,
                 // so that we can just write bytes to stdout without panicking.
                 let precision = resolve_asterisk_precision(*precision, args);
-                let s = args.next_string(position);
+                let os_str = args.next_string(position);
+                let bytes = os_str_as_bytes(os_str)?;
+
                 let truncated = match precision {
-                    Some(p) if p < s.len() => &s[..p],
+                    Some(p) if p < os_str.len() => &bytes[..p],
-                    _ => s,
+                    _ => bytes,
                 };
-                write_padded(
+                write_padded(writer, truncated, width, *align_left || neg_width)
-                    writer,
-                    truncated.as_bytes(),
-                    width,
-                    *align_left || neg_width,
-                )
             }
             Self::EscapedString { position } => {
-                let s = args.next_string(position);
+                let os_str = args.next_string(position);
-                let mut parsed = Vec::new();
+                let bytes = os_str_as_bytes(os_str)?;
-                for c in parse_escape_only(s.as_bytes(), OctalParsing::ThreeDigits) {
+                let mut parsed = Vec::<u8>::new();
+
+                for c in parse_escape_only(bytes, OctalParsing::ThreeDigits) {
                     match c.write(&mut parsed)? {
                         ControlFlow::Continue(()) => {}
                         ControlFlow::Break(()) => {
@@ -403,15 +404,11 @@ impl Spec {
             }
             Self::QuotedString { position } => {
                 let s = locale_aware_escape_name(
-                    args.next_string(position).as_ref(),
+                    args.next_string(position),
                     QuotingStyle::SHELL_ESCAPE,
                 );
-                #[cfg(unix)]
+                let bytes = os_str_as_bytes(&s)?;
-                let bytes = std::os::unix::ffi::OsStringExt::into_vec(s);
+                writer.write_all(bytes).map_err(FormatError::IoError)
-                #[cfg(not(unix))]
-                let bytes = s.to_string_lossy().as_bytes().to_owned();
-
-                writer.write_all(&bytes).map_err(FormatError::IoError)
             }
             Self::SignedInt {
                 width,
@@ -646,7 +643,7 @@ mod tests {
                 Some((42, false)),
                 resolve_asterisk_width(
                     Some(CanAsterisk::Asterisk(ArgumentLocation::NextArgument)),
-                    &mut FormatArguments::new(&[FormatArgument::Unparsed("42".to_string())]),
+                    &mut FormatArguments::new(&[FormatArgument::Unparsed("42".into())]),
                 )
             );
 
@@ -661,7 +658,7 @@ mod tests {
                 Some((42, true)),
                 resolve_asterisk_width(
                     Some(CanAsterisk::Asterisk(ArgumentLocation::NextArgument)),
-                    &mut FormatArguments::new(&[FormatArgument::Unparsed("-42".to_string())]),
+                    &mut FormatArguments::new(&[FormatArgument::Unparsed("-42".into())]),
                 )
             );
 
@@ -672,9 +669,9 @@ mod tests {
                         NonZero::new(2).unwrap()
                     ))),
                     &mut FormatArguments::new(&[
-                        FormatArgument::Unparsed("1".to_string()),
+                        FormatArgument::Unparsed("1".into()),
-                        FormatArgument::Unparsed("2".to_string()),
+                        FormatArgument::Unparsed("2".into()),
-                        FormatArgument::Unparsed("3".to_string())
+                        FormatArgument::Unparsed("3".into())
                     ]),
                 )
             );
@@ -717,7 +714,7 @@ mod tests {
                 Some(42),
                 resolve_asterisk_precision(
                     Some(CanAsterisk::Asterisk(ArgumentLocation::NextArgument)),
-                    &mut FormatArguments::new(&[FormatArgument::Unparsed("42".to_string())]),
+                    &mut FormatArguments::new(&[FormatArgument::Unparsed("42".into())]),
                 )
             );
 
@@ -732,7 +729,7 @@ mod tests {
                 Some(0),
                 resolve_asterisk_precision(
                     Some(CanAsterisk::Asterisk(ArgumentLocation::NextArgument)),
-                    &mut FormatArguments::new(&[FormatArgument::Unparsed("-42".to_string())]),
+                    &mut FormatArguments::new(&[FormatArgument::Unparsed("-42".into())]),
                 )
             );
             assert_eq!(
@@ -742,9 +739,9 @@ mod tests {
                         NonZero::new(2).unwrap()
                     ))),
                     &mut FormatArguments::new(&[
-                        FormatArgument::Unparsed("1".to_string()),
+                        FormatArgument::Unparsed("1".into()),
-                        FormatArgument::Unparsed("2".to_string()),
+                        FormatArgument::Unparsed("2".into()),
-                        FormatArgument::Unparsed("3".to_string())
+                        FormatArgument::Unparsed("3".into())
                     ]),
                 )
             );

src/uucore/src/lib/lib.rs

@@ -311,23 +311,39 @@ pub fn read_yes() -> bool {
     }
 }
 
+#[derive(Debug)]
+pub struct NonUtf8OsStrError {
+    input_lossy_string: String,
+}
+
+impl std::fmt::Display for NonUtf8OsStrError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        use os_display::Quotable;
+        let quoted = self.input_lossy_string.quote();
+        f.write_fmt(format_args!(
+            "invalid UTF-8 input {quoted} encountered when converting to bytes on a platform that doesn't expose byte arguments",
+        ))
+    }
+}
+
+impl std::error::Error for NonUtf8OsStrError {}
+impl error::UError for NonUtf8OsStrError {}
+
 /// Converts an `OsStr` to a UTF-8 `&[u8]`.
 ///
 /// This always succeeds on unix platforms,
 /// and fails on other platforms if the string can't be coerced to UTF-8.
-pub fn os_str_as_bytes(os_string: &OsStr) -> mods::error::UResult<&[u8]> {
+pub fn os_str_as_bytes(os_string: &OsStr) -> Result<&[u8], NonUtf8OsStrError> {
     #[cfg(unix)]
-    let bytes = os_string.as_bytes();
+    return Ok(os_string.as_bytes());
 
     #[cfg(not(unix))]
-    let bytes = os_string
+    os_string
         .to_str()
-        .ok_or_else(|| {
+        .ok_or_else(|| NonUtf8OsStrError {
-            mods::error::UUsageError::new(1, "invalid UTF-8 was detected in one or more arguments")
+            input_lossy_string: os_string.to_string_lossy().into_owned(),
-        })?
+        })
-        .as_bytes();
+        .map(|s| s.as_bytes())
-
-    Ok(bytes)
 }
 
 /// Performs a potentially lossy conversion from `OsStr` to UTF-8 bytes.
@@ -336,15 +352,13 @@ pub fn os_str_as_bytes(os_string: &OsStr) -> mods::error::UResult<&[u8]> {
 /// and wraps [`OsStr::to_string_lossy`] on non-unix platforms.
 pub fn os_str_as_bytes_lossy(os_string: &OsStr) -> Cow<[u8]> {
     #[cfg(unix)]
-    let bytes = Cow::from(os_string.as_bytes());
+    return Cow::from(os_string.as_bytes());
 
     #[cfg(not(unix))]
-    let bytes = match os_string.to_string_lossy() {
+    match os_string.to_string_lossy() {
         Cow::Borrowed(slice) => Cow::from(slice.as_bytes()),
         Cow::Owned(owned) => Cow::from(owned.into_bytes()),
-    };
+    }
-
-    bytes
 }
 
 /// Converts a `&[u8]` to an `&OsStr`,
@@ -354,13 +368,12 @@ pub fn os_str_as_bytes_lossy(os_string: &OsStr) -> Cow<[u8]> {
 /// and fails on other platforms if the bytes can't be parsed as UTF-8.
 pub fn os_str_from_bytes(bytes: &[u8]) -> mods::error::UResult<Cow<'_, OsStr>> {
     #[cfg(unix)]
-    let os_str = Cow::Borrowed(OsStr::from_bytes(bytes));
+    return Ok(Cow::Borrowed(OsStr::from_bytes(bytes)));
-    #[cfg(not(unix))]
-    let os_str = Cow::Owned(OsString::from(str::from_utf8(bytes).map_err(|_| {
-        mods::error::UUsageError::new(1, "Unable to transform bytes into OsStr")
-    })?));
 
-    Ok(os_str)
+    #[cfg(not(unix))]
+    Ok(Cow::Owned(OsString::from(str::from_utf8(bytes).map_err(
+        |_| mods::error::UUsageError::new(1, "Unable to transform bytes into OsStr"),
+    )?)))
 }
 
 /// Converts a `Vec<u8>` into an `OsString`, parsing as UTF-8 on non-unix platforms.
@@ -369,13 +382,12 @@ pub fn os_str_from_bytes(bytes: &[u8]) -> mods::error::UResult<Cow<'_, OsStr>> {
 /// and fails on other platforms if the bytes can't be parsed as UTF-8.
 pub fn os_string_from_vec(vec: Vec<u8>) -> mods::error::UResult<OsString> {
     #[cfg(unix)]
-    let s = OsString::from_vec(vec);
+    return Ok(OsString::from_vec(vec));
-    #[cfg(not(unix))]
-    let s = OsString::from(String::from_utf8(vec).map_err(|_| {
-        mods::error::UUsageError::new(1, "invalid UTF-8 was detected in one or more arguments")
-    })?);
 
-    Ok(s)
+    #[cfg(not(unix))]
+    Ok(OsString::from(String::from_utf8(vec).map_err(|_| {
+        mods::error::UUsageError::new(1, "invalid UTF-8 was detected in one or more arguments")
+    })?))
 }
 
 /// Converts an `OsString` into a `Vec<u8>`, parsing as UTF-8 on non-unix platforms.

tests/by-util/test_printf.rs

@@ -805,7 +805,7 @@ fn test_overflow() {
 fn partial_char() {
     new_ucmd!()
         .args(&["%d", "'abc"])
-        .fails_with_code(1)
+        .succeeds()
         .stdout_is("97")
         .stderr_is(
             "printf: warning: bc: character(s) following character constant have been ignored\n",
@@ -1293,23 +1293,80 @@ fn float_arg_with_whitespace() {
 
 #[test]
 fn mb_input() {
-    for format in ["\"á", "\'á", "'\u{e1}"] {
+    let cases = vec![
+        ("%04x\n", "\"á", "00e1\n"),
+        ("%04x\n", "'á", "00e1\n"),
+        ("%04x\n", "'\u{e1}", "00e1\n"),
+        ("%i\n", "\"á", "225\n"),
+        ("%i\n", "'á", "225\n"),
+        ("%i\n", "'\u{e1}", "225\n"),
+        ("%f\n", "'á", "225.000000\n"),
+    ];
+    for (format, arg, stdout) in cases {
         new_ucmd!()
-            .args(&["%04x\n", format])
+            .args(&[format, arg])
             .succeeds()
-            .stdout_only("00e1\n");
+            .stdout_only(stdout);
     }
 
     let cases = vec![
-        ("\"á=", "="),
+        ("%04x\n", "\"á=", "00e1\n", "="),
-        ("\'á-", "-"),
+        ("%04x\n", "'á-", "00e1\n", "-"),
-        ("\'á=-==", "=-=="),
+        ("%04x\n", "'á=-==", "00e1\n", "=-=="),
-        ("'\u{e1}++", "++"),
+        ("%04x\n", "'á'", "00e1\n", "'"),
+        ("%04x\n", "'\u{e1}++", "00e1\n", "++"),
+        ("%04x\n", "''á'", "0027\n", "á'"),
+        ("%i\n", "\"á=", "225\n", "="),
     ];
-
+    for (format, arg, stdout, stderr) in cases {
-    for (format, expected) in cases {
         new_ucmd!()
-            .args(&["%04x\n", format])
+            .args(&[format, arg])
+            .succeeds()
+            .stdout_is(stdout)
+            .stderr_is(format!("printf: warning: {stderr}: character(s) following character constant have been ignored\n"));
+    }
+
+    for arg in ["\"", "'"] {
+        new_ucmd!()
+            .args(&["%04x\n", arg])
+            .fails()
+            .stderr_contains("expected a numeric value");
+    }
+}
+
+#[test]
+#[cfg(target_family = "unix")]
+fn mb_invalid_unicode() {
+    use std::ffi::OsStr;
+    use std::os::unix::ffi::OsStrExt;
+
+    let cases = vec![
+        ("%04x\n", b"\"\xe1", "00e1\n"),
+        ("%04x\n", b"'\xe1", "00e1\n"),
+        ("%i\n", b"\"\xe1", "225\n"),
+        ("%i\n", b"'\xe1", "225\n"),
+        ("%f\n", b"'\xe1", "225.000000\n"),
+    ];
+    for (format, arg, stdout) in cases {
+        new_ucmd!()
+            .arg(format)
+            .arg(OsStr::from_bytes(arg))
+            .succeeds()
+            .stdout_only(stdout);
+    }
+
+    let cases = vec![
+        (b"\"\xe1=".as_slice(), "="),
+        (b"'\xe1-".as_slice(), "-"),
+        (b"'\xe1=-==".as_slice(), "=-=="),
+        (b"'\xe1'".as_slice(), "'"),
+        // unclear if original or replacement character is better in stderr
+        //(b"''\xe1'".as_slice(), "'�'"),
+    ];
+    for (arg, expected) in cases {
+        new_ucmd!()
+            .arg("%04x\n")
+            .arg(OsStr::from_bytes(arg))
             .succeeds()
             .stdout_is("00e1\n")
             .stderr_is(format!("printf: warning: {expected}: character(s) following character constant have been ignored\n"));
@@ -1364,3 +1421,35 @@ fn positional_format_specifiers() {
         .succeeds()
         .stdout_only("Octal: 115, Int: 42, Float: 3.141590, String: hello, Hex: ff, Scientific: 1.000000e-05, Char: A, Unsigned: 100, Integer: 123");
 }
+
+#[test]
+#[cfg(target_family = "unix")]
+fn non_utf_8_input() {
+    use std::ffi::OsStr;
+    use std::os::unix::ffi::OsStrExt;
+
+    // ISO-8859-1 encoded text
+    // spell-checker:disable
+    const INPUT_AND_OUTPUT: &[u8] =
+        b"Swer an rehte g\xFCete wendet s\xEEn gem\xFCete, dem volget s\xE6lde und \xEAre.";
+    // spell-checker:enable
+
+    let os_str = OsStr::from_bytes(INPUT_AND_OUTPUT);
+
+    new_ucmd!()
+        .arg("%s")
+        .arg(os_str)
+        .succeeds()
+        .stdout_only_bytes(INPUT_AND_OUTPUT);
+
+    new_ucmd!()
+        .arg(os_str)
+        .succeeds()
+        .stdout_only_bytes(INPUT_AND_OUTPUT);
+
+    new_ucmd!()
+        .arg("%d")
+        .arg(os_str)
+        .fails()
+        .stderr_contains("expected a numeric value");
+}

util/why-error.md

@@ -38,11 +38,7 @@ This file documents why some tests are failing:
 * gnu/tests/mv/part-hardlink.sh
 * gnu/tests/od/od-N.sh
 * gnu/tests/od/od-float.sh
-* gnu/tests/printf/printf-cov.pl
-* gnu/tests/printf/printf-indexed.sh
-* gnu/tests/printf/printf-mb.sh
 * gnu/tests/printf/printf-quote.sh
-* gnu/tests/printf/printf.sh
 * gnu/tests/ptx/ptx-overrun.sh
 * gnu/tests/ptx/ptx.pl
 * gnu/tests/rm/empty-inacc.sh - https://github.com/uutils/coreutils/issues/7033