Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/uucore/src/lib/features/format/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,23 @@ fn check_width(width: usize) -> std::io::Result<()> {
}
}

/// Write `count` copies of `pad` to `writer`.
///
/// Unlike `write!(writer, "{s:>width$}")`, this does not feed the width into the
/// standard formatting machinery, which panics once a dynamic width exceeds
/// `u16::MAX`. The padding is emitted in fixed-size chunks, so a wide field
/// costs no extra memory.
fn write_padding(mut writer: impl Write, pad: u8, count: usize) -> std::io::Result<()> {
let chunk = [pad; 64];
let mut remaining = count;
while remaining > 0 {
let n = remaining.min(chunk.len());
writer.write_all(&chunk[..n])?;
remaining -= n;
}
Ok(())
}

/// Reject a precision larger than printf/C allows (`i32::MAX`).
///
/// A precision near `usize::MAX` would otherwise overflow the precision/exponent
Expand Down
151 changes: 138 additions & 13 deletions src/uucore/src/lib/features/format/num_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@
//
// For the full copyright and license information, please view the LICENSE
// file that was distributed with this source code.
// spell-checker:ignore bigdecimal prec cppreference bignums
// spell-checker:ignore bigdecimal prec cppreference bignums unpadded
//! Utilities for formatting numbers in various formats

use bigdecimal::BigDecimal;
use bigdecimal::RoundingMode;
use bigdecimal::num_bigint::ToBigInt;
use num_traits::Signed;
use num_traits::Zero;
Expand Down Expand Up @@ -385,6 +386,21 @@ fn zero_pad_to(s: &str, width: usize) -> String {
}
}

/// Render zero with `precision` digits after the decimal point, as the `%e`,
/// `%g` and `%a` formatters need for a zero value.
///
/// `format!("{:.precision$}", 0.0)` would panic once the precision exceeds
/// `u16::MAX`.
fn zero_with_fraction_digits(precision: usize) -> String {
if precision == 0 {
return String::from("0");
}
let mut out = String::with_capacity(precision + 2);
out.push_str("0.");
out.extend(std::iter::repeat_n('0', precision));
out
}

fn get_sign_indicator(sign: PositiveSign, negative: bool) -> String {
if negative {
String::from("-")
Expand Down Expand Up @@ -426,10 +442,79 @@ fn format_float_decimal(
// Optimization when printing integers.
return bi.to_str_radix(10);
} else if force_decimal == ForceDecimal::Yes {
return format!("{bd:.0}.");
return format!("{}.", decimal_digits(bd, 0));
}
}
decimal_digits(bd, precision)
}

/// Render `bd` in decimal notation with exactly `precision` fractional digits.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you sure these long comments are valuable ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trimmed them, kept only why each branch is there. Same in the tests.

///
/// The digits are placed here rather than with `format!("{bd:.precision$}")`
/// because that path has two limits that `printf`/`seq` input reaches. The
/// standard formatting machinery panics with "Formatting argument out of range"
/// once a dynamic precision exceeds `u16::MAX`, and `bigdecimal`'s `Display`
/// stops zero-padding past `FMT_MAX_INTEGER_PADDING` digits and returns the
/// value unpadded, so `%.1000f` of `1` printed `1` instead of `1.000...`.
fn decimal_digits(bd: &BigDecimal, precision: usize) -> String {
let scale = bd.fractional_digit_count();

// Expanding an integer magnitude costs one output byte per power of ten.
// Above `MAX_FORMAT_WIDTH` powers, defer to `bigdecimal`'s exponential form
// rather than allocate gigabytes. Below it the value is expanded in full:
// `%f` is decimal by definition, and `bigdecimal`'s `Display` otherwise falls
// back to exponential past 1000 integer zeros (so `%f` of `1e5000` used to
// print `1e+5000`), which this keeps as a plain decimal instead.
if scale < -(super::MAX_FORMAT_WIDTH as i64) {
return format!("{bd:.0}");
}

// Rounding is only needed when the value holds more fractional digits than
// were asked for. That case produces exactly `precision` fractional digits
// with no padding, so `bigdecimal`'s `Display` handles it correctly and more
// cheaply than a manual round; only step in once the precision would make it
// panic. When no rounding is needed the missing digits are all zeros and get
// appended as text below, which keeps a large precision cheap.
if scale > precision as i64 && u16::try_from(precision).is_ok() {
return format!("{bd:.precision$}");
}
let (digits, scale) = if scale > precision as i64 {
bd.with_scale_round(precision as i64, RoundingMode::default())
.into_bigint_and_exponent()
} else {
bd.as_bigint_and_exponent()
};
let digits = digits.to_str_radix(10);

// `digits` is the value scaled by `10^scale`, so the decimal point belongs
// `scale` places from the right.
let mut out = String::with_capacity(digits.len() + precision + 2);
if scale <= 0 {
// An integer, held as `digits` followed by `-scale` zeros. Zero keeps a
// single digit rather than growing a run of leading zeros.
out.push_str(&digits);
if digits != "0" {
out.extend(std::iter::repeat_n('0', -scale as usize));
}
if precision > 0 {
out.push('.');
out.extend(std::iter::repeat_n('0', precision));
}
} else {
let scale = scale as usize;
if let Some(int_len) = digits.len().checked_sub(scale).filter(|len| *len > 0) {
out.push_str(&digits[..int_len]);
out.push('.');
out.push_str(&digits[int_len..]);
} else {
// No integer digits, so the fraction starts with leading zeros.
out.push_str("0.");
out.extend(std::iter::repeat_n('0', scale - digits.len()));
out.push_str(&digits);
}
out.extend(std::iter::repeat_n('0', precision - scale));
}
format!("{bd:.precision$}")
out
}

