From 8cb9c44375f243df4b994844b2085ea9e2a292e1 Mon Sep 17 00:00:00 2001 From: Jacob Pratt Date: Sun, 25 Sep 2022 01:44:36 -0400 Subject: [PATCH] Eliminate overflow in `Duration` constructors --- src/duration.rs | 45 +++++++++++++++++++++------ src/lib.rs | 20 ++++++++++++ tests/integration/duration.rs | 24 ++++++++++++++ tests/integration/main.rs | 10 ++++++ tests/integration/offset_date_time.rs | 10 ++---- 5 files changed, 92 insertions(+), 17 deletions(-) diff --git a/src/duration.rs b/src/duration.rs index 59f55722a..4499c7b8e 100644 --- a/src/duration.rs +++ b/src/duration.rs @@ -227,13 +227,18 @@ impl Duration { /// assert_eq!(Duration::new(1, 2_000_000_000), 3.seconds()); /// ``` pub const fn new(mut seconds: i64, mut nanoseconds: i32) -> Self { - seconds += nanoseconds as i64 / 1_000_000_000; + seconds = expect_opt!( + seconds.checked_add(nanoseconds as i64 / 1_000_000_000), + "overflow constructing `time::Duration`" + ); nanoseconds %= 1_000_000_000; if seconds > 0 && nanoseconds < 0 { + // `seconds` cannot overflow here because it is positive. seconds -= 1; nanoseconds += 1_000_000_000; } else if seconds < 0 && nanoseconds > 0 { + // `seconds` cannot overflow here because it is negative. seconds += 1; nanoseconds -= 1_000_000_000; } @@ -249,7 +254,10 @@ impl Duration { /// assert_eq!(Duration::weeks(1), 604_800.seconds()); /// ``` pub const fn weeks(weeks: i64) -> Self { - Self::seconds(weeks * 604_800) + Self::seconds(expect_opt!( + weeks.checked_mul(604_800), + "overflow constructing `time::Duration`" + )) } /// Create a new `Duration` with the given number of days. Equivalent to @@ -260,7 +268,10 @@ impl Duration { /// assert_eq!(Duration::days(1), 86_400.seconds()); /// ``` pub const fn days(days: i64) -> Self { - Self::seconds(days * 86_400) + Self::seconds(expect_opt!( + days.checked_mul(86_400), + "overflow constructing `time::Duration`" + )) } /// Create a new `Duration` with the given number of hours. Equivalent to @@ -271,7 +282,10 @@ impl Duration { /// assert_eq!(Duration::hours(1), 3_600.seconds()); /// ``` pub const fn hours(hours: i64) -> Self { - Self::seconds(hours * 3_600) + Self::seconds(expect_opt!( + hours.checked_mul(3_600), + "overflow constructing `time::Duration`" + )) } /// Create a new `Duration` with the given number of minutes. Equivalent to @@ -282,7 +296,10 @@ impl Duration { /// assert_eq!(Duration::minutes(1), 60.seconds()); /// ``` pub const fn minutes(minutes: i64) -> Self { - Self::seconds(minutes * 60) + Self::seconds(expect_opt!( + minutes.checked_mul(60), + "overflow constructing `time::Duration`" + )) } /// Create a new `Duration` with the given number of seconds. @@ -303,6 +320,9 @@ impl Duration { /// assert_eq!(Duration::seconds_f64(-0.5), -0.5.seconds()); /// ``` pub fn seconds_f64(seconds: f64) -> Self { + if seconds > i64::MAX as f64 || seconds < i64::MIN as f64 { + crate::expect_failed("overflow constructing `time::Duration`"); + } Self::new_unchecked(seconds as _, ((seconds % 1.) * 1_000_000_000.) as _) } @@ -314,6 +334,9 @@ impl Duration { /// assert_eq!(Duration::seconds_f32(-0.5), (-0.5).seconds()); /// ``` pub fn seconds_f32(seconds: f32) -> Self { + if seconds > i64::MAX as f32 || seconds < i64::MIN as f32 { + crate::expect_failed("overflow constructing `time::Duration`"); + } Self::new_unchecked(seconds as _, ((seconds % 1.) * 1_000_000_000.) as _) } @@ -364,10 +387,14 @@ impl Duration { /// As the input range cannot be fully mapped to the output, this should only be used where it's /// known to result in a valid value. pub(crate) const fn nanoseconds_i128(nanoseconds: i128) -> Self { - Self::new_unchecked( - (nanoseconds / 1_000_000_000) as _, - (nanoseconds % 1_000_000_000) as _, - ) + let seconds = nanoseconds / 1_000_000_000; + let nanoseconds = nanoseconds % 1_000_000_000; + + if seconds > i64::MAX as i128 || seconds < i64::MIN as i128 { + crate::expect_failed("overflow constructing `time::Duration`"); + } + + Self::new_unchecked(seconds as _, nanoseconds as _) } // endregion constructors diff --git a/src/lib.rs b/src/lib.rs index 0583b1668..b13a7df2b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -283,6 +283,18 @@ macro_rules! const_try_opt { } }; } + +/// Try to unwrap an expression, panicking if not possible. +/// +/// This is similar to `$e.expect($message)`, but is usable in `const` contexts. +macro_rules! expect_opt { + ($e:expr, $message:literal) => { + match $e { + Some(value) => value, + None => crate::expect_failed($message), + } + }; +} // endregion macros mod date; @@ -334,3 +346,11 @@ pub use crate::weekday::Weekday; /// An alias for [`std::result::Result`] with a generic error from the time crate. pub type Result = core::result::Result; + +/// This is a separate function to reduce the code size of `expect_opt!`. +#[inline(never)] +#[cold] +#[track_caller] +const fn expect_failed(message: &str) -> ! { + panic!("{}", message) +} diff --git a/tests/integration/duration.rs b/tests/integration/duration.rs index 66a2f3e17..285db91aa 100644 --- a/tests/integration/duration.rs +++ b/tests/integration/duration.rs @@ -82,6 +82,9 @@ fn new() { assert_eq!(Duration::new(1, -1_400_000_000), (-400).milliseconds()); assert_eq!(Duration::new(2, -1_400_000_000), 600.milliseconds()); assert_eq!(Duration::new(3, -1_400_000_000), 1_600.milliseconds()); + + assert_panic!(Duration::new(i64::MAX, 1_000_000_000)); + assert_panic!(Duration::new(i64::MIN, -1_000_000_000)); } #[test] @@ -90,6 +93,9 @@ fn weeks() { assert_eq!(Duration::weeks(2), (2 * 604_800).seconds()); assert_eq!(Duration::weeks(-1), (-604_800).seconds()); assert_eq!(Duration::weeks(-2), (2 * -604_800).seconds()); + + assert_panic!(Duration::weeks(i64::MAX)); + assert_panic!(Duration::weeks(i64::MIN)); } #[test] @@ -106,6 +112,9 @@ fn days() { assert_eq!(Duration::days(2), (2 * 86_400).seconds()); assert_eq!(Duration::days(-1), (-86_400).seconds()); assert_eq!(Duration::days(-2), (2 * -86_400).seconds()); + + assert_panic!(Duration::days(i64::MAX)); + assert_panic!(Duration::days(i64::MIN)); } #[test] @@ -122,6 +131,9 @@ fn hours() { assert_eq!(Duration::hours(2), (2 * 3_600).seconds()); assert_eq!(Duration::hours(-1), (-3_600).seconds()); assert_eq!(Duration::hours(-2), (2 * -3_600).seconds()); + + assert_panic!(Duration::hours(i64::MAX)); + assert_panic!(Duration::hours(i64::MIN)); } #[test] @@ -138,6 +150,9 @@ fn minutes() { assert_eq!(Duration::minutes(2), (2 * 60).seconds()); assert_eq!(Duration::minutes(-1), (-60).seconds()); assert_eq!(Duration::minutes(-2), (2 * -60).seconds()); + + assert_panic!(Duration::minutes(i64::MAX)); + assert_panic!(Duration::minutes(i64::MIN)); } #[test] @@ -168,6 +183,9 @@ fn whole_seconds() { fn seconds_f64() { assert_eq!(Duration::seconds_f64(0.5), 0.5.seconds()); assert_eq!(Duration::seconds_f64(-0.5), (-0.5).seconds()); + + assert_panic!(Duration::seconds_f64(f64::MAX)); + assert_panic!(Duration::seconds_f64(f64::MIN)); } #[test] @@ -185,6 +203,9 @@ fn as_seconds_f64() { fn seconds_f32() { assert_eq!(Duration::seconds_f32(0.5), 0.5.seconds()); assert_eq!(Duration::seconds_f32(-0.5), (-0.5).seconds()); + + assert_panic!(Duration::seconds_f32(f32::MAX)); + assert_panic!(Duration::seconds_f32(f32::MIN)); } #[test] @@ -600,6 +621,9 @@ fn std_sub_assign_overflow() { fn mul_int() { assert_eq!(1.seconds() * 2, 2.seconds()); assert_eq!(1.seconds() * -2, (-2).seconds()); + + assert_panic!(Duration::MAX * 2); + assert_panic!(Duration::MIN * 2); } #[test] diff --git a/tests/integration/main.rs b/tests/integration/main.rs index c30534788..7a0dcd2e1 100644 --- a/tests/integration/main.rs +++ b/tests/integration/main.rs @@ -71,6 +71,16 @@ macro_rules! modifier { (@value $field:ident $value:expr) => ($value); } +/// Assert that the given expression panics. +macro_rules! assert_panic { + ($($x:tt)*) => { + assert!(std::panic::catch_unwind(|| { + $($x)* + }) + .is_err()) + } +} + mod date; mod derives; mod duration; diff --git a/tests/integration/offset_date_time.rs b/tests/integration/offset_date_time.rs index 73baa9459..c6af8933e 100644 --- a/tests/integration/offset_date_time.rs +++ b/tests/integration/offset_date_time.rs @@ -51,14 +51,8 @@ fn to_offset() { #[test] fn to_offset_panic() { - assert!( - std::panic::catch_unwind(|| { PrimitiveDateTime::MAX.assume_utc().to_offset(offset!(+1)) }) - .is_err() - ); - assert!( - std::panic::catch_unwind(|| { PrimitiveDateTime::MIN.assume_utc().to_offset(offset!(-1)) }) - .is_err() - ); + assert_panic!(PrimitiveDateTime::MAX.assume_utc().to_offset(offset!(+1))); + assert_panic!(PrimitiveDateTime::MIN.assume_utc().to_offset(offset!(-1))); } #[test]