Skip to content

Reject trailing characters when parsing rfl::Timestamp - #736

Open
fhgffy wants to merge 1 commit into
getml:mainfrom
fhgffy:timestamp-trailing-chars
Open

fhgffy wants to merge 1 commit into
getml:mainfrom
fhgffy:timestamp-trailing-chars

Conversation

@fhgffy

@fhgffy fhgffy commented Oct 9, 2026

Copy link
Copy Markdown

rfl::Timestamp accepts strings with extra characters after the format:

using TS = rfl::Timestamp<"%Y-%m-%d">;
TS::from_string("1987-04-19T10:00:00"); // ok, time part is dropped
TS::from_string("1987-04-19x");         // ok
rfl::json::read<Person>(R"({"birthday":"1987-04-19x", ...})"); // ok

strptime() returns a pointer to the first character it didn't use, but only a NULL result 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_point parsing already rejects trailing input.

There's also a small fix in the Windows strptime fallback. When get_time reads to the end of the string, tellg() returns -1, so it returned _s - 1. It now returns the end of the string, the same way ParserTimePoint::parse_datetime handles 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.

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.
@liuzicheng1987

Copy link
Copy Markdown
Collaborator

@fhgffy this look good to me. Let me run the test pipelines and if they run through cleanly, I will merge.

@fhgffy

fhgffy commented Oct 9, 2026

Copy link
Copy Markdown
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

@liuzicheng1987

Copy link
Copy Markdown
Collaborator

@fhgffy OK, I'm re-running

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants