Assertion failure crash in `gix_date::time::write::<impl gix_date::Time>::write_to`

[bradlarsen]

Current behavior 😯

In Nosey Parker, I've seen crashes in gix_date::time::write::<impl gix_date::Time>::write_to on some Git inputs.

In particular, the crash occurs on certain Git commit objects with malformed timestamps, when Nosey Parker attempts to serialize the commit timestamp to a string for later use.

The code in question:

14: gix_date::time::write::<impl gix_date::Time>::write_to::hec5da7a5377c9024
    at /Users/blarsen/.cargo/registry/src/index.crates.io-6f17d22bba15001f/gix-date-0.8.5/src/time/write.rs:29
      27 │         let offset = self.offset.unsigned_abs();
      28 │         let hours = offset / SECONDS_PER_HOUR;
      29 >         assert!(hours < 25, "offset is more than a day: {hours}");
      30 │         let minutes = (offset - (hours * SECONDS_PER_HOUR)) / 60;
      31 │

Expected behavior 🤔

Gix shouldn't panic. 😎

Git behavior

Git seems to handle the malformed commit timestamp by treating it as the Unix epoch.

Steps to reproduce 🕹

An example commit with the malformed timestamp is vijaylbais/boto@c1eddff.

Oddly enough, GitHub still parses the timestamp into a non-default value.

If you dump that commit in its raw form using git show --format=raw c1eddff4ee3f62b6039f1083651b9118883e7f07, you get this. Note the double minus signs on the timezone offset:

commit c1eddff4ee3f62b6039f1083651b9118883e7f07
tree 0a851d7a2a66084ab10516c406a405d147e974ad
parent 31350f4f0f459485eff2131517e3450cf251f6fa
author Dan Loewenherz <dloewenherz@gmail.com> 1288373970 --700
committer Dan Loewenherz <dloewenherz@gmail.com> 1288373970 --700

    Fix a bug in the Google Storage Key class that manifested when setting policy in set_contents_from_file
    
    Without this, a TypeError will be thrown if policy is specified.

diff --git a/boto/gs/key.py b/boto/gs/key.py
old mode 100755
new mode 100644
index d9ac254b..310a7092
--- a/boto/gs/key.py
+++ b/boto/gs/key.py
@@ -163,6 +163,8 @@ class Key(S3Key):
         just overriding/sharing code the way it currently works).
         """
         provider = self.bucket.connection.provider
+        if headers is None:
+            headers = {}
         if policy:
             headers[provider.acl_header] = policy
         if hasattr(fp, 'name'):

With just a regular git show, you get this, at least with git version 2.39.3 (Apple Git-146), which shows the timestamp being parsed as the Unix epoch:

commit c1eddff4ee3f62b6039f1083651b9118883e7f07
Author: Dan Loewenherz <dloewenherz@gmail.com>
Date:   Thu Jan 1 00:00:00 1970 +0000

    Fix a bug in the Google Storage Key class that manifested when setting policy in set_contents_from_file

    Without this, a TypeError will be thrown if policy is specified.

diff --git a/boto/gs/key.py b/boto/gs/key.py
old mode 100755
new mode 100644
index d9ac254b..310a7092
--- a/boto/gs/key.py
+++ b/boto/gs/key.py
@@ -163,6 +163,8 @@ class Key(S3Key):
         just overriding/sharing code the way it currently works).
         """
         provider = self.bucket.connection.provider
+        if headers is None:
+            headers = {}
         if policy:
             headers[provider.acl_header] = policy
         if hasattr(fp, 'name'):

[bradlarsen]

I looked back at my old reported issues on gitoxide, and this same problematic input triggered another long-fixed bug in gix! See #950.

[Byron]

Thanks a lot for reporting!

I prefer the GitHub way of presenting the date, as it seems to ignore the invalid offset but presents the unix timestamp that it could parse as UTC. I will add a few tests to see what currently happens, and make sure there is no panic anymore.

[Byron]

It turned out that the --700 date in question could already be parsed losslessly, but additional tests were added to assure correct behaviour around large offset values.

I hope this fixes these date-related issues for good.