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 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/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 a780f8a5..a988cc50 100644 --- a/src/tests/format.rs +++ b/src/tests/format.rs @@ -1,57 +1,7 @@ -#![allow(clippy::should_panic_without_expect)] - 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 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'"); -} +use crate::Error; -#[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] @@ -875,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 4c993e03..b9c62d20 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -1,2 +1,56 @@ 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 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 = "assertion `left == right` failed"] +#[rustfmt::skip] +fn test_check_format_panics_on_error() { + let time = MockTime { year: 1111, ..Default::default() }; + + check_format(&time, "'%Y'", "'1112'"); +} + +#[test] +#[should_panic = "assertion `left == right` failed"] +#[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..223d427d --- /dev/null +++ b/src/tests/rust_fmt_argument_max_padding.rs @@ -0,0 +1,157 @@ +//! 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). +//! +//! 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: +//! +//! - +//! - + +#![allow(clippy::unreadable_literal)] + +#[cfg(feature = "alloc")] +use { + crate::format::TimeFormatter, + crate::Error, + alloc::{format, vec}, +}; + +use super::{check_all, 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", + "%2147483648c", // `INT_MAX + 1` + ] { + check_all(×, format, &[format, format]); + } +} + +#[test] +#[cfg(feature = "alloc")] +fn test_format_specifiers_large_width_success() { + // List of format specifiers that take a 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", + ]; + // 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.): + 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 { + // 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.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[..] { + [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() { + // 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::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.): + 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 err = TimeFormatter::new(&time, fmt_str.as_bytes()) + .fmt(&mut &mut buf[..]) + .unwrap_err(); + assert!( + matches!(err, Error::FormattedStringTooLarge), + "Expected write failure for specifier '{spec}' with width {width} but got unexpected error: {err:?}", + ); + } +}