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

Commit

Permalink
fix: correct range validation in ParseRfc3339 (#220)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
coryan authored Mar 10, 2020
1 parent a1f5f01 commit 6b1cd34
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 5 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

## v0.23.x - TBD

## v0.22.1 - 2020-03

* fix: correct range validation in ParseRfc3339 (#219) - the library incorrectly
rejected timestamps with '59' in the minutes part, the tests did not cover
the full range of valid inputs.

## v0.22.x - 2020-03

* fix: workaround a libstdc++ bug on `std::random_device` (#208)
Expand Down
2 changes: 1 addition & 1 deletion google/cloud/internal/parse_rfc3339.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ std::chrono::system_clock::time_point ParseDateTime(
if (hours < 0 || hours >= kHoursInDay) {
ReportError(timestamp, "Out of range hour.");
}
if (minutes < 0 || minutes >= kMinutesInHour - 1) {
if (minutes < 0 || minutes >= kMinutesInHour) {
ReportError(timestamp, "Out of range minute.");
}
// RFC-3339 points out that the seconds field can only assume value '60' for
Expand Down
33 changes: 29 additions & 4 deletions google/cloud/internal/parse_rfc3339_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,35 @@ TEST(ParseRfc3339Test, ParseEpoch) {
}

TEST(ParseRfc3339Test, ParseSimpleZulu) {
auto timestamp = ParseRfc3339("2018-05-18T14:42:03Z");
// Use `date -u +%s --date='2018-05-18T14:42:03'` to get the magic value:
EXPECT_EQ(1526654523L,
duration_cast<seconds>(timestamp.time_since_epoch()).count());
struct Timestamps {
std::string input;
std::time_t expected;
} tests[] = {
// Use `date -u +%s --date='....'` to get the expected values.
{"2018-05-18T14:42:03Z", 1526654523L},
{"2020-01-01T00:00:00Z", 1577836800L},
{"2020-01-31T00:00:00Z", 1580428800L},
{"2020-02-29T00:00:00Z", 1582934400L},
{"2020-03-31T00:00:00Z", 1585612800L},
{"2020-04-30T00:00:00Z", 1588204800L},
{"2020-05-31T00:00:00Z", 1590883200L},
{"2020-06-30T00:00:00Z", 1593475200L},
{"2020-07-31T00:00:00Z", 1596153600L},
{"2020-08-31T00:00:00Z", 1598832000L},
{"2020-09-30T00:00:00Z", 1601424000L},
{"2020-10-31T00:00:00Z", 1604102400L},
{"2020-11-20T00:00:00Z", 1605830400L},
{"2020-12-31T00:00:00Z", 1609372800L},
{"2020-01-01T00:00:59Z", 1577836859L},
{"2020-01-01T00:59:59Z", 1577840399L},
{"2020-01-01T23:59:59Z", 1577923199L},
};
for (auto const& test : tests) {
auto timestamp = ParseRfc3339(test.input);
auto actual = std::chrono::system_clock::to_time_t(timestamp);
EXPECT_EQ(actual, test.expected)
<< " when testing with input=" << test.input;
}
}

TEST(ParseRfc3339Test, ParseAlternativeSeparators) {
Expand Down

0 comments on commit 6b1cd34

Please sign in to comment.