From 448aed8f4bd964a2faf918c0ef91af2857fa28a7 Mon Sep 17 00:00:00 2001 From: Ryan Lopopolo Date: Thu, 20 Mar 2025 09:09:32 -0700 Subject: [PATCH 01/10] Add tests for dynamic width specifiers exceeding `u16::MAX` Rust PR rust-lang/rust#136932 (part of rust-lang/rust#99012) limited format string width and precision to 16 bits, causing panics when dynamic padding exceeds `u16::MAX`. These tests validate handling excessively large widths discovered via fuzzing in artichoke/strftime-ruby. They ensure correct, panic-free behavior consistent with CRuby's `Time#strftime`. Additionally add tests for width specifiers which exceed `INT_MAX` to ensure they return the formatting string verbatim, which were among the cases discussed in the upstream PR. See: - Upstream report: https://github.com/rust-lang/rust/pull/136932#issuecomment-2739434542 - Proposed fix: https://github.com/rust-lang/rust/pull/136932#issuecomment-2740004406 --- src/tests/format.rs | 52 +-- src/tests/mod.rs | 60 +++ src/tests/rust_fmt_argument_max_padding.rs | 456 +++++++++++++++++++++ 3 files changed, 518 insertions(+), 50 deletions(-) create mode 100644 src/tests/rust_fmt_argument_max_padding.rs diff --git a/src/tests/format.rs b/src/tests/format.rs index a780f8a5..176ea8a5 100644 --- a/src/tests/format.rs +++ b/src/tests/format.rs @@ -1,57 +1,9 @@ #![allow(clippy::should_panic_without_expect)] use crate::format::TimeFormatter; -use crate::{Error, Time}; +use crate::Error; -include!("../mock.rs.in"); - -fn get_format_err(time: &MockTime<'_>, format: &str) -> Error { - TimeFormatter::new(time, format) - .fmt(&mut &mut [0u8; 100][..]) - .unwrap_err() -} - -fn check_format(time: &MockTime<'_>, format: &str, expected: &str) { - const SIZE: usize = 100; - let mut buf = [0u8; SIZE]; - let mut cursor = &mut buf[..]; - - TimeFormatter::new(time, format).fmt(&mut cursor).unwrap(); - let written = SIZE - cursor.len(); - let data = core::str::from_utf8(&buf[..written]).unwrap(); - - assert_eq!(data, expected); -} - -fn check_all(times: &[MockTime<'_>], format: &str, all_expected: &[&str]) { - assert_eq!(times.len(), all_expected.len()); - for (time, expected) in times.iter().zip(all_expected) { - check_format(time, format, expected); - } -} - -#[test] -#[should_panic] -#[rustfmt::skip] -fn test_check_format_panics_on_error() { - let time = MockTime { year: 1111, ..Default::default() }; - - check_format(&time, "'%Y'", "'1112'"); -} - -#[test] -#[should_panic] -#[rustfmt::skip] -fn test_check_all_panics_on_error() { - let times = [ - MockTime { year: -1111, ..Default::default() }, - MockTime { year: -11, ..Default::default() }, - MockTime { year: 1, ..Default::default() }, - MockTime { year: 1111, ..Default::default() }, - ]; - - check_all(×, "'%Y'", &["'-1111'", "'-0011'", "'0001'", "'1112'"]); -} +use super::{check_all, check_format, get_format_err, MockTime}; #[test] #[rustfmt::skip] diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 4c993e03..5006a19f 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -1,2 +1,62 @@ mod error; mod format; +mod rust_fmt_argument_max_padding; + +use crate::format::TimeFormatter; +use crate::{Error, Time}; + +include!("../mock.rs.in"); + +fn get_format_err(time: &MockTime<'_>, format: &str) -> Error { + TimeFormatter::new(time, format) + .fmt(&mut &mut [0u8; 100][..]) + .unwrap_err() +} + +fn get_format_err_bytes(time: &MockTime<'_>, format: &[u8]) -> Error { + TimeFormatter::new(time, format) + .fmt(&mut &mut [0u8; 100][..]) + .unwrap_err() +} + +fn check_format(time: &MockTime<'_>, format: &str, expected: &str) { + const SIZE: usize = 100; + let mut buf = [0u8; SIZE]; + let mut cursor = &mut buf[..]; + + TimeFormatter::new(time, format).fmt(&mut cursor).unwrap(); + let written = SIZE - cursor.len(); + let data = core::str::from_utf8(&buf[..written]).unwrap(); + + assert_eq!(data, expected); +} + +fn check_all(times: &[MockTime<'_>], format: &str, all_expected: &[&str]) { + assert_eq!(times.len(), all_expected.len()); + for (time, expected) in times.iter().zip(all_expected) { + check_format(time, format, expected); + } +} + +#[test] +#[should_panic] +#[rustfmt::skip] +fn test_check_format_panics_on_error() { + let time = MockTime { year: 1111, ..Default::default() }; + + check_format(&time, "'%Y'", "'1112'"); +} + +#[test] +#[should_panic] +#[rustfmt::skip] +fn test_check_all_panics_on_error() { + let times = [ + MockTime { year: -1111, ..Default::default() }, + MockTime { year: -11, ..Default::default() }, + MockTime { year: 1, ..Default::default() }, + MockTime { year: 1111, ..Default::default() }, + ]; + + check_all(×, "'%Y'", &["'-1111'", "'-0011'", "'0001'", "'1112'"]); +} diff --git a/src/tests/rust_fmt_argument_max_padding.rs b/src/tests/rust_fmt_argument_max_padding.rs new file mode 100644 index 00000000..5c6faffc --- /dev/null +++ b/src/tests/rust_fmt_argument_max_padding.rs @@ -0,0 +1,456 @@ +//! These tests exist to validate padding behavior when using dynamic widths in +//! format strings. As of rust-lang/rust#136932 (part of #99012), the Rust +//! standard library now restricts width and precision fields in format strings +//! to u16::MAX, to improve memory layout and prevent silent truncation bugs on +//! cross-compilation targets. This change caused previously valid dynamic width +//! values exceeding u16::MAX to panic at runtime. (See rust-lang/rust#136932 +//! and artichoke/strftime-ruby#2f0ab4a). +//! +//! These test cases specifically target edge conditions revealed by fuzzing +//! strftime-ruby, ensuring we handle excessively large padding values without +//! panicking, aligning behavior closely with CRuby's `Time#strftime` +//! specification and the limitations described in rust-lang/rust#136932. +//! +//! Reference: +//! +//! - +//! - + +use crate::format::TimeFormatter; +use crate::Error; + +use super::{check_all, get_format_err, get_format_err_bytes, MockTime}; + +#[test] +fn test_larger_than_int_max_formats_are_returned_verbatim() { + let times = [ + MockTime::new(1970, 1, 1, 0, 0, 0, 0, 4, 1, 0, false, 0, ""), + MockTime::new(-1970, 1, 1, 0, 0, 0, 0, 4, 1, 0, false, 0, ""), + ]; + + for format in [ + "%100000000000000000000c", + "%1000000000000c", + "%10000000000c", + ] { + check_all(×, format, &[format, format]); + } +} + +#[test] +fn test_rust_136932_reduce_fmt_argument_width_and_precision_fuzzer_failures() { + let test_cases: [(MockTime<'_>, &str); 8] = [ + ( + MockTime::new( + -184549375, + 93, + 71, + 166, + 14, + 1, + 2788140547, + 166, + 65535, + 376621239690988031, + false, + -1499059802, + "Z\t\u{3}\u{1}9999" + ), + "=99\0\0\0Y%66067c\0\0" + ), + ( + MockTime::new( + 12255231, + 202, + 175, + 0, + 2, + 0, + 3284320768, + 251, + 0, + -5836660741859442688, + false, + 65538, + "" + ), + "\0%666666%%3%" + ), + ( + MockTime::new( + 1737472, + 0, + 0, + 0, + 0, + 11, + 0, + 0, + 0, + -256, + false, + 64256, + "" + ), + "%111111C1111111111111117\u{1}\0X\0XXD" + ), + ( + MockTime::new( + 3825760, + 0, + 0, + 51, + 0, + 0, + 1044381696, + 37, + 122, + 6584368208339936574, + false, + 0, + "" + ), + "@%1111111111z\0\0" + ), + ( + MockTime::new( + 1090397537, + 254, + 255, + 2, + 0, + 0, + 4294639365, + 255, + 65535, + 10455414982377471, + false, + -33540827, + "" + ), + "%%\n%5555555%55555555555u555" + ), + ( + MockTime::new( + -21761, + 255, + 58, + 0, + 37, + 37, + 620822272, + 255, + 43690, + 50396945, + false, + 3, + "" + ), + "%R%S%00000400000R%RR%%%\u{3}\0%\0\u{1b}\u{1b}\u{1b}" + ), + ( + MockTime::new( + 13566191, + 1, + 0, + 34, + 35, + 37, + 623190565, + 0, + 8741, + 2457877026632491810, + true, + 606414171, + "" + ), + "\0\0\0\0\0\0\0\"%2536361%172049423024124\"2\"\"9\"%^^^YN^\"" + ), + ( + MockTime::new( + 1749024784, + 96, + 0, + 96, + 104, + 96, + 620796776, + 0, + 15968, + 0, + false, + 6307376, + "" + ), + "%u\0\n\0\0\u{10}\0`\u{31b}@\0\0\02>`\0%33333333s33333#33333333333333333333333333333333333\u{13}33333333333u\0\0\0\0\0%u\u{4}\0\0\0\u{10}\0`" + ), + ]; + + for (time, format) in test_cases { + let err = get_format_err(&time, format); + assert!(matches!(err, Error::WriteZero)); + } +} + +#[test] +fn test_rust_136932_reduce_fmt_argument_width_and_precision_fuzzer_failures_bytes() { + let test_cases: [(MockTime<'_>, &[u8]); 8] = [ + ( + MockTime::new( + 674049317, + 244, + 42, + 40, + 180, + 106, + 4096628178, + 1, + 0, + 3239384020795663872, + false, + 1734567725, + "" + ), + b"\xd6\x00\x00\x00\x00\xda\xc7\xda%\xc1B%-666666F\xff\xff\xff" + ), + ( + MockTime::new( + 167837442, + 64, + 255, + 10, + 96, + 255, + 4294901760, + 255, + 65535, + -1, + true, + 235864006, + "" + ), + b"\x88%%%%%%%%%%%%%777777%%%\x0e" + ), + ( + MockTime::new( + 1375731722, + 82, + 82, + 82, + 85, + 82, + 1392508927, + 82, + 20992, + -48935437123235246, + false, + -1145324613, + "" + ), + b"\xbb\xbb%\x00\x00RR%\xbb\xbb\x00\x00\x00B%00124261%R\xff\x00\xbb\xbb\xbb\xbb\x00\x00\x00\x00\x00RRR\xa6RR" + ), + ( + MockTime::new( + 0, + 0, + 0, + 0, + 92, + 0, + 704643072, + 0, + 242, + 5548434740937171968, + false, + 0, + "" + ), + b"\x00\x00%7777773%" + ), + ( + MockTime::new( + 458557, + 0, + 64, + 128, + 128, + 128, + 4278222976, + 255, + 65535, + -205, + true, + 65535, + "\0\0\0" + ), + b"\x00%00324248X14\xff" + ), + ( + MockTime::new( + -774831746, + 209, + 209, + 209, + 13, + 13, + 218959117, + 13, + 3341, + 0, + false, + 218955776, + ":\r\r" + ), + b"\xff\x00\xff\xff\x08%%%%%%%%%%%%%5%%%%3333333%%" + ), + ( + MockTime::new( + 130826, + 3, + 223, + 29, + 37, + 37, + 623191333, + 93, + 13861, + 2892759177136963579, + true, + 623191333, + "!I\u{1}\0\u{3}" + ), + b"\xdf\x1d%\x1d?\xff\xdb\xfe#\x15I%$\x00\x00\x00%]%86995%%6\xfb" + ), + ( + MockTime::new( + 37454602, + 0, + 0, + 0, + 0, + 9, + 1048576, + 255, + 65535, + -1, + false, + 255, + "\0\0\0\0\0/\0" + ), + b"y%999999%q)%%z" + ), + ]; + + for (time, format) in test_cases { + let err = get_format_err_bytes(&time, format); + assert!(matches!(err, Error::WriteZero)); + } +} + +#[test] +#[cfg(feature = "alloc")] +fn test_format_specifiers_large_width_success() { + use alloc::format; + use alloc::vec; + + // List of format specifiers that take a width. + // For each, we construct a format string with a width of 65,636. + // The format string is wrapped in single quotes so that we can easily + // strip them and check that the inner formatted result has exactly the given width. + let specifiers = [ + "Y", "C", "y", "m", "B", "b", "d", "e", "j", "H", "k", "I", "l", "P", "p", "M", "S", "L", + "N", "z", ":z", "::z", ":::z", "Z", "A", "a", "u", "w", "G", "g", "V", "U", "W", "s", "n", + "t", "c", "D", "F", "v", "r", "R", "T", "X", + ]; + let width: usize = 65_636; + // A valid and interesting MockTime instance that exercises a wide range of + // specifiers (e.g. year, month, day, time, fractional seconds, week day, + // time zone, etc.): + let time = MockTime::new( + 2021, // year: 2021 (a recent common year) + 12, // month: December + 31, // day: 31st (last day of the year) + 23, // hour: 23 (will yield 11 PM in 12-hour formats) + 59, // minute: 59 + 60, // second: 60 (testing the leap-second edge case, as spec allows 00..=60) + 987654321, // nanoseconds: an interesting fraction for testing %L and %N + 5, // day_of_week: 5 (if 0 = Sunday, then 5 = Friday) + 365, // day_of_year: December 31 is the 365th day in a non-leap year + 1640995200, // to_int: seconds since epoch (an arbitrary value corresponding roughly to 2022-01-01T00:00:00 UTC) + false, // is_utc: false (indicating local time) + 3600, // utc_offset: +3600 seconds (i.e. UTC+1) + "CET", // time_zone: the time zone name (e.g. "CET") + ); + + for &spec in specifiers.iter() { + // Build a format string with the given width and specifier. + // For example, if spec is "Y", the format string will be: "|%65636Y|" + let fmt_str = format!("|%{}{}|", width, spec); + + // Allocate a buffer large enough to hold the resulting formatted string. + // We expect the specifier to produce an output shorter than the given width, + // so the result should be padded to exactly `width` characters (inside the quotes). + let mut buf = vec![0u8; width + 2]; // +2 for the surrounding quotes + + let result = TimeFormatter::new(&time, fmt_str.as_bytes()).fmt(&mut buf.as_mut_slice()); + result.expect(&format!("Failed for specifier '{spec}' with width {width}")); + + let output = core::str::from_utf8(&buf).expect("Output not valid UTF-8"); + match &buf[..] { + [b'|', inner @ .., b'|'] => { + assert_eq!( + inner.len(), + width, + "bad len for '{spec}': expected {width}, got {got}", + got = inner.len() + ); + } + _ => panic!("Output not properly quoted for specifier '{spec}': {output}"), + }; + } +} + +#[test] +#[cfg(feature = "alloc")] +fn test_format_specifiers_int_max_fail() { + use alloc::format; + + // Test that using a width equal to INT_MAX (2,147,483,647) causes an error + // (e.g. due to write buffer limits). We use a small output buffer so that + // the formatting attempt cannot succeed. + let specifiers = [ + "Y", "C", "y", "m", "B", "b", "d", "e", "j", "H", "k", "I", "l", "P", "p", "M", "S", "L", + "N", "z", ":z", "::z", ":::z", "Z", "A", "a", "u", "w", "G", "g", "V", "U", "W", "s", "n", + "t", "c", "D", "F", "v", "r", "R", "T", "X", + ]; + let width: usize = i32::MAX as usize; + // A valid and interesting MockTime instance that exercises a wide range of + // specifiers (e.g. year, month, day, time, fractional seconds, week day, + // time zone, etc.): + let time = MockTime::new( + 2021, // year: 2021 (a recent common year) + 12, // month: December + 31, // day: 31st (last day of the year) + 23, // hour: 23 (will yield 11 PM in 12-hour formats) + 59, // minute: 59 + 60, // second: 60 (testing the leap-second edge case, as spec allows 00..=60) + 987654321, // nanoseconds: an interesting fraction for testing %L and %N + 5, // day_of_week: 5 (if 0 = Sunday, then 5 = Friday) + 365, // day_of_year: December 31 is the 365th day in a non-leap year + 1640995200, // to_int: seconds since epoch (an arbitrary value corresponding roughly to 2022-01-01T00:00:00 UTC) + false, // is_utc: false (indicating local time) + 3600, // utc_offset: +3600 seconds (i.e. UTC+1) + "CET", // time_zone: the time zone name (e.g. "CET") + ); + + for spec in specifiers { + let fmt_str = format!("'%{}{}'", width, spec); + // Use a very small buffer to force a write failure. + let mut buf = [0u8; 100]; + let result = TimeFormatter::new(&time, fmt_str.as_bytes()).fmt(&mut &mut buf[..]); + assert!( + matches!(result, Err(Error::WriteZero)), + "Expected an error for specifier '{spec}' with width {width} but got success", + ); + } +} From 5a359a1410cb6bb1c55b0777b09c0322ccb5cd89 Mon Sep 17 00:00:00 2001 From: Ryan Lopopolo Date: Thu, 20 Mar 2025 10:26:31 -0700 Subject: [PATCH 02/10] fixup linter issues, improve docs --- clippy.toml | 4 + src/tests/format.rs | 2 - src/tests/mod.rs | 4 +- src/tests/rust_fmt_argument_max_padding.rs | 302 +++------------------ 4 files changed, 50 insertions(+), 262 deletions(-) create mode 100644 clippy.toml diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 00000000..a0852483 --- /dev/null +++ b/clippy.toml @@ -0,0 +1,4 @@ +doc-valid-idents = [ + "CRuby", + ".." +] diff --git a/src/tests/format.rs b/src/tests/format.rs index 176ea8a5..81e79212 100644 --- a/src/tests/format.rs +++ b/src/tests/format.rs @@ -1,5 +1,3 @@ -#![allow(clippy::should_panic_without_expect)] - use crate::format::TimeFormatter; use crate::Error; diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 5006a19f..0d511c79 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -39,7 +39,7 @@ fn check_all(times: &[MockTime<'_>], format: &str, all_expected: &[&str]) { } #[test] -#[should_panic] +#[should_panic = "assertion `left == right` failed"] #[rustfmt::skip] fn test_check_format_panics_on_error() { let time = MockTime { year: 1111, ..Default::default() }; @@ -48,7 +48,7 @@ fn test_check_format_panics_on_error() { } #[test] -#[should_panic] +#[should_panic = "assertion `left == right` failed"] #[rustfmt::skip] fn test_check_all_panics_on_error() { let times = [ diff --git a/src/tests/rust_fmt_argument_max_padding.rs b/src/tests/rust_fmt_argument_max_padding.rs index 5c6faffc..f52fde2d 100644 --- a/src/tests/rust_fmt_argument_max_padding.rs +++ b/src/tests/rust_fmt_argument_max_padding.rs @@ -1,10 +1,10 @@ //! These tests exist to validate padding behavior when using dynamic widths in //! format strings. As of rust-lang/rust#136932 (part of #99012), the Rust //! standard library now restricts width and precision fields in format strings -//! to u16::MAX, to improve memory layout and prevent silent truncation bugs on -//! cross-compilation targets. This change caused previously valid dynamic width -//! values exceeding u16::MAX to panic at runtime. (See rust-lang/rust#136932 -//! and artichoke/strftime-ruby#2f0ab4a). +//! to `u16::MAX`, to improve memory layout and prevent silent truncation bugs +//! on cross-compilation targets. This change caused previously valid dynamic +//! width values exceeding `u16::MAX` to panic at runtime. (See +//! rust-lang/rust#136932 and artichoke/strftime-ruby#2f0ab4a). //! //! These test cases specifically target edge conditions revealed by fuzzing //! strftime-ruby, ensuring we handle excessively large padding values without @@ -14,7 +14,9 @@ //! Reference: //! //! - -//! - +//! - + +#![allow(clippy::unreadable_literal)] use crate::format::TimeFormatter; use crate::Error; @@ -41,148 +43,36 @@ fn test_larger_than_int_max_formats_are_returned_verbatim() { fn test_rust_136932_reduce_fmt_argument_width_and_precision_fuzzer_failures() { let test_cases: [(MockTime<'_>, &str); 8] = [ ( - MockTime::new( - -184549375, - 93, - 71, - 166, - 14, - 1, - 2788140547, - 166, - 65535, - 376621239690988031, - false, - -1499059802, - "Z\t\u{3}\u{1}9999" - ), + MockTime::new(-184549375, 93, 71, 166, 14, 1, 2788140547, 166, 65535, 376621239690988031, false, -1499059802, "Z\t\u{3}\u{1}9999"), "=99\0\0\0Y%66067c\0\0" ), ( - MockTime::new( - 12255231, - 202, - 175, - 0, - 2, - 0, - 3284320768, - 251, - 0, - -5836660741859442688, - false, - 65538, - "" - ), + MockTime::new(12255231, 202, 175, 0, 2, 0, 3284320768, 251, 0, -5836660741859442688, false, 65538, ""), "\0%666666%%3%" ), ( - MockTime::new( - 1737472, - 0, - 0, - 0, - 0, - 11, - 0, - 0, - 0, - -256, - false, - 64256, - "" - ), + MockTime::new(1737472, 0, 0, 0, 0, 11, 0, 0, 0, -256, false, 64256, ""), "%111111C1111111111111117\u{1}\0X\0XXD" ), ( - MockTime::new( - 3825760, - 0, - 0, - 51, - 0, - 0, - 1044381696, - 37, - 122, - 6584368208339936574, - false, - 0, - "" - ), + MockTime::new(3825760, 0, 0, 51, 0, 0, 1044381696, 37, 122, 6584368208339936574, false, 0, ""), "@%1111111111z\0\0" ), ( - MockTime::new( - 1090397537, - 254, - 255, - 2, - 0, - 0, - 4294639365, - 255, - 65535, - 10455414982377471, - false, - -33540827, - "" - ), + MockTime::new(1090397537, 254, 255, 2, 0, 0, 4294639365, 255, 65535, 10455414982377471, false, -33540827, ""), "%%\n%5555555%55555555555u555" ), ( - MockTime::new( - -21761, - 255, - 58, - 0, - 37, - 37, - 620822272, - 255, - 43690, - 50396945, - false, - 3, - "" - ), + MockTime::new(-21761, 255, 58, 0, 37, 37, 620822272, 255, 43690, 50396945, false, 3, ""), "%R%S%00000400000R%RR%%%\u{3}\0%\0\u{1b}\u{1b}\u{1b}" ), ( - MockTime::new( - 13566191, - 1, - 0, - 34, - 35, - 37, - 623190565, - 0, - 8741, - 2457877026632491810, - true, - 606414171, - "" - ), + MockTime::new(13566191, 1, 0, 34, 35, 37, 623190565, 0, 8741, 2457877026632491810, true, 606414171, ""), "\0\0\0\0\0\0\0\"%2536361%172049423024124\"2\"\"9\"%^^^YN^\"" ), ( - MockTime::new( - 1749024784, - 96, - 0, - 96, - 104, - 96, - 620796776, - 0, - 15968, - 0, - false, - 6307376, - "" - ), - "%u\0\n\0\0\u{10}\0`\u{31b}@\0\0\02>`\0%33333333s33333#33333333333333333333333333333333333\u{13}33333333333u\0\0\0\0\0%u\u{4}\0\0\0\u{10}\0`" + MockTime::new(1749024784, 96, 0, 96, 104, 96, 620796776, 0, 15968, 0, false, 6307376, ""), + "%u\0\n\0\0\u{10}\0`\u{31b}@\0\0\x002>`\0%33333333s33333#33333333333333333333333333333333333\u{13}33333333333u\0\0\0\0\0%u\u{4}\0\0\0\u{10}\0`" ), ]; @@ -193,150 +83,39 @@ fn test_rust_136932_reduce_fmt_argument_width_and_precision_fuzzer_failures() { } #[test] +#[rustfmt::skip] fn test_rust_136932_reduce_fmt_argument_width_and_precision_fuzzer_failures_bytes() { let test_cases: [(MockTime<'_>, &[u8]); 8] = [ ( - MockTime::new( - 674049317, - 244, - 42, - 40, - 180, - 106, - 4096628178, - 1, - 0, - 3239384020795663872, - false, - 1734567725, - "" - ), + MockTime::new(674049317, 244, 42, 40, 180, 106, 4096628178, 1, 0, 3239384020795663872, false, 1734567725, ""), b"\xd6\x00\x00\x00\x00\xda\xc7\xda%\xc1B%-666666F\xff\xff\xff" ), ( - MockTime::new( - 167837442, - 64, - 255, - 10, - 96, - 255, - 4294901760, - 255, - 65535, - -1, - true, - 235864006, - "" - ), + MockTime::new(167837442, 64, 255, 10, 96, 255, 4294901760, 255, 65535, -1, true, 235864006, ""), b"\x88%%%%%%%%%%%%%777777%%%\x0e" ), ( - MockTime::new( - 1375731722, - 82, - 82, - 82, - 85, - 82, - 1392508927, - 82, - 20992, - -48935437123235246, - false, - -1145324613, - "" - ), + MockTime::new(1375731722, 82, 82, 82, 85, 82, 1392508927, 82, 20992, -48935437123235246, false, -1145324613, ""), b"\xbb\xbb%\x00\x00RR%\xbb\xbb\x00\x00\x00B%00124261%R\xff\x00\xbb\xbb\xbb\xbb\x00\x00\x00\x00\x00RRR\xa6RR" ), ( - MockTime::new( - 0, - 0, - 0, - 0, - 92, - 0, - 704643072, - 0, - 242, - 5548434740937171968, - false, - 0, - "" - ), + MockTime::new(0, 0, 0, 0, 92, 0, 704643072, 0, 242, 5548434740937171968, false, 0, ""), b"\x00\x00%7777773%" ), ( - MockTime::new( - 458557, - 0, - 64, - 128, - 128, - 128, - 4278222976, - 255, - 65535, - -205, - true, - 65535, - "\0\0\0" - ), + MockTime::new(458557, 0, 64, 128, 128, 128, 4278222976, 255, 65535, -205, true, 65535, "\0\0\0"), b"\x00%00324248X14\xff" ), ( - MockTime::new( - -774831746, - 209, - 209, - 209, - 13, - 13, - 218959117, - 13, - 3341, - 0, - false, - 218955776, - ":\r\r" - ), + MockTime::new(-774831746, 209, 209, 209, 13, 13, 218959117, 13, 3341, 0, false, 218955776, ":\r\r"), b"\xff\x00\xff\xff\x08%%%%%%%%%%%%%5%%%%3333333%%" ), ( - MockTime::new( - 130826, - 3, - 223, - 29, - 37, - 37, - 623191333, - 93, - 13861, - 2892759177136963579, - true, - 623191333, - "!I\u{1}\0\u{3}" - ), + MockTime::new(130826, 3, 223, 29, 37, 37, 623191333, 93, 13861, 2892759177136963579, true, 623191333, "!I\u{1}\0\u{3}"), b"\xdf\x1d%\x1d?\xff\xdb\xfe#\x15I%$\x00\x00\x00%]%86995%%6\xfb" ), ( - MockTime::new( - 37454602, - 0, - 0, - 0, - 0, - 9, - 1048576, - 255, - 65535, - -1, - false, - 255, - "\0\0\0\0\0/\0" - ), + MockTime::new(37454602, 0, 0, 0, 0, 9, 1048576, 255, 65535, -1, false, 255, "\0\0\0\0\0/\0"), b"y%999999%q)%%z" ), ]; @@ -354,15 +133,19 @@ fn test_format_specifiers_large_width_success() { use alloc::vec; // List of format specifiers that take a width. - // For each, we construct a format string with a width of 65,636. - // The format string is wrapped in single quotes so that we can easily - // strip them and check that the inner formatted result has exactly the given width. + // + // For each, we construct a format string with a width of 131,072. The + // format string is wrapped in single quotes so that we can easily strip + // them and check that the inner formatted result has exactly the given + // width. let specifiers = [ "Y", "C", "y", "m", "B", "b", "d", "e", "j", "H", "k", "I", "l", "P", "p", "M", "S", "L", "N", "z", ":z", "::z", ":::z", "Z", "A", "a", "u", "w", "G", "g", "V", "U", "W", "s", "n", "t", "c", "D", "F", "v", "r", "R", "T", "X", ]; - let width: usize = 65_636; + // Some width greater than `u16::MAX`. + let width = 2 * usize::from(u16::MAX); + // A valid and interesting MockTime instance that exercises a wide range of // specifiers (e.g. year, month, day, time, fractional seconds, week day, // time zone, etc.): @@ -382,10 +165,10 @@ fn test_format_specifiers_large_width_success() { "CET", // time_zone: the time zone name (e.g. "CET") ); - for &spec in specifiers.iter() { + for spec in specifiers { // Build a format string with the given width and specifier. // For example, if spec is "Y", the format string will be: "|%65636Y|" - let fmt_str = format!("|%{}{}|", width, spec); + let fmt_str = format!("|%{width}{spec}|"); // Allocate a buffer large enough to hold the resulting formatted string. // We expect the specifier to produce an output shorter than the given width, @@ -393,7 +176,7 @@ fn test_format_specifiers_large_width_success() { let mut buf = vec![0u8; width + 2]; // +2 for the surrounding quotes let result = TimeFormatter::new(&time, fmt_str.as_bytes()).fmt(&mut buf.as_mut_slice()); - result.expect(&format!("Failed for specifier '{spec}' with width {width}")); + result.unwrap_or_else(|_| panic!("Failed for specifier '{spec}' with width {width}")); let output = core::str::from_utf8(&buf).expect("Output not valid UTF-8"); match &buf[..] { @@ -415,15 +198,18 @@ fn test_format_specifiers_large_width_success() { fn test_format_specifiers_int_max_fail() { use alloc::format; - // Test that using a width equal to INT_MAX (2,147,483,647) causes an error - // (e.g. due to write buffer limits). We use a small output buffer so that - // the formatting attempt cannot succeed. + // List of format specifiers that take a width. + // + // Test that using a width equal to `INT_MAX` (2,147,483,647) causes an + // error (e.g. due to write buffer limits). We use a small output buffer so + // that the formatting attempt cannot succeed. let specifiers = [ "Y", "C", "y", "m", "B", "b", "d", "e", "j", "H", "k", "I", "l", "P", "p", "M", "S", "L", "N", "z", ":z", "::z", ":::z", "Z", "A", "a", "u", "w", "G", "g", "V", "U", "W", "s", "n", "t", "c", "D", "F", "v", "r", "R", "T", "X", ]; - let width: usize = i32::MAX as usize; + let width = usize::try_from(i32::MAX).unwrap(); + // A valid and interesting MockTime instance that exercises a wide range of // specifiers (e.g. year, month, day, time, fractional seconds, week day, // time zone, etc.): @@ -444,7 +230,7 @@ fn test_format_specifiers_int_max_fail() { ); for spec in specifiers { - let fmt_str = format!("'%{}{}'", width, spec); + let fmt_str = format!("'%{width}{spec}'"); // Use a very small buffer to force a write failure. let mut buf = [0u8; 100]; let result = TimeFormatter::new(&time, fmt_str.as_bytes()).fmt(&mut &mut buf[..]); From 6dae31a1aeae6e809167d596a909640526d5e476 Mon Sep 17 00:00:00 2001 From: Ryan Lopopolo Date: Thu, 20 Mar 2025 10:30:44 -0700 Subject: [PATCH 03/10] Ensure the tests compile with no-default-features --- src/tests/rust_fmt_argument_max_padding.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tests/rust_fmt_argument_max_padding.rs b/src/tests/rust_fmt_argument_max_padding.rs index f52fde2d..54973ea0 100644 --- a/src/tests/rust_fmt_argument_max_padding.rs +++ b/src/tests/rust_fmt_argument_max_padding.rs @@ -18,7 +18,12 @@ #![allow(clippy::unreadable_literal)] -use crate::format::TimeFormatter; +#[cfg(feature = "alloc")] +use { + crate::format::TimeFormatter, + alloc::{format, vec}, +}; + use crate::Error; use super::{check_all, get_format_err, get_format_err_bytes, MockTime}; @@ -129,9 +134,6 @@ fn test_rust_136932_reduce_fmt_argument_width_and_precision_fuzzer_failures_byte #[test] #[cfg(feature = "alloc")] fn test_format_specifiers_large_width_success() { - use alloc::format; - use alloc::vec; - // List of format specifiers that take a width. // // For each, we construct a format string with a width of 131,072. The @@ -196,8 +198,6 @@ fn test_format_specifiers_large_width_success() { #[test] #[cfg(feature = "alloc")] fn test_format_specifiers_int_max_fail() { - use alloc::format; - // List of format specifiers that take a width. // // Test that using a width equal to `INT_MAX` (2,147,483,647) causes an From decff389a77e461693d1994e3dfec42997701e27 Mon Sep 17 00:00:00 2001 From: Ryan Lopopolo Date: Thu, 20 Mar 2025 11:55:15 -0700 Subject: [PATCH 04/10] Use a more specific error assert --- src/tests/rust_fmt_argument_max_padding.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/tests/rust_fmt_argument_max_padding.rs b/src/tests/rust_fmt_argument_max_padding.rs index 54973ea0..b1d966f4 100644 --- a/src/tests/rust_fmt_argument_max_padding.rs +++ b/src/tests/rust_fmt_argument_max_padding.rs @@ -233,10 +233,12 @@ fn test_format_specifiers_int_max_fail() { let fmt_str = format!("'%{width}{spec}'"); // Use a very small buffer to force a write failure. let mut buf = [0u8; 100]; - let result = TimeFormatter::new(&time, fmt_str.as_bytes()).fmt(&mut &mut buf[..]); + let err = TimeFormatter::new(&time, fmt_str.as_bytes()) + .fmt(&mut &mut buf[..]) + .unwrap_err(); assert!( - matches!(result, Err(Error::WriteZero)), - "Expected an error for specifier '{spec}' with width {width} but got success", + matches!(err, Error::WriteZero), + "Expected write failure for specifier '{spec}' with width {width} but got unexpected error: {err:?}", ); } } From 5d12a08f4e046e6ee8cd314e71588ff0ba50f7dc Mon Sep 17 00:00:00 2001 From: Ryan Lopopolo Date: Fri, 21 Mar 2025 11:08:58 -0700 Subject: [PATCH 05/10] Update src/tests/rust_fmt_argument_max_padding.rs --- src/tests/rust_fmt_argument_max_padding.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/rust_fmt_argument_max_padding.rs b/src/tests/rust_fmt_argument_max_padding.rs index b1d966f4..9894adde 100644 --- a/src/tests/rust_fmt_argument_max_padding.rs +++ b/src/tests/rust_fmt_argument_max_padding.rs @@ -4,7 +4,7 @@ //! to `u16::MAX`, to improve memory layout and prevent silent truncation bugs //! on cross-compilation targets. This change caused previously valid dynamic //! width values exceeding `u16::MAX` to panic at runtime. (See -//! rust-lang/rust#136932 and artichoke/strftime-ruby#2f0ab4a). +//! rust-lang/rust#136932). //! //! These test cases specifically target edge conditions revealed by fuzzing //! strftime-ruby, ensuring we handle excessively large padding values without From 8f189ba08e94839fbe998db7639744656d590738 Mon Sep 17 00:00:00 2001 From: Ryan Lopopolo Date: Fri, 21 Mar 2025 11:25:55 -0700 Subject: [PATCH 06/10] Remove fuzzer-generated tests for format arg width changes Since the root cause was identified, the additional tests are more exhaustive and clearer to read --- src/tests/rust_fmt_argument_max_padding.rs | 87 ---------------------- 1 file changed, 87 deletions(-) diff --git a/src/tests/rust_fmt_argument_max_padding.rs b/src/tests/rust_fmt_argument_max_padding.rs index 9894adde..056ddece 100644 --- a/src/tests/rust_fmt_argument_max_padding.rs +++ b/src/tests/rust_fmt_argument_max_padding.rs @@ -44,93 +44,6 @@ fn test_larger_than_int_max_formats_are_returned_verbatim() { } } -#[test] -fn test_rust_136932_reduce_fmt_argument_width_and_precision_fuzzer_failures() { - let test_cases: [(MockTime<'_>, &str); 8] = [ - ( - MockTime::new(-184549375, 93, 71, 166, 14, 1, 2788140547, 166, 65535, 376621239690988031, false, -1499059802, "Z\t\u{3}\u{1}9999"), - "=99\0\0\0Y%66067c\0\0" - ), - ( - MockTime::new(12255231, 202, 175, 0, 2, 0, 3284320768, 251, 0, -5836660741859442688, false, 65538, ""), - "\0%666666%%3%" - ), - ( - MockTime::new(1737472, 0, 0, 0, 0, 11, 0, 0, 0, -256, false, 64256, ""), - "%111111C1111111111111117\u{1}\0X\0XXD" - ), - ( - MockTime::new(3825760, 0, 0, 51, 0, 0, 1044381696, 37, 122, 6584368208339936574, false, 0, ""), - "@%1111111111z\0\0" - ), - ( - MockTime::new(1090397537, 254, 255, 2, 0, 0, 4294639365, 255, 65535, 10455414982377471, false, -33540827, ""), - "%%\n%5555555%55555555555u555" - ), - ( - MockTime::new(-21761, 255, 58, 0, 37, 37, 620822272, 255, 43690, 50396945, false, 3, ""), - "%R%S%00000400000R%RR%%%\u{3}\0%\0\u{1b}\u{1b}\u{1b}" - ), - ( - MockTime::new(13566191, 1, 0, 34, 35, 37, 623190565, 0, 8741, 2457877026632491810, true, 606414171, ""), - "\0\0\0\0\0\0\0\"%2536361%172049423024124\"2\"\"9\"%^^^YN^\"" - ), - ( - MockTime::new(1749024784, 96, 0, 96, 104, 96, 620796776, 0, 15968, 0, false, 6307376, ""), - "%u\0\n\0\0\u{10}\0`\u{31b}@\0\0\x002>`\0%33333333s33333#33333333333333333333333333333333333\u{13}33333333333u\0\0\0\0\0%u\u{4}\0\0\0\u{10}\0`" - ), - ]; - - for (time, format) in test_cases { - let err = get_format_err(&time, format); - assert!(matches!(err, Error::WriteZero)); - } -} - -#[test] -#[rustfmt::skip] -fn test_rust_136932_reduce_fmt_argument_width_and_precision_fuzzer_failures_bytes() { - let test_cases: [(MockTime<'_>, &[u8]); 8] = [ - ( - MockTime::new(674049317, 244, 42, 40, 180, 106, 4096628178, 1, 0, 3239384020795663872, false, 1734567725, ""), - b"\xd6\x00\x00\x00\x00\xda\xc7\xda%\xc1B%-666666F\xff\xff\xff" - ), - ( - MockTime::new(167837442, 64, 255, 10, 96, 255, 4294901760, 255, 65535, -1, true, 235864006, ""), - b"\x88%%%%%%%%%%%%%777777%%%\x0e" - ), - ( - MockTime::new(1375731722, 82, 82, 82, 85, 82, 1392508927, 82, 20992, -48935437123235246, false, -1145324613, ""), - b"\xbb\xbb%\x00\x00RR%\xbb\xbb\x00\x00\x00B%00124261%R\xff\x00\xbb\xbb\xbb\xbb\x00\x00\x00\x00\x00RRR\xa6RR" - ), - ( - MockTime::new(0, 0, 0, 0, 92, 0, 704643072, 0, 242, 5548434740937171968, false, 0, ""), - b"\x00\x00%7777773%" - ), - ( - MockTime::new(458557, 0, 64, 128, 128, 128, 4278222976, 255, 65535, -205, true, 65535, "\0\0\0"), - b"\x00%00324248X14\xff" - ), - ( - MockTime::new(-774831746, 209, 209, 209, 13, 13, 218959117, 13, 3341, 0, false, 218955776, ":\r\r"), - b"\xff\x00\xff\xff\x08%%%%%%%%%%%%%5%%%%3333333%%" - ), - ( - MockTime::new(130826, 3, 223, 29, 37, 37, 623191333, 93, 13861, 2892759177136963579, true, 623191333, "!I\u{1}\0\u{3}"), - b"\xdf\x1d%\x1d?\xff\xdb\xfe#\x15I%$\x00\x00\x00%]%86995%%6\xfb" - ), - ( - MockTime::new(37454602, 0, 0, 0, 0, 9, 1048576, 255, 65535, -1, false, 255, "\0\0\0\0\0/\0"), - b"y%999999%q)%%z" - ), - ]; - - for (time, format) in test_cases { - let err = get_format_err_bytes(&time, format); - assert!(matches!(err, Error::WriteZero)); - } -} - #[test] #[cfg(feature = "alloc")] fn test_format_specifiers_large_width_success() { From 3cb8345d297b1e2b37e38bd89f579554cf02bda8 Mon Sep 17 00:00:00 2001 From: x-hgg-x <39058530+x-hgg-x@users.noreply.github.com> Date: Fri, 21 Mar 2025 19:31:40 +0100 Subject: [PATCH 07/10] Update src/tests/rust_fmt_argument_max_padding.rs Co-authored-by: Ryan Lopopolo --- src/tests/rust_fmt_argument_max_padding.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tests/rust_fmt_argument_max_padding.rs b/src/tests/rust_fmt_argument_max_padding.rs index 056ddece..c9415ca6 100644 --- a/src/tests/rust_fmt_argument_max_padding.rs +++ b/src/tests/rust_fmt_argument_max_padding.rs @@ -39,6 +39,7 @@ fn test_larger_than_int_max_formats_are_returned_verbatim() { "%100000000000000000000c", "%1000000000000c", "%10000000000c", + "%2147483648c", // `INT_MAX + 1` ] { check_all(×, format, &[format, format]); } From ed4d803a7b291b78b879566b61feec8c796a964c Mon Sep 17 00:00:00 2001 From: x-hgg-x <39058530+x-hgg-x@users.noreply.github.com> Date: Fri, 21 Mar 2025 20:36:55 +0100 Subject: [PATCH 08/10] Handle large formatting widths by padding directly Co-authored-by: Mara Bos --- src/format/mod.rs | 125 +++++++++++---------- src/format/utils.rs | 22 +++- src/tests/format.rs | 2 +- src/tests/mod.rs | 6 - src/tests/rust_fmt_argument_max_padding.rs | 4 +- 5 files changed, 92 insertions(+), 67 deletions(-) diff --git a/src/format/mod.rs b/src/format/mod.rs index aee6a971..5b1af4d0 100644 --- a/src/format/mod.rs +++ b/src/format/mod.rs @@ -303,10 +303,10 @@ impl Piece { if self.flags.contains(Flag::LeftPadding) { write!(f, "{value}") } else if self.padding == Padding::Spaces { - let width = self.width.unwrap_or(default_width); + let width = self.pad_width(f, b' ', default_width)?; write!(f, "{value: >width$}") } else { - let width = self.width.unwrap_or(default_width); + let width = self.pad_width(f, b'0', default_width)?; write!(f, "{value:0width$}") } } @@ -321,16 +321,24 @@ impl Piece { if self.flags.contains(Flag::LeftPadding) { write!(f, "{value}") } else if self.padding == Padding::Zeros { - let width = self.width.unwrap_or(default_width); + let width = self.pad_width(f, b'0', default_width)?; write!(f, "{value:0width$}") } else { - let width = self.width.unwrap_or(default_width); + let width = self.pad_width(f, b' ', default_width)?; write!(f, "{value: >width$}") } } + /// Returns the width to use for the padding. + /// + /// Prints any excessive padding directly. + fn pad_width(&self, f: &mut SizeLimiter<'_>, pad: u8, default: usize) -> Result { + let width = self.width.unwrap_or(default); + f.pad(pad, width.saturating_sub(u16::MAX.into()))?; + Ok(width.min(u16::MAX.into())) + } + /// Format nanoseconds with the specified precision. - #[allow(clippy::uninlined_format_args)] // for readability and symmetry between if branches fn format_nanoseconds( &self, f: &mut SizeLimiter<'_>, @@ -341,38 +349,37 @@ impl Piece { if width <= 9 { let value = nanoseconds / 10u32.pow(9 - width as u32); - write!(f, "{value:0n$}", n = width) + write!(f, "{value:0width$}") } else { - write!(f, "{nanoseconds:09}{:0n$}", 0, n = width - 9) + write!(f, "{nanoseconds:09}")?; + f.pad(b'0', width - 9) } } /// Format a string value. fn format_string(&self, f: &mut SizeLimiter<'_>, s: &str) -> Result<(), Error> { - match self.width { - None => write!(f, "{s}"), - Some(width) => { - if self.flags.contains(Flag::LeftPadding) { - write!(f, "{s}") - } else if self.padding == Padding::Zeros { - write!(f, "{s:0>width$}") - } else { - write!(f, "{s: >width$}") - } - } + if !self.flags.contains(Flag::LeftPadding) { + self.write_padding(f, s.len())?; } + + write!(f, "{s}") } /// Write padding separately. fn write_padding(&self, f: &mut SizeLimiter<'_>, min_width: usize) -> Result<(), Error> { - if let Some(width) = self.width { - let n = width.saturating_sub(min_width); + let Some(width) = self.width else { + return Ok(()); + }; + + let n = width.saturating_sub(min_width); + + let pad = match self.padding { + Padding::Zeros => b'0', + _ => b' ', + }; + + f.pad(pad, n)?; - match self.padding { - Padding::Zeros => write!(f, "{:0>n$}", "")?, - _ => write!(f, "{: >n$}", "")?, - }; - } Ok(()) } @@ -396,14 +403,34 @@ impl Piece { UtcOffset::new(hour, minute, second) } - /// Compute hour padding for the `%z` specifier. - fn hour_padding(&self, min_width: usize) -> usize { - const MIN_PADDING: usize = "+hh".len(); + /// Write the hour sign. + fn write_hour_sign(f: &mut SizeLimiter<'_>, hour: f64) -> Result<(), Error> { + if hour.is_sign_negative() { + write!(f, "-")?; + } else { + write!(f, "+")?; + } + + Ok(()) + } - match self.width { - Some(width) => width.saturating_sub(min_width) + MIN_PADDING, - None => MIN_PADDING, + /// Write the hour with padding for the `%z` specifier. + fn write_offset_hour(&self, f: &mut SizeLimiter<'_>, hour: f64, w: usize) -> Result<(), Error> { + let mut pad = self.width.unwrap_or(0).saturating_sub(w); + + if hour < 10.0 { + pad += 1; + } + + if self.padding == Padding::Spaces { + f.pad(b' ', pad)?; + Self::write_hour_sign(f, hour)?; + } else { + Self::write_hour_sign(f, hour)?; + f.pad(b'0', pad)?; } + + write!(f, "{:.0}", hour.abs()) } /// Write the time zone UTC offset as `"+hh"`. @@ -412,13 +439,7 @@ impl Piece { f: &mut SizeLimiter<'_>, utc_offset: &UtcOffset, ) -> Result<(), Error> { - let hour = utc_offset.hour; - let n = self.hour_padding("+hh".len()); - - match self.padding { - Padding::Spaces => write!(f, "{hour: >+n$.0}"), - _ => write!(f, "{hour:+0n$.0}"), - } + self.write_offset_hour(f, utc_offset.hour, "+hh".len()) } /// Write the time zone UTC offset as `"+hhmm"`. @@ -427,13 +448,10 @@ impl Piece { f: &mut SizeLimiter<'_>, utc_offset: &UtcOffset, ) -> Result<(), Error> { - let UtcOffset { hour, minute, .. } = utc_offset; - let n = self.hour_padding("+hhmm".len()); + let UtcOffset { hour, minute, .. } = *utc_offset; - match self.padding { - Padding::Spaces => write!(f, "{hour: >+n$.0}{minute:02}"), - _ => write!(f, "{hour:+0n$.0}{minute:02}"), - } + self.write_offset_hour(f, hour, "+hhmm".len())?; + write!(f, "{minute:02}") } /// Write the time zone UTC offset as `"+hh:mm"`. @@ -442,13 +460,10 @@ impl Piece { f: &mut SizeLimiter<'_>, utc_offset: &UtcOffset, ) -> Result<(), Error> { - let UtcOffset { hour, minute, .. } = utc_offset; - let n = self.hour_padding("+hh:mm".len()); + let UtcOffset { hour, minute, .. } = *utc_offset; - match self.padding { - Padding::Spaces => write!(f, "{hour: >+n$.0}:{minute:02}"), - _ => write!(f, "{hour:+0n$.0}:{minute:02}"), - } + self.write_offset_hour(f, hour, "+hh:mm".len())?; + write!(f, ":{minute:02}") } /// Write the time zone UTC offset as `"+hh:mm:ss"`. @@ -461,14 +476,10 @@ impl Piece { hour, minute, second, - } = utc_offset; - - let n = self.hour_padding("+hh:mm:ss".len()); + } = *utc_offset; - match self.padding { - Padding::Spaces => write!(f, "{hour: >+n$.0}:{minute:02}:{second:02}"), - _ => write!(f, "{hour:+0n$.0}:{minute:02}:{second:02}"), - } + self.write_offset_hour(f, hour, "+hh:mm:ss".len())?; + write!(f, ":{minute:02}:{second:02}") } /// Format time using the formatting directive. diff --git a/src/format/utils.rs b/src/format/utils.rs index 61481add..2ac3771b 100644 --- a/src/format/utils.rs +++ b/src/format/utils.rs @@ -81,11 +81,31 @@ impl<'a> SizeLimiter<'a> { count: 0, } } + + /// Pad with the specified character. + pub(crate) fn pad(&mut self, c: u8, n: usize) -> Result<(), Error> { + if self.count.saturating_add(n) > self.size_limit { + return Err(Error::FormattedStringTooLarge); + } + + let buffer = [c; 1024]; + let mut remaining = n; + + while remaining > 0 { + let size = remaining.min(1024); + self.inner.write_all(&buffer[..size])?; + remaining -= size; + } + + self.count += n; + + Ok(()) + } } impl Write for SizeLimiter<'_> { fn write(&mut self, buf: &[u8]) -> Result { - if self.count + buf.len() > self.size_limit { + if self.count.saturating_add(buf.len()) > self.size_limit { return Err(Error::FormattedStringTooLarge); } diff --git a/src/tests/format.rs b/src/tests/format.rs index 81e79212..a988cc50 100644 --- a/src/tests/format.rs +++ b/src/tests/format.rs @@ -825,7 +825,7 @@ fn test_format_large_width() { check_format(&time, "%-100000000m", "1"); check_format(&time, "%2147483648m", "%2147483648m"); - let err = get_format_err(&time, "%2147483647m"); + let err = get_format_err(&time, "%1000m"); assert!(matches!(err, Error::WriteZero)); } diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 0d511c79..b9c62d20 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -13,12 +13,6 @@ fn get_format_err(time: &MockTime<'_>, format: &str) -> Error { .unwrap_err() } -fn get_format_err_bytes(time: &MockTime<'_>, format: &[u8]) -> Error { - TimeFormatter::new(time, format) - .fmt(&mut &mut [0u8; 100][..]) - .unwrap_err() -} - fn check_format(time: &MockTime<'_>, format: &str, expected: &str) { const SIZE: usize = 100; let mut buf = [0u8; SIZE]; diff --git a/src/tests/rust_fmt_argument_max_padding.rs b/src/tests/rust_fmt_argument_max_padding.rs index c9415ca6..4dbc39ce 100644 --- a/src/tests/rust_fmt_argument_max_padding.rs +++ b/src/tests/rust_fmt_argument_max_padding.rs @@ -26,7 +26,7 @@ use { use crate::Error; -use super::{check_all, get_format_err, get_format_err_bytes, MockTime}; +use super::{check_all, MockTime}; #[test] fn test_larger_than_int_max_formats_are_returned_verbatim() { @@ -151,7 +151,7 @@ fn test_format_specifiers_int_max_fail() { .fmt(&mut &mut buf[..]) .unwrap_err(); assert!( - matches!(err, Error::WriteZero), + matches!(err, Error::FormattedStringTooLarge), "Expected write failure for specifier '{spec}' with width {width} but got unexpected error: {err:?}", ); } From 5330aec1c26d8bd423912622be496bd75ca3bcdd Mon Sep 17 00:00:00 2001 From: Ryan Lopopolo Date: Fri, 21 Mar 2025 15:27:18 -0700 Subject: [PATCH 09/10] Fixup unused import error with no-default-features --- src/tests/rust_fmt_argument_max_padding.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tests/rust_fmt_argument_max_padding.rs b/src/tests/rust_fmt_argument_max_padding.rs index 4dbc39ce..223d427d 100644 --- a/src/tests/rust_fmt_argument_max_padding.rs +++ b/src/tests/rust_fmt_argument_max_padding.rs @@ -21,11 +21,10 @@ #[cfg(feature = "alloc")] use { crate::format::TimeFormatter, + crate::Error, alloc::{format, vec}, }; -use crate::Error; - use super::{check_all, MockTime}; #[test] From 0be636c640207d3cc8bf5008037d606ff5cbddcd Mon Sep 17 00:00:00 2001 From: Ryan Lopopolo Date: Fri, 21 Mar 2025 15:38:52 -0700 Subject: [PATCH 10/10] Raise MSRV to Rust 1.84.0, prep v1.2.0 release `f64::abs` only recently became available in `no_std` crates: - https://github.com/rust-lang/rust/pull/131304 --- .github/workflows/ci.yaml | 2 +- Cargo.toml | 12 +++++++++--- README.md | 4 ++-- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 7b672091..ff15fe19 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -73,7 +73,7 @@ jobs: - name: Install Rust toolchain uses: artichoke/setup-rust/build-and-test@v2.0.1 with: - toolchain: "1.76.0" + toolchain: "1.84.0" - name: Compile run: cargo build --verbose diff --git a/Cargo.toml b/Cargo.toml index 883de7ac..5f6e343a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,18 +1,24 @@ [package] name = "strftime-ruby" # remember to set `html_root_url` in `src/lib.rs`. -version = "1.1.0" +version = "1.2.0" authors = ["Ryan Lopopolo ", "x-hgg-x"] license = "MIT" edition = "2021" -rust-version = "1.76.0" +rust-version = "1.84.0" readme = "README.md" repository = "https://github.com/artichoke/strftime-ruby" documentation = "https://docs.rs/strftime-ruby" homepage = "https://github.com/artichoke/strftime-ruby" description = "Ruby `Time#strftime` parser and formatter" keywords = ["ruby", "strftime", "time"] -categories = ["date-and-time", "no-std", "no-std::no-alloc", "parser-implementations", "value-formatting"] +categories = [ + "date-and-time", + "no-std", + "no-std::no-alloc", + "parser-implementations", + "value-formatting", +] include = ["/src/**/*", "/tests/**/*", "/LICENSE", "/README.md"] [lib] diff --git a/README.md b/README.md index e2fa5fbe..9cacf3b8 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,7 @@ Add this to your `Cargo.toml`: ```toml [dependencies] -strftime-ruby = "1.1.0" +strftime-ruby = "1.2.0" ``` ## Crate features @@ -64,7 +64,7 @@ All features are enabled by default. ### Minimum Supported Rust Version -This crate requires at least Rust 1.76.0. This version can be bumped in minor +This crate requires at least Rust 1.84.0. This version can be bumped in minor releases. ## License