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

ARGUMENT arguments

Other implementations of `printf` permit arbitrary data to be passed
to `printf`. The only restriction is that a null byte terminates
FORMAT and ARGUMENT argument strings (since they are C strings).

The current implementation only accepts FORMAT and ARGUMENT
arguments that are valid UTF-8 (this is being enforced by clap).

This commit removes the UTF-8 validation by switching to OsStr
and OsString.

This allows users to use `printf` to transmit or reformat null-safe
but not UTF-8-safe data, such as text encoded in an 8-bit text
encoding. See the `non_utf_8_input` test for an example (ISO-8859-1
text).

Author: andrewliebenow

src/uu/echo/Cargo.toml

@@ -18,7 +18,7 @@ path = "src/echo.rs"
 
 [dependencies]
 clap = { workspace = true }
-uucore = { workspace = true }
+uucore = { workspace = true, features = ["format"] }
 
 [[bin]]
 name = "echo"

src/uu/echo/src/echo.rs

@@ -6,12 +6,12 @@
 use clap::builder::ValueParser;
 use clap::parser::ValuesRef;
 use clap::{crate_version, Arg, ArgAction, Command};
-use std::ffi::{OsStr, OsString};
+use std::ffi::OsString;
 use std::io::{self, StdoutLock, Write};
 use std::iter::Peekable;
 use std::ops::ControlFlow;
 use std::slice::Iter;
-use uucore::error::{UResult, USimpleError};
+use uucore::error::UResult;
 use uucore::{format_usage, help_about, help_section, help_usage};
 
 const ABOUT: &str = help_about!("echo.md");
