This repository was archived by the owner on Aug 19, 2019. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 11
Fix parsing of fractional seconds. #65
Merged
Merged
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
4091724
Add initial unit test for time.cc. Fix a subtle bug exposed by the test.
igorpeshansky 6eaf96a
Add test/.gitignore; normalize src/.gitignore.
igorpeshansky 35701a5
Allow running "make test" in src/.
igorpeshansky 615e6c6
Handle timestamps without nanoseconds.
igorpeshansky b0501e0
Ensure fractional seconds are converted to nanoseconds.
igorpeshansky bd90968
Ensure nanos start with a digit.
igorpeshansky d0d054a
Add more tests for timestamp parsing.
igorpeshansky f715f2d
Even more tests.
igorpeshansky a45f57a
Add testing instructions to README.md.
igorpeshansky 51ba2f3
An alternate implementation that parses seconds and nanoseconds as a …
igorpeshansky 9a46457
Ensure that seconds is exactly 2 digits.
igorpeshansky 256eb2b
Add more tests.
igorpeshansky bcc75b6
Remove useless check and add a clarifying comment.
igorpeshansky 603e1ac
Ensure that the time is in the right format before parsing it as a do…
igorpeshansky e70bb6c
Keep the original time format.
igorpeshansky File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| metadatad | ||
| init-submodules | ||
| build-cpp-netlib | ||
| build-yaml-cpp | ||
| metadatad |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,8 @@ | |
|
|
||
| #include "time.h" | ||
|
|
||
| #include <cctype> | ||
| #include <cmath> | ||
| #include <cstdlib> | ||
| #include <iomanip> | ||
| #include <mutex> | ||
|
|
@@ -31,8 +33,11 @@ namespace { | |
| // See http://stackoverflow.com/questions/4137748. | ||
| int UTCOffset() { | ||
| const std::time_t local = std::time(nullptr); | ||
| const std::time_t utc = std::mktime(std::gmtime(&local)); | ||
| return utc - local; | ||
| std::tm utc_time = safe_gmtime(&local); | ||
| // Since we're only using this for conversion to UTC, always turn off DST. | ||
| utc_time.tm_isdst = 0; | ||
| const std::time_t utc = std::mktime(&utc_time); | ||
| return local - utc; | ||
| } | ||
|
|
||
| const int kUtcOffset = UTCOffset(); | ||
|
|
@@ -57,17 +62,36 @@ std::string ToString(const std::chrono::system_clock::time_point& t) { | |
|
|
||
| std::chrono::system_clock::time_point FromString(const std::string& s) { | ||
| std::tm tm; | ||
| const char* end = strptime(s.c_str(), "%Y-%m-%dT%H:%M:%S", &tm); | ||
| if (end == nullptr || end - s.c_str() != s.find('.')) { | ||
| // TODO | ||
| char* const end = strptime(s.c_str(), "%Y-%m-%dT%H:%M:%S", &tm); | ||
| if (end == nullptr || !std::isdigit(*(end - 2))) { | ||
| // TODO: Invalid time format. | ||
| return std::chrono::system_clock::time_point(); | ||
| } | ||
| char* zone; | ||
| long ns = std::strtol(end + 1, &zone, 10); | ||
| if (zone <= end + 1 || *zone != 'Z' || *(zone+1) != '\0') { | ||
| // TODO | ||
| if (*end == '.') { | ||
| (void)std::strtol(end + 1, &zone, 10); | ||
| if (zone <= end + 1) { | ||
| // TODO: Missing nanoseconds. | ||
| return std::chrono::system_clock::time_point(); | ||
| } | ||
| } else { | ||
| zone = end; | ||
| } | ||
| if (*zone != 'Z' || *(zone+1) != '\0') { | ||
| // TODO: Invalid timezone. | ||
| return std::chrono::system_clock::time_point(); | ||
| } | ||
| char* d_end; | ||
| double seconds = std::strtod(end - 2, &d_end); | ||
| if (d_end != zone) { | ||
| // TODO: Internal error. | ||
| return std::chrono::system_clock::time_point(); | ||
| } | ||
| static_assert(sizeof(long) == 8, "long is too small"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When would this happen?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For example with a 32-bit compiler. I'd rather fail compilation if someone ever tries to build for Amazon's 32-bit AMI or Windows than silently overflow. |
||
| // Truncate to 9 digits by rounding to 10 and discarding the last one. | ||
| long ns = std::lround((seconds - tm.tm_sec) * 10000000000) / 10; | ||
| // Our UTC offset constant assumes no DST. | ||
| tm.tm_isdst = 0; | ||
| const std::time_t local_time = std::mktime(&tm); | ||
| const std::time_t utc_time = local_time + kUtcOffset; | ||
| std::chrono::system_clock::time_point sec = | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| init-submodules | ||
| *_unittest |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,160 @@ | ||
| #include "../src/time.h" | ||
| #include "gtest/gtest.h" | ||
|
|
||
| using namespace google; | ||
|
|
||
| namespace { | ||
|
|
||
| TEST(TimeTest, EpochToString) { | ||
| const std::chrono::system_clock::time_point epoch; | ||
| EXPECT_EQ( | ||
| "1970-01-01T00:00:00.000000000Z", | ||
| rfc3339::ToString(epoch) | ||
| ); | ||
| } | ||
|
|
||
| TEST(TimeTest, RoundtripViaTimePoint) { | ||
| const std::chrono::system_clock::time_point t = | ||
| rfc3339::FromString("2018-03-03T01:23:45.678901234Z"); | ||
| EXPECT_EQ( | ||
| "2018-03-03T01:23:45.678901234Z", | ||
| rfc3339::ToString(t) | ||
| ); | ||
| } | ||
|
|
||
| TEST(TimeTest, RoundtripViaString) { | ||
| const std::chrono::system_clock::time_point t = | ||
| std::chrono::system_clock::now(); | ||
| EXPECT_EQ( | ||
| t, | ||
| rfc3339::FromString(rfc3339::ToString(t)) | ||
| ); | ||
| } | ||
|
|
||
| TEST(TimeTest, FromStringNoNanos) { | ||
| const std::chrono::system_clock::time_point t = | ||
| rfc3339::FromString("2018-03-03T01:23:45Z"); | ||
| EXPECT_EQ( | ||
| "2018-03-03T01:23:45.000000000Z", | ||
| rfc3339::ToString(t) | ||
| ); | ||
| } | ||
|
|
||
| TEST(TimeTest, FromStringFewerDigits) { | ||
| const std::chrono::system_clock::time_point t = | ||
| rfc3339::FromString("2018-03-03T01:23:45.6789Z"); | ||
| EXPECT_EQ( | ||
| "2018-03-03T01:23:45.678900000Z", | ||
| rfc3339::ToString(t) | ||
| ); | ||
| } | ||
|
|
||
| TEST(TimeTest, FromStringMoreDigits) { | ||
| const std::chrono::system_clock::time_point t = | ||
| rfc3339::FromString("2018-03-03T01:23:45.67890123456789Z"); | ||
| EXPECT_EQ( | ||
| "2018-03-03T01:23:45.678901234Z", | ||
| rfc3339::ToString(t) | ||
| ); | ||
| } | ||
|
|
||
| TEST(TimeTest, FromStringLargeNanos) { | ||
| const std::chrono::system_clock::time_point t = | ||
| rfc3339::FromString("2018-03-03T01:23:45.9876543210987Z"); | ||
| EXPECT_EQ( | ||
| "2018-03-03T01:23:45.987654321Z", | ||
| rfc3339::ToString(t) | ||
| ); | ||
| } | ||
|
|
||
| TEST(TimeTest, FromStringPositiveNanos) { | ||
| const std::chrono::system_clock::time_point t = | ||
| rfc3339::FromString("2018-03-03T01:23:45.+678901234Z"); | ||
| EXPECT_EQ( | ||
| "1970-01-01T00:00:00.000000000Z", | ||
| rfc3339::ToString(t) | ||
| ); | ||
| } | ||
|
|
||
| TEST(TimeTest, FromStringNegativeNanos) { | ||
| const std::chrono::system_clock::time_point t = | ||
| rfc3339::FromString("2018-03-03T01:23:45.-678901234Z"); | ||
| EXPECT_EQ( | ||
| "1970-01-01T00:00:00.000000000Z", | ||
| rfc3339::ToString(t) | ||
| ); | ||
| } | ||
|
|
||
| TEST(TimeTest, FromStringPositiveSeconds) { | ||
| const std::chrono::system_clock::time_point t = | ||
| rfc3339::FromString("2018-03-03T01:23:+4.567890123Z"); | ||
| EXPECT_EQ( | ||
| "1970-01-01T00:00:00.000000000Z", | ||
| rfc3339::ToString(t) | ||
| ); | ||
| } | ||
|
|
||
| TEST(TimeTest, FromStringNegativeSeconds) { | ||
| const std::chrono::system_clock::time_point t = | ||
| rfc3339::FromString("2018-03-03T01:23:-4.567890123Z"); | ||
| EXPECT_EQ( | ||
| "1970-01-01T00:00:00.000000000Z", | ||
| rfc3339::ToString(t) | ||
| ); | ||
| } | ||
|
|
||
| TEST(TimeTest, FromStringHexSeconds) { | ||
| const std::chrono::system_clock::time_point t = | ||
| rfc3339::FromString("2018-03-03T01:23:0x45.678901234Z"); | ||
| EXPECT_EQ( | ||
| "1970-01-01T00:00:00.000000000Z", | ||
| rfc3339::ToString(t) | ||
| ); | ||
| } | ||
|
|
||
| TEST(TimeTest, FromStringInfSeconds) { | ||
| const std::chrono::system_clock::time_point t1 = | ||
| rfc3339::FromString("2018-03-03T01:23:+infZ"); | ||
| EXPECT_EQ( | ||
| "1970-01-01T00:00:00.000000000Z", | ||
| rfc3339::ToString(t1) | ||
| ); | ||
| const std::chrono::system_clock::time_point t2 = | ||
| rfc3339::FromString("2018-03-03T01:23:-infZ"); | ||
| EXPECT_EQ( | ||
| "1970-01-01T00:00:00.000000000Z", | ||
| rfc3339::ToString(t2) | ||
| ); | ||
| const std::chrono::system_clock::time_point t3 = | ||
| rfc3339::FromString("2018-03-03T01:23:+nanZ"); | ||
| EXPECT_EQ( | ||
| "1970-01-01T00:00:00.000000000Z", | ||
| rfc3339::ToString(t3) | ||
| ); | ||
| const std::chrono::system_clock::time_point t4 = | ||
| rfc3339::FromString("2018-03-03T01:23:-nanZ"); | ||
| EXPECT_EQ( | ||
| "1970-01-01T00:00:00.000000000Z", | ||
| rfc3339::ToString(t4) | ||
| ); | ||
| } | ||
|
|
||
| TEST(TimeTest, FromStringTooManySeconds) { | ||
| const std::chrono::system_clock::time_point t = | ||
| rfc3339::FromString("2018-03-03T01:23:1045.678901234Z"); | ||
| EXPECT_EQ( | ||
| "1970-01-01T00:00:00.000000000Z", | ||
| rfc3339::ToString(t) | ||
| ); | ||
| } | ||
|
|
||
| TEST(TimeTest, FromStringScientificSeconds) { | ||
| const std::chrono::system_clock::time_point t = | ||
| rfc3339::FromString("2018-03-03T01:23:45e+0Z"); | ||
| EXPECT_EQ( | ||
| "1970-01-01T00:00:00.000000000Z", | ||
| rfc3339::ToString(t) | ||
| ); | ||
| } | ||
|
|
||
| } |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it not safe to assume 0 for nanoseconds?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It depends on whether we treat a trailing decimal point with no digits after as valid input or an error. As I read the spec, a decimal point must be followed by at least one digit (search for
time-fraction).