fix: handle time parsing overload gracefully #35
Merged
Conversation
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
greghuels
changed the title
greghuels
force-pushed
the
greg.huels/FFESUPPORT-348/fix-end-date-parsing-2
branch
2 times, most recently
from
c6bac03 to
ce6098d
Compare
greghuels
force-pushed
the
greg.huels/FFESUPPORT-348/fix-end-date-parsing-2
branch
10 times, most recently
from
0b4982f to
02765b9
Compare
dd-oleksii
approved these changes
Dec 1, 2025
Collaborator
dd-oleksii
left a comment
dd-oleksii
left a comment
There was a problem hiding this comment.
It's improving the handling compared to what we currently have so approving but I think we should properly handle true parse errors and the case of underflow.
greghuels
force-pushed
the
greg.huels/FFESUPPORT-348/fix-end-date-parsing-2
branch
3 times, most recently
from
b76d870 to
639de63
Compare
greghuels
force-pushed
the
greg.huels/FFESUPPORT-348/fix-end-date-parsing-2
branch
4 times, most recently
from
4aac24a to
fb3c196
Compare
Replace $(MAKE) with cmake --build to support all CMake generators, not just Unix Makefiles. This fixes the Windows CI build which uses Visual Studio generators by default. cmake --build works with any generator (Makefiles, Ninja, Visual Studio, Xcode, etc.) and automatically invokes the correct build tool. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Use gmtime_s instead of gmtime on Windows (MSVC C4996) - Fix unreachable code warning in json_utils.hpp by replacing the unreachable return with an else clause containing a static_assert to catch unsupported types at compile time These warnings were being treated as errors with /WX, causing Windows CI builds to fail. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Create a separate OBJECT library for third-party sources (md5_wrapper.c) with warnings disabled (/W0 on MSVC, -w on GCC/Clang). This prevents picohash.h warnings from being treated as errors when EPPOCLIENT_ERR_ON_WARNINGS is enabled. The third-party code is compiled separately and linked into the main library using $<TARGET_OBJECTS:>, ensuring our strict warning flags only apply to our own code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
greghuels
force-pushed
the
greg.huels/FFESUPPORT-348/fix-end-date-parsing-2
branch
from
fb3c196 to
cbdffd1
Compare
Co-authored-by: Oleksii Shmalko <oleksii.shmalko@datadoghq.com>
greghuels
force-pushed
the
greg.huels/FFESUPPORT-348/fix-end-date-parsing-2
branch
from
cbdffd1 to
62a765b
Compare
greghuels
commented
Dec 2, 2025
| br.updatedAt = parseISOTimestamp(j["updatedAt"].get_ref<const std::string&>(), error); | ||
| // TODO: log error | ||
| // if (!error.empty()) { | ||
| // logger.error("BanditResponse: Invalid updatedAt: " + error); |
dd-oleksii
approved these changes
Dec 2, 2025
Comment on lines
+57
to
+82
| // Validate that we have at least 4 characters and they're all digits | ||
| if (timestamp.length() < 4) { | ||
| error = "Invalid timestamp: " + timestamp; | ||
| return std::chrono::system_clock::time_point(); | ||
| } | ||
|
|
||
| std::string yearStr = timestamp.substr(0, 4); | ||
| for (char c : yearStr) { | ||
| if (!std::isdigit(static_cast<unsigned char>(c))) { | ||
| error = "Invalid timestamp: " + timestamp; | ||
| return std::chrono::system_clock::time_point(); | ||
| } | ||
| } | ||
|
|
||
| // Validate that the 5th character is a dash (if it exists) | ||
| if (timestamp.length() > 4 && timestamp[4] != '-') { | ||
| error = "Invalid timestamp: " + timestamp; | ||
| return std::chrono::system_clock::time_point(); | ||
| } | ||
|
|
||
| int year = std::stoi(yearStr); | ||
| if (year < 1970) { | ||
| return std::chrono::system_clock::time_point::min(); | ||
| } else { | ||
| return std::chrono::system_clock::time_point::max(); | ||
| } |
Collaborator
There was a problem hiding this comment.
nit: there's a lot of shenanigans in this section, so worth adding a comment explaining the rational — i.e. that timegm may fail with under/overflow and we try to detect that and return "saturated" value instead of 0.
greghuels
changed the title
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.
Eppo Internal:
🎟️ Fixes FFESUPPORT-348
See also:
Motivation and Context
When parsing the configuration value
"endAt": "9999-12-31T00:00:00.000Z", the SDK was returning{__d_={__rep_=0}}(epoch/zero) instead of a far-future date on windows systems. This causes all allocations with year 9999 end dates to be rejected withAFTER_END_TIMEerror code.