Skip to content

Commit

Permalink
Eliminate overflow in Duration constructors
Browse files Browse the repository at this point in the history
  • Loading branch information
jhpratt committed Sep 25, 2022
1 parent d0bab6a commit 8cb9c44
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 17 deletions.
45 changes: 36 additions & 9 deletions src/duration.rs
Expand Up @@ -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;
}
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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 _)
}

Expand All @@ -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 _)
}

Expand Down Expand Up @@ -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

Expand Down
20 changes: 20 additions & 0 deletions src/lib.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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<T> = core::result::Result<T, Error>;

/// 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)
}
24 changes: 24 additions & 0 deletions tests/integration/duration.rs
Expand Up @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand Down Expand Up @@ -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]
Expand All @@ -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]
Expand Down Expand Up @@ -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]
Expand Down
10 changes: 10 additions & 0 deletions tests/integration/main.rs
Expand Up @@ -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;
Expand Down
10 changes: 2 additions & 8 deletions tests/integration/offset_date_time.rs
Expand Up @@ -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]
Expand Down

0 comments on commit 8cb9c44

Please sign in to comment.