@@ -355,12 +355,7 @@ fn execute(
     arguments_after_options: ValuesRef<'_, OsString>,
 ) -> UResult<()> {
     for (i, input) in arguments_after_options.enumerate() {
-        let Some(bytes) = bytes_from_os_string(input.as_os_str()) else {
-            return Err(USimpleError::new(
-                1,
-                "Non-UTF-8 arguments provided, but this platform does not support them",
-            ));
-        };
+        let bytes = uucore::format::bytes_from_os_str(input)?;
 
         if i > 0 {
             stdout_lock.write_all(b" ")?;
@@ -381,23 +376,3 @@ fn execute(
 
     Ok(())
 }
-
-fn bytes_from_os_string(input: &OsStr) -> Option<&[u8]> {
-    let option = {
-        #[cfg(target_family = "unix")]
-        {
-            use std::os::unix::ffi::OsStrExt;
-
-            Some(input.as_bytes())
-        }
-
-        #[cfg(not(target_family = "unix"))]
-        {
-            // TODO
-            // Verify that this works correctly on these platforms
-            input.to_str().map(|st| st.as_bytes())
-        }
-    };
-
-    option
-}

src/uu/printf/src/printf.rs

@@ -5,11 +5,13 @@
 
 #![allow(dead_code)]
 
+use clap::builder::ValueParser;
 use clap::{crate_version, Arg, ArgAction, Command};
+use std::ffi::OsString;
 use std::io::stdout;
 use std::ops::ControlFlow;
 use uucore::error::{UResult, UUsageError};
-use uucore::format::{parse_spec_and_escape, FormatArgument, FormatItem};
+use uucore::format::{bytes_from_os_str, parse_spec_and_escape, FormatArgument, FormatItem};
 use uucore::{format_usage, help_about, help_section, help_usage};
 
 const VERSION: &str = "version";
@@ -28,17 +30,22 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let matches = uu_app().get_matches_from(args);
 
     let format = matches
-        .get_one::<String>(options::FORMAT)
+        .get_one::<OsString>(options::FORMAT)
         .ok_or_else(|| UUsageError::new(1, "missing operand"))?;
 
-    let values: Vec<_> = match matches.get_many::<String>(options::ARGUMENT) {
-        Some(s) => s.map(|s| FormatArgument::Unparsed(s.to_string())).collect(),
-        None => vec![],
+    let format_bytes = bytes_from_os_str(format)?;
+
+    let values = match matches.get_many::<OsString>(options::ARGUMENT) {
+        Some(os_string) => os_string
+            .map(|os_string_ref| FormatArgument::Unparsed(os_string_ref.to_owned()))
+            .collect(),
+        None => Vec::<FormatArgument>::new(),
     };
 
     let mut format_seen = false;
     let mut args = values.iter().peekable();
-    for item in parse_spec_and_escape(format.as_ref()) {
+
+    for item in parse_spec_and_escape(format_bytes) {
         if let Ok(FormatItem::Spec(_)) = item {
             format_seen = true;
         }
@@ -55,7 +62,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     }
 
     while args.peek().is_some() {
-        for item in parse_spec_and_escape(format.as_ref()) {
+        for item in parse_spec_and_escape(format_bytes) {
             match item?.write(stdout(), &mut args)? {
                 ControlFlow::Continue(()) => {}
                 ControlFlow::Break(()) => return Ok(()),
@@ -86,6 +93,10 @@ pub fn uu_app() -> Command {
                 .help("Print version information")
                 .action(ArgAction::Version),
         )
-        .arg(Arg::new(options::FORMAT))
-        .arg(Arg::new(options::ARGUMENT).action(ArgAction::Append))
+        .arg(Arg::new(options::FORMAT).value_parser(ValueParser::os_string()))
+        .arg(
+            Arg::new(options::ARGUMENT)
+                .action(ArgAction::Append)
+                .value_parser(ValueParser::os_string()),
+        )
 }

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

@@ -4,13 +4,13 @@
 // file that was distributed with this source code.
 
 use crate::{
-    error::set_exit_code,
+    error::{set_exit_code, UResult, USimpleError},
     features::format::num_parser::{ParseError, ParsedNumber},
     quoting_style::{escape_name, Quotes, QuotingStyle},
-    show_error, show_warning,
+    show, show_error, show_warning,
 };
 use os_display::Quotable;
-use std::ffi::OsStr;
+use std::ffi::{OsStr, OsString};
 
 /// An argument for formatting
 ///
@@ -22,20 +22,20 @@ use std::ffi::OsStr;
 #[derive(Clone, Debug)]
 pub enum FormatArgument {
     Char(char),
-    String(String),
+    String(OsString),
     UnsignedInt(u64),
     SignedInt(i64),
     Float(f64),
     /// Special argument that gets coerced into the other variants
-    Unparsed(String),
+    Unparsed(OsString),
 }
 
 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_str(&mut self) -> &'a str;
+    fn get_str(&mut self) -> &'a OsStr;
 }
 
 impl<'a, T: Iterator<Item = &'a FormatArgument>> ArgumentIter<'a> for T {
@@ -45,7 +45,10 @@ impl<'a, T: Iterator<Item = &'a FormatArgument>> ArgumentIter<'a> for T {
         };
         match next {
             FormatArgument::Char(c) => *c as u8,
-            FormatArgument::Unparsed(s) => s.bytes().next().unwrap_or(b'\0'),
+            FormatArgument::Unparsed(os) => match bytes_from_os_str(os).unwrap().first() {
+                Some(&byte) => byte,
+                None => b'\0',
+            },
             _ => b'\0',
         }
     }
@@ -56,7 +59,11 @@ impl<'a, T: Iterator<Item = &'a FormatArgument>> ArgumentIter<'a> for T {
         };
         match next {
             FormatArgument::UnsignedInt(n) => *n,
-            FormatArgument::Unparsed(s) => extract_value(ParsedNumber::parse_u64(s), s),
+            FormatArgument::Unparsed(os) => {
+                let str = get_str_or_exit_with_error(os);
+
+                extract_value(ParsedNumber::parse_u64(str), str)
+            }
             _ => 0,
         }
     }
@@ -67,7 +74,11 @@ impl<'a, T: Iterator<Item = &'a FormatArgument>> ArgumentIter<'a> for T {
         };
         match next {
             FormatArgument::SignedInt(n) => *n,
-            FormatArgument::Unparsed(s) => extract_value(ParsedNumber::parse_i64(s), s),
+            FormatArgument::Unparsed(os) => {
+                let str = get_str_or_exit_with_error(os);
+
+                extract_value(ParsedNumber::parse_i64(str), str)
+            }
             _ => 0,
         }
     }
@@ -78,15 +89,19 @@ impl<'a, T: Iterator<Item = &'a FormatArgument>> ArgumentIter<'a> for T {
         };
         match next {
             FormatArgument::Float(n) => *n,
-            FormatArgument::Unparsed(s) => extract_value(ParsedNumber::parse_f64(s), s),
+            FormatArgument::Unparsed(os) => {
+                let str = get_str_or_exit_with_error(os);
+
+                extract_value(ParsedNumber::parse_f64(str), str)
+            }
             _ => 0.0,
         }
     }
 
