Skip to content

Commit 3448fd9

Browse files
ByronByron
committed
fix: assure writing invalid dates doesn't panic (#1367).
1 parent 5197b5a commit 3448fd9

File tree

3 files changed

+146
-59
lines changed

3 files changed

+146
-59
lines changed

‎gix-date/src/time/write.rs

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ use crate::{time::Sign, Time};
55
/// Serialization with standard `git` format
66
impl Time {
77
/// Serialize this instance into memory, similar to what [`write_to()`][Self::write_to()] would do with arbitrary `Write` implementations.
8+
///
9+
/// # Panics
10+
///
11+
/// If the underlying call fails as this instance can't be represented, typically due to an invalid offset.
812
pub fn to_bstring(&self) -> BString {
913
let mut buf = Vec::with_capacity(64);
1014
self.write_to(&mut buf).expect("write to memory cannot fail");
@@ -13,6 +17,18 @@ impl Time {
1317

1418
/// Serialize this instance to `out` in a format suitable for use in header fields of serialized git commits or tags.
1519
pub fn write_to(&self, out: &mut dyn std::io::Write) -> std::io::Result<()> {
20+
const SECONDS_PER_HOUR: u32 = 60 * 60;
21+
let offset = self.offset.unsigned_abs();
22+
let hours = offset / SECONDS_PER_HOUR;
23+
let minutes = (offset - (hours * SECONDS_PER_HOUR)) / 60;
24+
25+
if hours > 99 {
26+
return Err(std::io::Error::new(
27+
std::io::ErrorKind::Other,
28+
"Cannot represent offsets larger than +-9900",
29+
));
30+
}
31+
1632
let mut itoa = itoa::Buffer::new();
1733
out.write_all(itoa.format(self.seconds).as_bytes())?;
1834
out.write_all(b" ")?;
@@ -23,12 +39,6 @@ impl Time {
2339

2440
const ZERO: &[u8; 1] = b"0";
2541

26-
const SECONDS_PER_HOUR: u32 = 60 * 60;
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-
3242
if hours < 10 {
3343
out.write_all(ZERO)?;
3444
}

‎gix-date/tests/time/mod.rs

Lines changed: 109 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
use bstr::ByteSlice;
2-
use gix_date::{time::Sign, SecondsSinceUnixEpoch, Time};
1+
use gix_date::Time;
32

43
mod baseline;
54
mod format;
@@ -32,57 +31,114 @@ fn is_set() {
3231
.is_set());
3332
}
3433

35-
#[test]
36-
fn write_to() -> Result<(), Box<dyn std::error::Error>> {
37-
for (time, expected) in [
38-
(
39-
Time {
40-
seconds: SecondsSinceUnixEpoch::MAX,
41-
offset: 0,
42-
sign: Sign::Minus,
43-
},
44-
"9223372036854775807 -0000",
45-
),
46-
(
47-
Time {
48-
seconds: SecondsSinceUnixEpoch::MIN,
49-
offset: 0,
50-
sign: Sign::Minus,
51-
},
52-
"-9223372036854775808 -0000",
53-
),
54-
(
55-
Time {
56-
seconds: 500,
57-
offset: 9000,
58-
sign: Sign::Plus,
59-
},
60-
"500 +0230",
61-
),
62-
(
63-
Time {
64-
seconds: 189009009,
65-
offset: -36000,
66-
sign: Sign::Minus,
67-
},
68-
"189009009 -1000",
69-
),
70-
(
71-
Time {
72-
seconds: 0,
73-
offset: 0,
74-
sign: Sign::Minus,
75-
},
76-
"0 -0000",
77-
),
78-
] {
79-
let mut output = Vec::new();
80-
time.write_to(&mut output)?;
81-
assert_eq!(output.as_bstr(), expected);
82-
assert_eq!(time.size(), output.len());
34+
mod write_to {
35+
use bstr::ByteSlice;
36+
use gix_date::time::Sign;
37+
use gix_date::{SecondsSinceUnixEpoch, Time};
38+
39+
#[test]
40+
fn invalid() {
41+
let time = Time {
42+
seconds: 0,
43+
offset: (100 * 60 * 60) + 30 * 60,
44+
sign: Sign::Plus,
45+
};
46+
let err = time.write_to(&mut Vec::new()).unwrap_err();
47+
assert_eq!(err.to_string(), "Cannot represent offsets larger than +-9900");
48+
}
8349

84-
let actual = gix_date::parse(&output.as_bstr().to_string(), None).expect("round-trippable");
85-
assert_eq!(time, actual);
50+
#[test]
51+
fn valid_roundtrips() -> Result<(), Box<dyn std::error::Error>> {
52+
for (time, expected) in [
53+
(
54+
Time {
55+
seconds: SecondsSinceUnixEpoch::MAX,
56+
offset: 0,
57+
sign: Sign::Minus,
58+
},
59+
"9223372036854775807 -0000",
60+
),
61+
(
62+
Time {
63+
seconds: SecondsSinceUnixEpoch::MIN,
64+
offset: 0,
65+
sign: Sign::Minus,
66+
},
67+
"-9223372036854775808 -0000",
68+
),
69+
(
70+
Time {
71+
seconds: 500,
72+
offset: 9000,
73+
sign: Sign::Plus,
74+
},
75+
"500 +0230",
76+
),
77+
(
78+
Time {
79+
seconds: 189009009,
80+
offset: -36000,
81+
sign: Sign::Minus,
82+
},
83+
"189009009 -1000",
84+
),
85+
(
86+
Time {
87+
seconds: 0,
88+
offset: 0,
89+
sign: Sign::Minus,
90+
},
91+
"0 -0000",
92+
),
93+
(
94+
Time {
95+
seconds: 0,
96+
offset: -24 * 60 * 60,
97+
sign: Sign::Minus,
98+
},
99+
"0 -2400",
100+
),
101+
(
102+
Time {
103+
seconds: 0,
104+
offset: 24 * 60 * 60,
105+
sign: Sign::Plus,
106+
},
107+
"0 +2400",
108+
),
109+
(
110+
Time {
111+
seconds: 0,
112+
offset: (25 * 60 * 60) + 30 * 60,
113+
sign: Sign::Plus,
114+
},
115+
"0 +2530",
116+
),
117+
(
118+
Time {
119+
seconds: 0,
120+
offset: (-25 * 60 * 60) - 30 * 60,
121+
sign: Sign::Minus,
122+
},
123+
"0 -2530",
124+
),
125+
(
126+
Time {
127+
seconds: 0,
128+
offset: (99 * 60 * 60) + 59 * 60,
129+
sign: Sign::Plus,
130+
},
131+
"0 +9959",
132+
),
133+
] {
134+
let mut output = Vec::new();
135+
time.write_to(&mut output)?;
136+
assert_eq!(output.as_bstr(), expected);
137+
assert_eq!(time.size(), output.len());
138+
139+
let actual = gix_date::parse(&output.as_bstr().to_string(), None).expect("round-trippable");
140+
assert_eq!(time, actual);
141+
}
142+
Ok(())
86143
}
87-
Ok(())
88144
}

‎gix-date/tests/time/parse.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ fn bad_raw() {
9191
"123456 +060",
9292
"123456 -060",
9393
"123456 +06000",
94+
"123456 +10030",
9495
"123456 06000",
9596
"123456 0600",
9697
"123456 +0600 extra",
@@ -101,6 +102,26 @@ fn bad_raw() {
101102
}
102103
}
103104

105+
#[test]
106+
fn double_negation_in_offset() {
107+
let actual = gix_date::parse("1288373970 --700", None).unwrap();
108+
assert_eq!(
109+
actual,
110+
gix_date::Time {
111+
seconds: 1288373970,
112+
offset: 25200,
113+
sign: Sign::Minus,
114+
},
115+
"double-negation stays negative, and is parseable."
116+
);
117+
118+
assert_eq!(
119+
actual.to_bstring(),
120+
"1288373970 -0700",
121+
"serialization corrects the issue"
122+
);
123+
}
124+
104125
#[test]
105126
fn git_default() {
106127
assert_eq!(

0 commit comments

Comments
 (0)