Skip to content
This repository was archived by the owner on Dec 8, 2021. It is now read-only.

Commit 6b1cd34

Browse files
authored
fix: correct range validation in ParseRfc3339 (#220)
It was not accepting 59 as a valid minute, this bug was introduced in the clang-tidy cleanup, and our tests validate that things *outside* the valid ranges are correctly reported as errors, but not that things *exactly* at the boundary conditions are reported as valid. Sigh.
1 parent a1f5f01 commit 6b1cd34

File tree

3 files changed

+36
-5
lines changed

3 files changed

+36
-5
lines changed

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,12 @@
22

33
## v0.23.x - TBD
44

5+
## v0.22.1 - 2020-03
6+
7+
* fix: correct range validation in ParseRfc3339 (#219) - the library incorrectly
8+
rejected timestamps with '59' in the minutes part, the tests did not cover
9+
the full range of valid inputs.
10+
511
## v0.22.x - 2020-03
612

713
* fix: workaround a libstdc++ bug on `std::random_device` (#208)

google/cloud/internal/parse_rfc3339.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ std::chrono::system_clock::time_point ParseDateTime(
9494
if (hours < 0 || hours >= kHoursInDay) {
9595
ReportError(timestamp, "Out of range hour.");
9696
}
97-
if (minutes < 0 || minutes >= kMinutesInHour - 1) {
97+
if (minutes < 0 || minutes >= kMinutesInHour) {
9898
ReportError(timestamp, "Out of range minute.");
9999
}
100100
// RFC-3339 points out that the seconds field can only assume value '60' for

google/cloud/internal/parse_rfc3339_test.cc

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,35 @@ TEST(ParseRfc3339Test, ParseEpoch) {
4545
}
4646

4747
TEST(ParseRfc3339Test, ParseSimpleZulu) {
48-
auto timestamp = ParseRfc3339("2018-05-18T14:42:03Z");
49-
// Use `date -u +%s --date='2018-05-18T14:42:03'` to get the magic value:
50-
EXPECT_EQ(1526654523L,
51-
duration_cast<seconds>(timestamp.time_since_epoch()).count());
48+
struct Timestamps {
49+
std::string input;
50+
std::time_t expected;
51+
} tests[] = {
52+
// Use `date -u +%s --date='....'` to get the expected values.
53+
{"2018-05-18T14:42:03Z", 1526654523L},
54+
{"2020-01-01T00:00:00Z", 1577836800L},
55+
{"2020-01-31T00:00:00Z", 1580428800L},
56+
{"2020-02-29T00:00:00Z", 1582934400L},
57+
{"2020-03-31T00:00:00Z", 1585612800L},
58+
{"2020-04-30T00:00:00Z", 1588204800L},
59+
{"2020-05-31T00:00:00Z", 1590883200L},
60+
{"2020-06-30T00:00:00Z", 1593475200L},
61+
{"2020-07-31T00:00:00Z", 1596153600L},
62+
{"2020-08-31T00:00:00Z", 1598832000L},
63+
{"2020-09-30T00:00:00Z", 1601424000L},
64+
{"2020-10-31T00:00:00Z", 1604102400L},
65+
{"2020-11-20T00:00:00Z", 1605830400L},
66+
{"2020-12-31T00:00:00Z", 1609372800L},
67+
{"2020-01-01T00:00:59Z", 1577836859L},
68+
{"2020-01-01T00:59:59Z", 1577840399L},
69+
{"2020-01-01T23:59:59Z", 1577923199L},
70+
};
71+
for (auto const& test : tests) {
72+
auto timestamp = ParseRfc3339(test.input);
73+
auto actual = std::chrono::system_clock::to_time_t(timestamp);
74+
EXPECT_EQ(actual, test.expected)
75+
<< " when testing with input=" << test.input;
76+
}
5277
}
5378

5479
TEST(ParseRfc3339Test, ParseAlternativeSeparators) {

0 commit comments

Comments
 (0)