printf: Consistently handle negative widths/precision

Also allows character constants with " instead of ', and for
interpolated values with %b to use \0XXX notation for octal bytes

Author: sargas

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

@@ -33,6 +33,9 @@ pub enum FormatArgument {
 pub trait ArgumentIter<'a>: Iterator<Item = &'a FormatArgument> {
     fn get_char(&mut self) -> u8;
     fn get_i64(&mut self) -> i64;
+    /// extract an i64, returning either that value or zero for any negative value (including
+    /// negative numbers below i64::MIN)
+    fn get_i64_non_negative(&mut self) -> i64;
     fn get_u64(&mut self) -> u64;
     fn get_f64(&mut self) -> f64;
     fn get_str(&mut self) -> &'a str;
@@ -72,6 +75,19 @@ impl<'a, T: Iterator<Item = &'a FormatArgument>> ArgumentIter<'a> for T {
         }
     }
 
+    fn get_i64_non_negative(&mut self) -> i64 {
+        let Some(next) = self.next() else {
+            return 0;
+        };
+        match next {
+            FormatArgument::SignedInt(n) if !n.is_negative() => *n,
+            FormatArgument::Unparsed(s) if !s.starts_with('-') => {
+                extract_value(ParsedNumber::parse_i64(s), s)
+            }
+            _ => 0,
+        }
+    }
+
     fn get_f64(&mut self) -> f64 {
         let Some(next) = self.next() else {
             return 0.0;
@@ -113,7 +129,7 @@ fn extract_value<T: Default>(p: Result<T, ParseError<'_, T>>, input: &str) -> T
                 }
                 ParseError::PartialMatch(v, rest) => {
                     let bytes = input.as_encoded_bytes();
-                    if !bytes.is_empty() && bytes[0] == b'\'' {
+                    if !bytes.is_empty() && (bytes[0] == b'\'' || bytes[0] == b'"') {
                         show_warning!(
                             "{}: character(s) following character constant have been ignored",
                             &rest,

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

@@ -161,7 +161,7 @@ impl ParsedNumber {
     #[allow(clippy::cognitive_complexity)]
     fn parse(input: &str, integral_only: bool) -> Result<Self, ParseError<'_, Self>> {
         // Parse the "'" prefix separately
-        if let Some(rest) = input.strip_prefix('\'') {
+        if let Some(rest) = input.strip_prefix(['\'', '"']) {
             let mut chars = rest.char_indices().fuse();
             let v = chars.next().map(|(_, c)| Self {
                 base: Base::Decimal,

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

@@ -316,7 +316,7 @@ impl Spec {
         match self {
             Self::Char { width, align_left } => {
                 let (width, neg_width) =
-                    resolve_asterisk_maybe_negative(*width, &mut args).unwrap_or_default();
+                    resolve_asterisk_width(*width, &mut args).unwrap_or_default();
                 write_padded(writer, &[args.get_char()], width, *align_left || neg_width)
             }
             Self::String {
@@ -325,15 +325,15 @@ impl Spec {
                 precision,
             } => {
                 let (width, neg_width) =
-                    resolve_asterisk_maybe_negative(*width, &mut args).unwrap_or_default();
+                    resolve_asterisk_width(*width, &mut args).unwrap_or_default();
 
                 // GNU does do this truncation on a byte level, see for instance:
                 //     printf "%.1s" 🙃
                 //     > �
                 // For now, we let printf panic when we truncate within a code point.
                 // 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 precision = resolve_asterisk_precision(*precision, &mut args);
                 let s = args.get_str();
                 let truncated = match precision {
                     Some(p) if p < s.len() => &s[..p],
@@ -349,7 +349,7 @@ impl Spec {
             Self::EscapedString => {
                 let s = args.get_str();
                 let mut parsed = Vec::new();
-                for c in parse_escape_only(s.as_bytes(), OctalParsing::default()) {
+                for c in parse_escape_only(s.as_bytes(), OctalParsing::ThreeDigits) {
                     match c.write(&mut parsed)? {
                         ControlFlow::Continue(()) => {}
                         ControlFlow::Break(()) => {
@@ -382,8 +382,10 @@ impl Spec {
                 positive_sign,
                 alignment,
             } => {
-                let width = resolve_asterisk(*width, &mut args).unwrap_or(0);
-                let precision = resolve_asterisk(*precision, &mut args).unwrap_or(0);
+                let (width, neg_width) =
+                    resolve_asterisk_width(*width, &mut args).unwrap_or((0, false));
+                let precision =
+                    resolve_asterisk_precision(*precision, &mut args).unwrap_or_default();
                 let i = args.get_i64();
 
                 if precision as u64 > i32::MAX as u64 {
@@ -394,7 +396,11 @@ impl Spec {
                     width,
                     precision,
                     positive_sign: *positive_sign,
-                    alignment: *alignment,
+                    alignment: if neg_width {
+                        NumberAlignment::Left
+                    } else {
+                        *alignment
+                    },
                 }
                 .fmt(writer, i)
                 .map_err(FormatError::IoError)
@@ -405,8 +411,10 @@ impl Spec {
                 precision,
                 alignment,
             } => {
-                let width = resolve_asterisk(*width, &mut args).unwrap_or(0);
-                let precision = resolve_asterisk(*precision, &mut args).unwrap_or(0);
+                let (width, neg_width) =
+                    resolve_asterisk_width(*width, &mut args).unwrap_or((0, false));
+                let precision =
+                    resolve_asterisk_precision(*precision, &mut args).unwrap_or_default();
                 let i = args.get_u64();
 
                 if precision as u64 > i32::MAX as u64 {
@@ -417,7 +425,11 @@ impl Spec {
                     variant: *variant,
                     precision,
                     width,
-                    alignment: *alignment,
+                    alignment: if neg_width {
+                        NumberAlignment::Left
+                    } else {
+                        *alignment
+                    },
                 }
                 .fmt(writer, i)
                 .map_err(FormatError::IoError)
@@ -431,8 +443,9 @@ impl Spec {
                 alignment,
                 precision,
             } => {
-                let width = resolve_asterisk(*width, &mut args).unwrap_or(0);
-                let precision = resolve_asterisk(*precision, &mut args).unwrap_or(6);
+                let (width, neg_width) =
+                    resolve_asterisk_width(*width, &mut args).unwrap_or((0, false));
+                let precision = resolve_asterisk_precision(*precision, &mut args).unwrap_or(6);
                 // TODO: We should implement some get_extendedBigDecimal function in args to avoid losing precision.
                 let f: ExtendedBigDecimal = args.get_f64().into();
 
@@ -447,7 +460,11 @@ impl Spec {
                     case: *case,
                     force_decimal: *force_decimal,
                     positive_sign: *positive_sign,
-                    alignment: *alignment,
+                    alignment: if neg_width {
+                        NumberAlignment::Left
+                    } else {
+                        *alignment
+                    },
                 }
                 .fmt(writer, &f)
                 .map_err(FormatError::IoError)
@@ -456,18 +473,7 @@ impl Spec {
     }
 }
 
-fn resolve_asterisk<'a>(
-    option: Option<CanAsterisk<usize>>,
-    mut args: impl ArgumentIter<'a>,
-) -> Option<usize> {
-    match option {
-        None => None,
-        Some(CanAsterisk::Asterisk) => Some(usize::try_from(args.get_u64()).ok().unwrap_or(0)),
-        Some(CanAsterisk::Fixed(w)) => Some(w),
-    }
-}
-
-fn resolve_asterisk_maybe_negative<'a>(
+fn resolve_asterisk_width<'a>(
     option: Option<CanAsterisk<usize>>,
     mut args: impl ArgumentIter<'a>,
 ) -> Option<(usize, bool)> {
@@ -485,6 +491,17 @@ fn resolve_asterisk_maybe_negative<'a>(
     }
 }
 
+fn resolve_asterisk_precision<'a>(
+    option: Option<CanAsterisk<usize>>,
+    mut args: impl ArgumentIter<'a>,
+) -> Option<usize> {
+    match option {
+        None => None,
+        Some(CanAsterisk::Asterisk) => usize::try_from(args.get_i64_non_negative()).ok(),
+        Some(CanAsterisk::Fixed(w)) => Some(w),
+    }
+}
+
 fn write_padded(
     mut writer: impl Write,
     text: &[u8],

tests/by-util/test_printf.rs

@@ -82,6 +82,19 @@ fn escaped_unicode_eight_digit() {
         .stdout_only("ĥ");
 }
 
+#[test]
+fn escaped_unicode_null_byte() {
+    new_ucmd!()
+        .args(&["\\0001_"])
+        .succeeds()
+        .stdout_is_bytes([0u8, b'1', b'_']);
+
+    new_ucmd!()
+        .args(&["%b", "\\0001_"])
+        .succeeds()
+        .stdout_is_bytes([1u8, b'_']);
+}
+
 #[test]
 fn escaped_percent_sign() {
     new_ucmd!()
@@ -260,6 +273,16 @@ fn sub_num_int_char_const_in() {
         .args(&["emoji is %i", "'🙃"])
         .succeeds()
         .stdout_only("emoji is 128579");
+
+    new_ucmd!()
+        .args(&["ninety seven is %i", "\"a"])
+        .succeeds()
+        .stdout_only("ninety seven is 97");
+
+    new_ucmd!()
+        .args(&["emoji is %i", "\"🙃"])
+        .succeeds()
+        .stdout_only("emoji is 128579");
 }
 
 #[test]
@@ -546,6 +569,68 @@ fn sub_any_asterisk_negative_first_param() {
         .stdout_only("a(x    )b"); // Would be 'a(    x)b' if -5 was 5
 }
 
+#[test]
+fn sub_any_asterisk_first_param_with_integer() {
+    new_ucmd!()
+        .args(&["|%*d|", "3", "0"])
+        .succeeds()
+        .stdout_only("|  0|");
+
+    new_ucmd!()
+        .args(&["|%*d|", "1", "0"])
+        .succeeds()
+        .stdout_only("|0|");
+
+    new_ucmd!()
+        .args(&["|%*d|", "0", "0"])
+        .succeeds()
+        .stdout_only("|0|");
+
+    new_ucmd!()
+        .args(&["|%*d|", "-1", "0"])
+        .succeeds()
+        .stdout_only("|0|");
+
+    new_ucmd!()
+        .args(&["|%*d|", "-2", "0"])
+        .succeeds()
+        .stdout_only("|0 |");
+}
+
+#[test]
+fn sub_any_asterisk_second_param_with_integer() {
+    new_ucmd!()
+        .args(&["|%.*d|", "3", "10"])
+        .succeeds()
+        .stdout_only("|010|");
+
+    new_ucmd!()
+        .args(&["|%*.d|", "1", "10"])
+        .succeeds()
+        .stdout_only("|10|");
+
+    new_ucmd!()
+        .args(&["|%.*d|", "0", "10"])
+        .succeeds()
+        .stdout_only("|10|");
+
+    new_ucmd!()
+        .args(&["|%.*d|", "-1", "10"])
+        .succeeds()
+        .stdout_only("|10|");
+
+    new_ucmd!()
+        .args(&["|%.*d|", "-2", "10"])
+        .succeeds()
+        .stdout_only("|10|");
+
+    new_ucmd!()
+        // Force underflow
+        .args(&["|%.*d|", &format!("-{}", u128::MAX), "10"])
+        .succeeds()
+        .stdout_only("|10|");
+}
+
 #[test]
 fn sub_any_specifiers_no_params() {
     new_ucmd!()