Skip to content

Commit

Permalink
fix: underflow during datetime->nanos conversion
Browse files Browse the repository at this point in the history
Fixes #1289.
  • Loading branch information
crepererum authored and pitdicker committed Sep 15, 2023
1 parent 46ad2c2 commit 2afdde8
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 1 deletion.
33 changes: 33 additions & 0 deletions src/datetime/tests.rs
Expand Up @@ -1486,3 +1486,36 @@ fn locale_decimal_point() {
assert_eq!(dt.format_localized("%T%.6f", ar_SY).to_string(), "18:58:00.123456");
assert_eq!(dt.format_localized("%T%.9f", ar_SY).to_string(), "18:58:00.123456780");
}

/// This is an extended test for <https://github.com/chronotope/chrono/issues/1289>.
#[test]
fn nano_roundrip() {
const BILLION: i64 = 1_000_000_000;

for nanos in [
i64::MIN,
i64::MIN + 1,
i64::MIN + 2,
i64::MIN + BILLION - 1,
i64::MIN + BILLION,
i64::MIN + BILLION + 1,
-BILLION - 1,
-BILLION,
-BILLION + 1,
0,
BILLION - 1,
BILLION,
BILLION + 1,
i64::MAX - BILLION - 1,
i64::MAX - BILLION,
i64::MAX - BILLION + 1,
i64::MAX - 2,
i64::MAX - 1,
i64::MAX,
] {
println!("nanos: {}", nanos);
let dt = Utc.timestamp_nanos(nanos);
let nanos2 = dt.timestamp_nanos_opt().expect("value roundtrips");
assert_eq!(nanos, nanos2);
}
}
21 changes: 20 additions & 1 deletion src/naive/datetime/mod.rs
Expand Up @@ -503,7 +503,26 @@ impl NaiveDateTime {
#[inline]
#[must_use]
pub fn timestamp_nanos_opt(&self) -> Option<i64> {
self.timestamp().checked_mul(1_000_000_000)?.checked_add(self.time.nanosecond() as i64)
let mut timestamp = self.timestamp();
let mut timestamp_subsec_nanos = i64::from(self.timestamp_subsec_nanos());

// subsec nanos are always non-negative, however the timestamp itself (both in seconds and in nanos) can be
// negative. Now i64::MIN is NOT dividable by 1_000_000_000, so
//
// (timestamp * 1_000_000_000) + nanos
//
// may underflow (even when in theory we COULD represent the datetime as i64) because we add the non-negative
// nanos AFTER the multiplication. This is fixed by converting the negative case to
//
// ((timestamp + 1) * 1_000_000_000) + (ns - 1_000_000_000)
//
// Also see <https://github.com/chronotope/chrono/issues/1289>.
if timestamp < 0 && timestamp_subsec_nanos > 0 {
timestamp_subsec_nanos -= 1_000_000_000;
timestamp += 1;
}

timestamp.checked_mul(1_000_000_000).and_then(|ns| ns.checked_add(timestamp_subsec_nanos))
}

/// Returns the number of milliseconds since the last whole non-leap second.
Expand Down

0 comments on commit 2afdde8

Please sign in to comment.