fix: assure writing invalid dates doesn't panic (#1367).

Author: Byron

gix-date/src/time/write.rs

@@ -5,6 +5,10 @@ use crate::{time::Sign, Time};
 /// Serialization with standard `git` format
 impl Time {
     /// Serialize this instance into memory, similar to what [`write_to()`][Self::write_to()] would do with arbitrary `Write` implementations.
+    ///
+    /// # Panics
+    ///
+    /// If the underlying call fails as this instance can't be represented, typically due to an invalid offset.
     pub fn to_bstring(&self) -> BString {
         let mut buf = Vec::with_capacity(64);
         self.write_to(&mut buf).expect("write to memory cannot fail");
@@ -13,6 +17,18 @@ impl Time {
 
     /// Serialize this instance to `out` in a format suitable for use in header fields of serialized git commits or tags.
     pub fn write_to(&self, out: &mut dyn std::io::Write) -> std::io::Result<()> {
+        const SECONDS_PER_HOUR: u32 = 60 * 60;
+        let offset = self.offset.unsigned_abs();
+        let hours = offset / SECONDS_PER_HOUR;
+        let minutes = (offset - (hours * SECONDS_PER_HOUR)) / 60;
+
+        if hours > 99 {
+            return Err(std::io::Error::new(
+                std::io::ErrorKind::Other,
+                "Cannot represent offsets larger than +-9900",
+            ));
+        }
+
         let mut itoa = itoa::Buffer::new();
         out.write_all(itoa.format(self.seconds).as_bytes())?;
         out.write_all(b" ")?;
@@ -23,12 +39,6 @@ impl Time {
 
         const ZERO: &[u8; 1] = b"0";
 
-        const SECONDS_PER_HOUR: u32 = 60 * 60;
-        let offset = self.offset.unsigned_abs();
-        let hours = offset / SECONDS_PER_HOUR;
-        assert!(hours < 25, "offset is more than a day: {hours}");
-        let minutes = (offset - (hours * SECONDS_PER_HOUR)) / 60;
-
         if hours < 10 {
             out.write_all(ZERO)?;
         }

gix-date/tests/time/mod.rs

@@ -1,5 +1,4 @@
-use bstr::ByteSlice;
-use gix_date::{time::Sign, SecondsSinceUnixEpoch, Time};
+use gix_date::Time;
 
 mod baseline;
 mod format;
@@ -32,57 +31,114 @@ fn is_set() {
     .is_set());
 }
 
-#[test]
-fn write_to() -> Result<(), Box<dyn std::error::Error>> {
-    for (time, expected) in [
-        (
-            Time {
-                seconds: SecondsSinceUnixEpoch::MAX,
-                offset: 0,
-                sign: Sign::Minus,
-            },
-            "9223372036854775807 -0000",
-        ),
-        (
-            Time {
-                seconds: SecondsSinceUnixEpoch::MIN,
-                offset: 0,
-                sign: Sign::Minus,
-            },
-            "-9223372036854775808 -0000",
-        ),
-        (
-            Time {
-                seconds: 500,
-                offset: 9000,
-                sign: Sign::Plus,
-            },
-            "500 +0230",
-        ),
-        (
-            Time {
-                seconds: 189009009,
-                offset: -36000,
-                sign: Sign::Minus,
-            },
-            "189009009 -1000",
-        ),
-        (
-            Time {
-                seconds: 0,
-                offset: 0,
-                sign: Sign::Minus,
-            },
-            "0 -0000",
-        ),
-    ] {
-        let mut output = Vec::new();
-        time.write_to(&mut output)?;
-        assert_eq!(output.as_bstr(), expected);
-        assert_eq!(time.size(), output.len());
+mod write_to {
+    use bstr::ByteSlice;
+    use gix_date::time::Sign;
+    use gix_date::{SecondsSinceUnixEpoch, Time};
+
+    #[test]
+    fn invalid() {
+        let time = Time {
+            seconds: 0,
+            offset: (100 * 60 * 60) + 30 * 60,
+            sign: Sign::Plus,
+        };
+        let err = time.write_to(&mut Vec::new()).unwrap_err();
+        assert_eq!(err.to_string(), "Cannot represent offsets larger than +-9900");
+    }
 
-        let actual = gix_date::parse(&output.as_bstr().to_string(), None).expect("round-trippable");
-        assert_eq!(time, actual);
+    #[test]
+    fn valid_roundtrips() -> Result<(), Box<dyn std::error::Error>> {
+        for (time, expected) in [
+            (
+                Time {
+                    seconds: SecondsSinceUnixEpoch::MAX,
+                    offset: 0,
+                    sign: Sign::Minus,
+                },
+                "9223372036854775807 -0000",
+            ),
+            (
+                Time {
+                    seconds: SecondsSinceUnixEpoch::MIN,
+                    offset: 0,
+                    sign: Sign::Minus,
+                },
+                "-9223372036854775808 -0000",
+            ),
+            (
+                Time {
+                    seconds: 500,
+                    offset: 9000,
+                    sign: Sign::Plus,
+                },
+                "500 +0230",
+            ),
+            (
+                Time {
+                    seconds: 189009009,
+                    offset: -36000,
+                    sign: Sign::Minus,
+                },
+                "189009009 -1000",
+            ),
+            (
+                Time {
+                    seconds: 0,
+                    offset: 0,
+                    sign: Sign::Minus,
+                },
+                "0 -0000",
+            ),
+            (
+                Time {
+                    seconds: 0,
+                    offset: -24 * 60 * 60,
+                    sign: Sign::Minus,
+                },
+                "0 -2400",
+            ),
+            (
+                Time {
+                    seconds: 0,
+                    offset: 24 * 60 * 60,
+                    sign: Sign::Plus,
+                },
+                "0 +2400",
+            ),
+            (
+                Time {
+                    seconds: 0,
+                    offset: (25 * 60 * 60) + 30 * 60,
+                    sign: Sign::Plus,
+                },
+                "0 +2530",
+            ),
+            (
+                Time {
+                    seconds: 0,
+                    offset: (-25 * 60 * 60) - 30 * 60,
+                    sign: Sign::Minus,
+                },
+                "0 -2530",
+            ),
+            (
+                Time {
+                    seconds: 0,
+                    offset: (99 * 60 * 60) + 59 * 60,
+                    sign: Sign::Plus,
+                },
+                "0 +9959",
+            ),
+        ] {
+            let mut output = Vec::new();
+            time.write_to(&mut output)?;
+            assert_eq!(output.as_bstr(), expected);
+            assert_eq!(time.size(), output.len());
+
+            let actual = gix_date::parse(&output.as_bstr().to_string(), None).expect("round-trippable");
+            assert_eq!(time, actual);
+        }
+        Ok(())
     }
-    Ok(())
 }

gix-date/tests/time/parse.rs

@@ -91,6 +91,7 @@ fn bad_raw() {
         "123456 +060",
         "123456 -060",
         "123456 +06000",
+        "123456 +10030",
         "123456 06000",
         "123456  0600",
         "123456 +0600 extra",
@@ -101,6 +102,26 @@ fn bad_raw() {
     }
 }
 
+#[test]
+fn double_negation_in_offset() {
+    let actual = gix_date::parse("1288373970 --700", None).unwrap();
+    assert_eq!(
+        actual,
+        gix_date::Time {
+            seconds: 1288373970,
+            offset: 25200,
+            sign: Sign::Minus,
+        },
+        "double-negation stays negative, and is parseable."
+    );
+
+    assert_eq!(
+        actual.to_bstring(),
+        "1288373970 -0700",
+        "serialization corrects the issue"
+    );
+}
+
 #[test]
 fn git_default() {
     assert_eq!(