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

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

Author: RenjiSann

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::os_str_as_bytes;
+use uucore::{format_usage, 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())
-            .map_err(|_| USimpleError::new(1, get_message("echo-error-non-utf8")))?;
+        let bytes = os_str_as_bytes(&arg)?;
 
+        // 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) {
-        // FIXME: use os_str_as_bytes once FormatArgument supports Vec<u8>
+    let values: Vec<_> = match matches.get_many::<OsString>(options::ARGUMENT) {
         Some(s) => s
-            .map(|os_string| {
-                FormatArgument::Unparsed(std::ffi::OsStr::to_string_lossy(os_string).to_string())
-            })
+            .map(|os_string| FormatArgument::Unparsed(os_string.to_owned()))
             .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,62 +76,115 @@ 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,
         }
     }
 
     pub fn next_u64(&mut self, position: &ArgumentLocation) -> u64 {
         match self.next_arg(position) {
             Some(FormatArgument::UnsignedInt(n)) => *n,
-            Some(FormatArgument::Unparsed(s)) => {
-                // 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)
-            }
+            Some(FormatArgument::Unparsed(os)) => Self::get_num::<u64>(os),
             _ => 0,
         }
     }
 
     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)) => {
-                extract_value(ExtendedBigDecimal::extended_parse(s), s)
-            }
+            Some(FormatArgument::Unparsed(os)) => Self::get_num::<ExtendedBigDecimal>(os),
             _ => 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 !bytes.is_empty() && (bytes[0] == b'\'' || bytes[0] == b'"') {
+                    if quote_start {
+                        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::Unparsed("123".to_string()),
-            FormatArgument::String("world".to_string()),
+            FormatArgument::String("hello".into()),
+            FormatArgument::Unparsed("123".into()),
+            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("123".to_string()),
-            FormatArgument::Unparsed("hello".to_string()),
-            FormatArgument::Unparsed("456".to_string()),
+            FormatArgument::Unparsed("hello".into()),
+            FormatArgument::Unparsed("123".into()),
+            FormatArgument::Unparsed("hello".into()),
+            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);