/// Converts a `&BigDecimal` to a scientific-like `X.XX * 10^e`.
Expand Down Expand Up @@ -484,7 +569,7 @@ fn format_float_scientific(
return if force_decimal == ForceDecimal::Yes && precision == 0 {
format!("0.{exp_char}+00")
} else {
format!("{:.precision$}{exp_char}+00", 0.0)
format!("{}{exp_char}+00", zero_with_fraction_digits(precision))
};
}

Expand Down Expand Up @@ -521,9 +606,7 @@ fn format_float_shortest(
if BigDecimal::zero().eq(bd) {
return match (force_decimal, precision) {
(ForceDecimal::Yes, 1) => "0.".into(),
(ForceDecimal::Yes, _) => {
format!("{:.*}", precision - 1, 0.0)
}
(ForceDecimal::Yes, _) => zero_with_fraction_digits(precision - 1),
(ForceDecimal::No, _) => "0".into(),
};
}
Expand Down Expand Up @@ -624,7 +707,10 @@ fn format_float_hexadecimal(
return if force_decimal == ForceDecimal::Yes && precision.unwrap_or(0) == 0 {
format!("0x0.{exp_char}+0")
} else {
format!("0x{:.*}{exp_char}+0", precision.unwrap_or(0), 0.0)
format!(
"0x{}{exp_char}+0",
zero_with_fraction_digits(precision.unwrap_or(0))
)
};
}

Expand Down Expand Up @@ -770,15 +856,25 @@ fn write_output(
super::check_width(remaining_width)?;

match alignment {
NumberAlignment::Left => write!(writer, "{sign_indicator}{s:<remaining_width$}"),
NumberAlignment::Left => {
writer.write_all(sign_indicator.as_bytes())?;
writer.write_all(s.as_bytes())?;
super::write_padding(&mut writer, b' ', remaining_width.saturating_sub(s.len()))
}
NumberAlignment::RightSpace => {
let is_sign = sign_indicator.starts_with('-') || sign_indicator.starts_with('+'); // When sign_indicator is in ['-', '+']
if is_sign && remaining_width > 0 {
// Make sure sign_indicator is just next to number, e.g. "% +5.1f" 1 ==> $ +1.0
let s = sign_indicator + s.as_str();
write!(writer, "{s:>width$}", width = remaining_width + 1) // Since we now add sign_indicator and s together, plus 1
// Since we now add sign_indicator and s together, plus 1
let width = remaining_width + 1;
let len = sign_indicator.len() + s.len();
super::write_padding(&mut writer, b' ', width.saturating_sub(len))?;
writer.write_all(sign_indicator.as_bytes())?;
writer.write_all(s.as_bytes())
} else {
write!(writer, "{sign_indicator}{s:>remaining_width$}")
writer.write_all(sign_indicator.as_bytes())?;
super::write_padding(&mut writer, b' ', remaining_width.saturating_sub(s.len()))?;
writer.write_all(s.as_bytes())
}
}
NumberAlignment::RightZero => {
Expand All @@ -789,7 +885,14 @@ fn write_output(
("", s.as_str())
};
let remaining_width = remaining_width.saturating_sub(prefix.len());
write!(writer, "{sign_indicator}{prefix}{rest:0>remaining_width$}")
writer.write_all(sign_indicator.as_bytes())?;
writer.write_all(prefix.as_bytes())?;
super::write_padding(
&mut writer,
b'0',
remaining_width.saturating_sub(rest.len()),
)?;
writer.write_all(rest.as_bytes())
}
}
}
Expand Down Expand Up @@ -1268,6 +1371,28 @@ mod test {
assert!(s.ends_with("0ff"));
}

#[test]
fn format_float_large_precision_and_width() {
// A precision above u16::MAX must not panic here either (#12708), and
// a precision above 1000 must still print its zeros.
let one = ExtendedBigDecimal::BigDecimal(BigDecimal::from_str("1").unwrap());

let format = Format::<Float, &ExtendedBigDecimal>::parse("%.100000f").unwrap();
let s = fmt(&format, &one);
assert_eq!(s.len(), 100_002);
assert!(s.starts_with("1."));
assert!(s.ends_with('0'));

let format = Format::<Float, &ExtendedBigDecimal>::parse("%.1000f").unwrap();
assert_eq!(fmt(&format, &one), format!("1.{}", "0".repeat(1000)));

let format = Format::<Float, &ExtendedBigDecimal>::parse("%100000f").unwrap();
let s = fmt(&format, &one);
assert_eq!(s.len(), 100_000);
assert!(s.ends_with("1.000000"));
assert!(s.starts_with(' '));
}

#[test]
fn format_signed_int_precision_zero() {
let format = Format::<SignedInt, i64>::parse("%.0d").unwrap();
Expand Down
4 changes: 2 additions & 2 deletions src/uucore/src/lib/features/format/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -550,9 +550,9 @@ fn write_padded(

if left {
writer.write_all(text)?;
write!(writer, "{: <padlen$}", "")
super::write_padding(&mut writer, b' ', padlen)
} else {
write!(writer, "{: >padlen$}", "")?;
super::write_padding(&mut writer, b' ', padlen)?;
writer.write_all(text)
}
.map_err(FormatError::IoError)
Expand Down
77 changes: 76 additions & 1 deletion tests/by-util/test_printf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// For the full copyright and license information, please view the LICENSE
// file that was distributed with this source code.

// spell-checker:ignore fffffffffffffffc
// spell-checker:ignore fffffffffffffffc bigdecimal unpadded
use uutests::new_ucmd;

#[test]
Expand Down Expand Up @@ -1528,6 +1528,81 @@ fn test_large_width_format() {
}
}

#[test]
fn large_width_pads_instead_of_aborting() {
// A dynamic width above u16::MAX makes std::fmt panic with "Formatting
// argument out of range", which the release profile turns into SIGABRT.
// 65535 always worked, so 65536 is the interesting boundary.
let cases = [
("%65536d", "1", " ".repeat(65535) + "1"),
("%70000d", "1", " ".repeat(69999) + "1"),
("%70000s", "x", " ".repeat(69999) + "x"),
("%70000c", "y", " ".repeat(69999) + "y"),
("%-70000d", "1", "1".to_string() + &" ".repeat(69999)),
("%070000d", "1", "0".repeat(69999) + "1"),
];

for (format, arg, expected) in cases {
new_ucmd!()
.args(&[format, arg])
.succeeds()
.stdout_only(expected);
}
}

#[test]
fn large_precision_pads_instead_of_aborting() {
let cases = [
("%.70000f", "1", format!("1.{}", "0".repeat(70000))),
("%.70000f", "0", format!("0.{}", "0".repeat(70000))),
("%.70000e", "0", format!("0.{}e+00", "0".repeat(70000))),
("%.70000E", "0", format!("0.{}E+00", "0".repeat(70000))),
("%.70000a", "0", format!("0x0.{}p+0", "0".repeat(70000))),
("%#.70000g", "0", format!("0.{}", "0".repeat(69999))),
("%#.70000G", "0", format!("0.{}", "0".repeat(69999))),
];

for (format, arg, expected) in cases {
new_ucmd!()
.args(&[format, arg])
.succeeds()
.stdout_only(expected);
}
}

#[test]
fn precision_above_1000_keeps_fractional_zeros() {
// bigdecimal's Display stops zero-padding an integer past
// FMT_MAX_INTEGER_PADDING (1000) and returns the digits unpadded, so this
// used to print "1" with no fractional part at all.
new_ucmd!()
.args(&["%.1000f", "1"])
.succeeds()
.stdout_only(format!("1.{}", "0".repeat(1000)));

new_ucmd!()
.args(&["%.5000f", "100"])
.succeeds()
.stdout_only(format!("100.{}", "0".repeat(5000)));

// A value with a fractional part took a different bigdecimal branch and
// was already correct; keep it that way.
new_ucmd!()
.args(&["%.1000f", "1.5"])
.succeeds()
.stdout_only(format!("1.5{}", "0".repeat(999)));
}

#[test]
fn large_integer_magnitude_stays_decimal() {
// A large integer magnitude hits the same bigdecimal padding cap and used
// to print in exponential form (`1e+2000`), which `%f` must not do.
new_ucmd!()
.args(&["%.0f", "1e2000"])
.succeeds()
.stdout_only(format!("1{}", "0".repeat(2000)));
}

#[test]
fn test_extreme_field_width_overflow() {
// Test the specific case that was causing panic due to integer overflow
Expand Down
8 changes: 8 additions & 0 deletions tests/by-util/test_seq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,14 @@ fn test_parse_error_hex() {
.usage_error("invalid floating point argument: '0xlmnop'");
}

#[test]
fn test_large_precision_format() {
new_ucmd!()
.args(&["-f", "%.70000f", "1", "1"])
.succeeds()
.stdout_only(format!("1.{}\n", "0".repeat(70000)));
}

#[test]
fn test_format_option() {
new_ucmd!()
Expand Down
Loading