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

@@ -119,7 +119,7 @@ fn extract_value<T: Default>(p: Result<T, ExtendedParserError<'_, T>>, input: &s
                 }
                 ExtendedParserError::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

@@ -354,8 +354,8 @@ fn parse(
     input: &str,
     integral_only: bool,
 ) -> Result<ExtendedBigDecimal, ExtendedParserError<'_, ExtendedBigDecimal>> {
-    // Parse the "'" prefix separately
-    if let Some(rest) = input.strip_prefix('\'') {
+    // Parse the " and ' prefixes separately
+    if let Some(rest) = input.strip_prefix(['\'', '"']) {
         let mut chars = rest.char_indices().fuse();
         let v = chars
             .next()
@@ -459,11 +459,11 @@ fn parse(
 
     // If nothing has been parsed, check if this is a special value, or declare the parsing unsuccessful
     if let Some((0, _)) = chars.peek() {
-        if integral_only {
-            return Err(ExtendedParserError::NotNumeric);
+        return if integral_only {
+            Err(ExtendedParserError::NotNumeric)
         } else {
-            return parse_special_value(unsigned, negative);
-        }
+            parse_special_value(unsigned, negative)
+        };
     }
 
     let ebd_result = construct_extended_big_decimal(digits, negative, base, scale, exponent);

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);
                 let f: ExtendedBigDecimal = args.get_extended_big_decimal();
 
                 if precision as u64 > i32::MAX as u64 {
@@ -446,7 +459,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)
@@ -455,18 +472,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)> {
@@ -484,6 +490,21 @@ 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) => match args.get_i64() {
+            v if v >= 0 => usize::try_from(v).ok(),
+            v if v < 0 => Some(0usize),
+            _ => None,
+        },
+        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,76 @@ 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|");
+
+    // Negative widths are left-aligned
+    new_ucmd!()
+        .args(&["|%*d|", "-3", "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!()
+        .args(&["|%.*d|", &i64::MIN.to_string(), "10"])
+        .succeeds()
+        .stdout_only("|10|");
+
+    new_ucmd!()
+        .args(&["|%.*d|", &format!("-{}", u128::MAX), "10"])
+        .fails_with_code(1)
+        .stdout_is("|10|")
+        .stderr_is(
+            "printf: '-340282366920938463463374607431768211455': Numerical result out of range\n",
+        );
+}
+
 #[test]
 fn sub_any_specifiers_no_params() {
     new_ucmd!()
@@ -901,6 +994,14 @@ fn negative_zero_padding_with_space_test() {
         .stdout_only("-01");
 }
 
+#[test]
+fn spaces_before_numbers_are_ignored() {
+    new_ucmd!()
+        .args(&["%*.*d", "   5", "  3", " 6"])
+        .succeeds()
+        .stdout_only("  006");
+}
+
 #[test]
 fn float_with_zero_precision_should_pad() {
     new_ucmd!()