Skip to content

Commit

Permalink
Fix allow negative dates with from [-100,-999]
Browse files Browse the repository at this point in the history
A bug exists where a date like `-100.11.12` will be incorrectly parsed
as the fast path fails and then fallsback to trying to parse the
synthetic input for the fast path.

This code fixes this by having the fast path return none in the advent
that it can't handle the input.

Benchmarks remain unaffected.
  • Loading branch information
nickbabcock committed Nov 12, 2023
1 parent 17c889b commit 4a00cf1
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 27 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ quickcheck = "1"
quickcheck_macros = "1"
flate2 = "1"
serde_with = "2.3.1"
rstest = "0.18.2"

[[bin]]
name = "json"
Expand Down
83 changes: 56 additions & 27 deletions src/common/date.rs
Original file line number Diff line number Diff line change
Expand Up @@ -517,10 +517,10 @@ impl Date {
}

#[inline]
fn fast_parse(r: [u8; 8]) -> Result<Self, DateError> {
fn fast_parse(r: [u8; 8]) -> Option<Result<Self, DateError>> {
if r.iter().all(|x| x.is_ascii_digit()) {
let n = r.map(|x| x - b'0');
Self::from_expanded(ExpandedRawDate {
let result = Self::from_expanded(ExpandedRawDate {
year: i16::from(n[0]) * 1000
+ i16::from(n[1]) * 100
+ i16::from(n[2]) * 10
Expand All @@ -529,9 +529,10 @@ impl Date {
day: n[6] * 10 + n[7],
hour: 0,
})
.ok_or(DateError)
.ok_or(DateError);
Some(result)
} else {
Self::fallback(&r)
None
}
}

Expand All @@ -551,19 +552,19 @@ impl Date {
match s {
[y1, y2, y3, y4, b'.', m1, m2, b'.', d1, d2] => {
let r = [*y1, *y2, *y3, *y4, *m1, *m2, *d1, *d2];
Self::fast_parse(r)
Self::fast_parse(r).unwrap_or_else(|| Self::fallback(s))
}
[y1, y2, y3, y4, b'.', m1, m2, b'.', d1] => {
let r = [*y1, *y2, *y3, *y4, *m1, *m2, b'0', *d1];
Self::fast_parse(r)
Self::fast_parse(r).unwrap_or_else(|| Self::fallback(s))
}
[y1, y2, y3, y4, b'.', m1, b'.', d1, d2] => {
let r = [*y1, *y2, *y3, *y4, b'0', *m1, *d1, *d2];
Self::fast_parse(r)
Self::fast_parse(r).unwrap_or_else(|| Self::fallback(s))
}
[y1, y2, y3, y4, b'.', m1, b'.', d1] => {
let r = [*y1, *y2, *y3, *y4, b'0', *m1, b'0', *d1];
Self::fast_parse(r)
Self::fast_parse(r).unwrap_or_else(|| Self::fallback(s))
}
_ => Self::fallback(s),
}
Expand Down Expand Up @@ -1279,9 +1280,10 @@ mod datederive {}
mod tests {
use super::*;
use quickcheck_macros::quickcheck;
use rstest::*;

#[test]
fn test_date_roundtrip() {
fn test_date_iso() {
let date = Date::parse("1400.1.2").unwrap();
assert_eq!(date.iso_8601().to_string(), String::from("1400-01-02"));
}
Expand All @@ -1291,24 +1293,6 @@ mod tests {
assert_eq!(Date::parse("1.01.01").unwrap(), Date::from_ymd(1, 1, 1));
}

#[test]
fn test_game_fmt() {
let test_cases = [
"1400.1.2",
"1457.3.5",
"1.1.1",
"1444.11.11",
"1444.11.30",
"1444.2.19",
"1444.12.3",
];

for case in &test_cases {
let date = Date::parse(case).unwrap();
assert_eq!(date.game_fmt().to_string(), case.to_string());
}
}

#[test]
fn test_first_bin_date() {
let date = Date::from_binary(56379360).unwrap();
Expand Down Expand Up @@ -1365,6 +1349,51 @@ mod tests {
assert_eq!(date, date2);
}

#[rstest]
#[case("-1.1.1")]
#[case("-1.1.12")]
#[case("-1.11.1")]
#[case("-1.11.12")]
#[case("-10.1.1")]
#[case("-10.1.12")]
#[case("-10.11.1")]
#[case("-10.11.12")]
#[case("-100.1.1")]
#[case("-100.1.12")]
#[case("-100.11.1")]
#[case("-100.11.12")]
#[case("-1000.1.1")]
#[case("-1000.1.12")]
#[case("-1000.11.1")]
#[case("-1000.11.12")]
#[case("1.1.1")]
#[case("1.1.12")]
#[case("1.11.1")]
#[case("1.11.12")]
#[case("10.1.1")]
#[case("10.1.12")]
#[case("10.11.1")]
#[case("10.11.12")]
#[case("100.1.1")]
#[case("100.1.12")]
#[case("100.11.1")]
#[case("100.11.12")]
#[case("1000.1.1")]
#[case("1000.1.12")]
#[case("1000.11.1")]
#[case("1000.11.12")]
#[case("1400.1.2")]
#[case("1457.3.5")]
#[case("1.1.1")]
#[case("1444.11.11")]
#[case("1444.11.30")]
#[case("1444.2.19")]
#[case("1444.12.3")]
fn test_date_game_fmt_roundtrip(#[case] input: &str) {
let s = Date::parse(input).unwrap().game_fmt().to_string();
assert_eq!(&s, input);
}

#[test]
fn test_zero_date() {
let date = Date::from_binary(43800000).unwrap();
Expand Down

0 comments on commit 4a00cf1

Please sign in to comment.