From 1bc0648b3c22b44d50c325ac62d120ef8b945169 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Wed, 21 Aug 2024 11:15:34 +0200 Subject: [PATCH] Utility::FormatDateTime(): use boost::numeric_cast<>() The previous implementation actually had undefined behavior when called with a double that can't be represented as time_t. With boost::numeric_cast, there's a convenient cast available that avoids this and throws an exceptions on overflow. It's undefined behavior ([0], where the implicit conversion rule comes into play because the C-style cast uses static_cast [1] which in turn uses the imlicit conversion as per rule 5 of [2]): > A prvalue of floating-point type can be converted to a prvalue of any integer > type. The fractional part is truncated, that is, the fractional part is > discarded. > > * If the truncated value cannot fit into the destination type, the behavior > is undefined (even when the destination type is unsigned, modulo arithmetic > does not apply). Note that on Linux amd64, the undefined behavior typically manifests itself in the result being the minimal value of time_t which then results in localtime_r failing with EOVERFLOW. [0]: https://en.cppreference.com/w/cpp/language/implicit_conversion#Floating.E2.80.93integral_conversions [1]: https://en.cppreference.com/w/cpp/language/explicit_cast [2]: https://en.cppreference.com/w/cpp/language/static_cast --- lib/base/utility.cpp | 4 +++- test/base-utility.cpp | 7 +++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/base/utility.cpp b/lib/base/utility.cpp index 6ff84ae65..6f272178a 100644 --- a/lib/base/utility.cpp +++ b/lib/base/utility.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -1052,7 +1053,8 @@ String Utility::FormatDuration(double duration) String Utility::FormatDateTime(const char *format, double ts) { char timestamp[128]; - auto tempts = (time_t)ts; /* We don't handle sub-second timestamps here just yet. */ + // Sub-second precision is removed, strftime() has no format specifiers for that anyway. + auto tempts = boost::numeric_cast(ts); tm tmthen; #ifdef _MSC_VER diff --git a/test/base-utility.cpp b/test/base-utility.cpp index 77d91f47a..ced81ae4a 100644 --- a/test/base-utility.cpp +++ b/test/base-utility.cpp @@ -137,6 +137,9 @@ BOOST_AUTO_TEST_CASE(TruncateUsingHash) BOOST_AUTO_TEST_CASE(FormatDateTime) { using time_t_limit = std::numeric_limits; + using double_limit = std::numeric_limits; + using boost::numeric::negative_overflow; + using boost::numeric::positive_overflow; // Helper to repeat a given string a number of times. auto repeat = [](const std::string& s, size_t n) { @@ -182,6 +185,10 @@ BOOST_AUTO_TEST_CASE(FormatDateTime) { // timestamps, so localtime_r() returns EOVERFLOW which makes the implementation throw an exception. BOOST_CHECK_THROW(Utility::FormatDateTime("%Y", std::nextafter(time_t_limit::min(), 0)), posix_error); BOOST_CHECK_THROW(Utility::FormatDateTime("%Y", std::nextafter(time_t_limit::max(), 0)), posix_error); + + // Out of range timestamps. + BOOST_CHECK_THROW(Utility::FormatDateTime("%Y", std::nextafter(time_t_limit::min(), -double_limit::infinity())), negative_overflow); + BOOST_CHECK_THROW(Utility::FormatDateTime("%Y", std::nextafter(time_t_limit::max(), +double_limit::infinity())), positive_overflow); } BOOST_AUTO_TEST_SUITE_END()