signed integer overflow in `Duration` constructors

[lopopolo]

Duration has several constructors that perform unchecked signed multiplication, which can result in panics in debug mode and unexpected wrapping in release.

I don't have a need to these inputs into Duration constructors but I found this unexpected given the pervasiveness of checked operations in the public API.

Playground link: https://play.rust-lang.org/?version=stable&mode=release&edition=2021&gist=731f151589796275ac9c7678ec43a709

Code:

extern crate time; // 0.3.14

use time::Duration;

fn main() {
    dbg!(Duration::weeks(i64::MAX));
    dbg!(Duration::days(i64::MAX));
    dbg!(Duration::hours(i64::MAX));
    dbg!(Duration::minutes(i64::MAX));

    dbg!(Duration::weeks(i64::MIN));
    dbg!(Duration::days(i64::MIN));
    dbg!(Duration::hours(i64::MIN));
    dbg!(Duration::minutes(i64::MIN));
}

Output:

   Compiling playground v0.0.1 (/playground)
    Finished release [optimized] target(s) in 0.60s
     Running `target/release/playground`
[src/main.rs:6] Duration::weeks(i64::MAX) = Duration {
    seconds: -604800,
    nanoseconds: 0,
}
[src/main.rs:7] Duration::days(i64::MAX) = Duration {
    seconds: -86400,
    nanoseconds: 0,
}
[src/main.rs:8] Duration::hours(i64::MAX) = Duration {
    seconds: -3600,
    nanoseconds: 0,
}
[src/main.rs:9] Duration::minutes(i64::MAX) = Duration {
    seconds: -60,
    nanoseconds: 0,
}
[src/main.rs:11] Duration::weeks(i64::MIN) = Duration {
    seconds: 0,
    nanoseconds: 0,
}
[src/main.rs:12] Duration::days(i64::MIN) = Duration {
    seconds: 0,
    nanoseconds: 0,
}
[src/main.rs:13] Duration::hours(i64::MIN) = Duration {
    seconds: 0,
    nanoseconds: 0,
}
[src/main.rs:14] Duration::minutes(i64::MIN) = Duration {
    seconds: 0,
    nanoseconds: 0,
}

[jhpratt]

This is certainly not the intended behavior. I'll add overflow checks to the necessary methods. The reason I did not originally opt for returning a Result is because the time spans are beyond large (as in longer than the age of the universe), so there is no reason anyone should even be remotely close to the bound.

[jhpratt]

Fixed on main.

[lopopolo]

Thanks for merging a fix @jhpratt 💯

I took a peek at the commit and left a few comments / questions: https://github.com/time-rs/time/commit/5efea65d2e50c09e3990332511aff317fd42747e

I think there is still some unexpected behavior in these constructors around NAN and infinite floats.

[Cheban1996]

How about use u64 instead of i64?

[jhpratt]

@Cheban1996 That wouldn't change anything, not to mention that Duration is a signed type.