Repository navigation
Conversation
strptime() stops at the end of the format and returns a pointer to whatever is left, but only a NULL result was treated as an error, so "1987-04-19T10:00:00" was accepted for "%Y-%m-%d" and the rest was silently dropped. Require the whole string to be consumed. The Windows fallback also returned _s + tellg() when the stream hit EOF, where tellg() is -1; return the end of the string in that case, as ParserTimePoint already does.
Collaborator
|
@fhgffy this look good to me. Let me run the test pipelines and if they run through cleanly, I will merge. |
Author
|
Thanks for running the pipelines. The Linux and Windows workflows passed. The remaining macOS Intel benchmark job stopped while vcpkg was downloading Thrift: curl could not resolve archive.apache.org, before the project was configured or built. The other macOS test and benchmark jobs passed. I cannot rerun upstream jobs with my fork permissions; the failed job may only need a rerun: https://github.com/getml/reflect-cpp/actions/runs/37864987451/job/113823336605 |
Collaborator
|
@fhgffy OK, I'm re-running |
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
rfl::Timestampaccepts strings with extra characters after the format:strptime()returns a pointer to the first character it didn't use, but only aNULLresult was treated as an error. This also requires the whole string to be consumed, so these now fail with the existing "did not match format" error.std::chrono::system_clock::time_pointparsing already rejects trailing input.There's also a small fix in the Windows
strptimefallback. Whenget_timereads to the end of the string,tellg()returns -1, so it returned_s - 1. It now returns the end of the string, the same wayParserTimePoint::parse_datetimehandles it. I couldn't build on Windows to check that part.Added
json.test_timestamp_trailing_characters. It fails before the change, and the JSON test suite passes after it.