time-rs / time

The most used Rust library for date and time handling.
https://time-rs.github.io
Apache License 2.0
1.13k stars 281 forks source link

Overflows on range checking in v0.3.24 #606

Closed vthib closed 1 year ago

vthib commented 1 year ago

Trying to update to v0.3.24 lead to some overflows in my tests

I basically had a test that ended up doing a .replace_milliseconds() call on a OffsetDateTime with different values. Notably, out of range values, ensuring the call returns Err.

For example:

let date = time::OffsetDateTime::now_utc();
assert!(date.replace_millisecond(9999).is_err());

This passes in v0.3.23, but gives this on v0.3.24 (in debug profile):

thread 'main' panicked at 'attempt to multiply with overflow', /home/vthib/.cargo/registry/src/index.crates.io-6f17d22bba15001f/time-0.3.24/src/time.rs:663:13

Afaict, this is a regression caused by 7e95cb2, where the milliseconds are normalized as nanoseconds, which overflows u32::MAX, before doing the range checking.

I also tested wrapping after the overflows. For example:

date.replace_millisecond(4294); // returns Err
date.replace_millisecond(4295); // returns Ok

But this was apparently also the case in 0.3.23.

jhpratt commented 1 year ago

Thanks for the report. It's definitely a bug that was introduced in the latest release. That's the unfortunate side of reworking the internals. I'll look into this in the near future.

jhpratt commented 1 year ago

Fixed on main.