-    fn get_str(&mut self) -> &'a str {
+    fn get_str(&mut self) -> &'a OsStr {
         match self.next() {
-            Some(FormatArgument::Unparsed(s) | FormatArgument::String(s)) => s,
-            _ => "",
+            Some(FormatArgument::Unparsed(os) | FormatArgument::String(os)) => os,
+            _ => "".as_ref(),
         }
     }
 }
@@ -126,3 +141,50 @@ fn extract_value<T: Default>(p: Result<T, ParseError<'_, T>>, input: &str) -> T
         }
     }
 }
+
+pub fn bytes_from_os_str(input: &OsStr) -> UResult<&[u8]> {
+    let result = {
+        #[cfg(target_family = "unix")]
+        {
+            use std::os::unix::ffi::OsStrExt;
+
+            Ok(input.as_bytes())
+        }
+
+        #[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",
+                )),
+            }
+        }
+    };
+
+    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/spec.rs

@@ -5,15 +5,15 @@
 
 // spell-checker:ignore (vars) intmax ptrdiff padlen
 
-use crate::quoting_style::{escape_name, QuotingStyle};
-
 use super::{
+    bytes_from_os_str,
     num_format::{
         self, Case, FloatVariant, ForceDecimal, Formatter, NumberAlignment, PositiveSign, Prefix,
         UnsignedIntVariant,
     },
     parse_escape_only, ArgumentIter, FormatChar, FormatError,
 };
+use crate::quoting_style::{escape_name, QuotingStyle};
 use std::{io::Write, ops::ControlFlow};
 
 /// A parsed specification for formatting a value
@@ -331,17 +331,26 @@ 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, &mut args)?;
-                let s = args.get_str();
+
+                let os_str = args.get_str();
+
+                let bytes = bytes_from_os_str(os_str).unwrap();
+
                 let truncated = match precision {
-                    Some(p) if p < s.len() => &s[..p],
-                    _ => s,
+                    Some(p) if p < os_str.len() => &bytes[..p],
+                    _ => bytes,
                 };
-                write_padded(writer, truncated.as_bytes(), width, *align_left)
+
+                write_padded(writer, truncated, width, *align_left)
             }
             Self::EscapedString => {
-                let s = args.get_str();
-                let mut parsed = Vec::new();
-                for c in parse_escape_only(s.as_bytes()) {
+                let os_str = args.get_str();
+
+                let bytes = bytes_from_os_str(os_str).unwrap();
+
+                let mut parsed = Vec::<u8>::new();
+
+                for c in parse_escape_only(bytes) {
                     match c.write(&mut parsed)? {
                         ControlFlow::Continue(()) => {}
                         ControlFlow::Break(()) => {
@@ -353,11 +362,12 @@ impl Spec {
                 writer.write_all(&parsed).map_err(FormatError::IoError)
             }
             Self::QuotedString => {
-                let s = args.get_str();
+                let os = args.get_str();
+
                 writer
                     .write_all(
                         escape_name(
-                            s.as_ref(),
+                            os,
                             &QuotingStyle::Shell {
                                 escape: true,
                                 always_quote: false,

tests/by-util/test_printf.rs

@@ -916,3 +916,29 @@ fn float_flag_position_space_padding() {
         .succeeds()
         .stdout_only(" +1.0");
 }
+
+#[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);
+}