fix: underflow during datetime->nanos conversion

crepererum · pitdicker · commit 2afdde8f7f23 · 2023-09-15T14:51:10.000+02:00
Fixes #1289.
diff --git a/src/datetime/tests.rs b/src/datetime/tests.rs
@@ -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);
+    }
+}
diff --git a/src/naive/datetime/mod.rs b/src/naive/datetime/mod.rs
@@ -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.
