diff --git a/lib/base/utility.cpp b/lib/base/utility.cpp index 6ff84ae65..338e4f77f 100644 --- a/lib/base/utility.cpp +++ b/lib/base/utility.cpp @@ -4,6 +4,7 @@ #include "base/utility.hpp" #include "base/convert.hpp" #include "base/application.hpp" +#include "base/defer.hpp" #include "base/logger.hpp" #include "base/exception.hpp" #include "base/socket.hpp" @@ -19,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -1049,22 +1051,19 @@ String Utility::FormatDuration(double duration) return NaturalJoin(tokens); } -String Utility::FormatDateTime(const char *format, double ts) +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 - tm *temp = localtime(&tempts); - - if (!temp) { + errno_t err = localtime_s(&tmthen, &tempts); + if (err) { BOOST_THROW_EXCEPTION(posix_error() - << boost::errinfo_api_function("localtime") - << boost::errinfo_errno(errno)); + << boost::errinfo_api_function("localtime_s") + << boost::errinfo_errno(err)); } - - tmthen = *temp; #else /* _MSC_VER */ if (!localtime_r(&tempts, &tmthen)) { BOOST_THROW_EXCEPTION(posix_error() @@ -1073,9 +1072,61 @@ String Utility::FormatDateTime(const char *format, double ts) } #endif /* _MSC_VER */ - strftime(timestamp, sizeof(timestamp), format, &tmthen); + return FormatDateTime(format, &tmthen); +} - return timestamp; +String Utility::FormatDateTime(const char* format, const tm* t) { + /* Known limitations of the implementation: Only works if the result is at most 127 bytes, otherwise returns an + * empty string. An empty string is also returned in all other error cases as proper error handling for strftime() + * is impossible. + * + * From strftime(3): + * + * If the output string would exceed max bytes, errno is not set. This makes it impossible to distinguish this + * error case from cases where the format string legitimately produces a zero-length output string. POSIX.1-2001 + * does not specify any errno settings for strftime(). + * + * https://manpages.debian.org/bookworm/manpages-dev/strftime.3.en.html#BUGS + * + * There's also std::put_time() from C++ which works with an ostream and does not have a fixed size output buffer + * and should allow using the error handling of the ostream. However, there seem to be an unfortunate implementation + * of this on some Windows versions where passing an invalid format string results in std::bad_alloc and the process + * allocating more and more memory before throwing the exception. In case someone in the future wants to try + * std::put_time() again: better build packages for Windows and test them across all supported versions. + * Hypothesis: it's implemented using a fixed output buffer and retrying with a larger buffer on error, assuming + * the error was due to the buffer being too small. + */ + +#ifdef _MSC_VER + /* On Windows, the strftime() function family invokes an invalid parameter handler when the format string is + * invalid (see the "Remarks" section in their documentation). std::put_time() shows the same behavior as it + * uses _wcsftime_l() internally. The default invalid parameter handler may terminate the process, which can + * be a problem given that the format string can be specified by the user from the Icinga DSL. + * + * Thus, temporarily set a thread-local no-op handler to disable the default one allowing the program to + * continue. This then simply results in the function returning an error which then results in an exception as + * we ask the stream to throw one. + * + * See also: + * https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/strftime-wcsftime-strftime-l-wcsftime-l?view=msvc-170 + * https://learn.microsoft.com/en-us/cpp/c-runtime-library/parameter-validation?view=msvc-170 + * https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/set-invalid-parameter-handler-set-thread-local-invalid-parameter-handler?view=msvc-170 + */ + + auto oldHandler = _set_thread_local_invalid_parameter_handler( + [](const wchar_t*, const wchar_t*, const wchar_t*, unsigned int, uintptr_t) { + // Intentionally do nothing to continue executing. + }); + + Defer resetHandler([oldHandler]() { + _set_thread_local_invalid_parameter_handler(oldHandler); + }); +#endif /* _MSC_VER */ + + char buf[128]; + size_t n = strftime(buf, sizeof(buf), format, t); + // On error, n == 0 and an empty string is returned. + return std::string(buf, n); } String Utility::FormatErrorNumber(int code) { diff --git a/lib/base/utility.hpp b/lib/base/utility.hpp index 47b68d251..5132673ca 100644 --- a/lib/base/utility.hpp +++ b/lib/base/utility.hpp @@ -77,7 +77,8 @@ public: static String Join(const Array::Ptr& tokens, char separator, bool escapeSeparator = true); static String FormatDuration(double duration); - static String FormatDateTime(const char *format, double ts); + static String FormatDateTime(const char* format, double ts); + static String FormatDateTime(const char* format, const tm* t); static String FormatErrorNumber(int code); #ifndef _WIN32 diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 28b528463..e3772886a 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -128,6 +128,7 @@ add_boost_test(base base_utility/validateutf8 base_utility/EscapeCreateProcessArg base_utility/TruncateUsingHash + base_utility/FormatDateTime base_value/scalar base_value/convert base_value/format diff --git a/test/base-utility.cpp b/test/base-utility.cpp index 65222e1fd..8fe1814da 100644 --- a/test/base-utility.cpp +++ b/test/base-utility.cpp @@ -135,4 +135,78 @@ BOOST_AUTO_TEST_CASE(TruncateUsingHash) std::string(37, 'a') + "...86f33652fcffd7fa1443e246dd34fe5d00e25ffd"); } +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) { + std::ostringstream stream; + for (size_t i = 0; i < n; ++i) { + stream << s; + } + return stream.str(); + }; + + // Valid inputs. + const double ts = 1136214245.0; // 2006-01-02 15:04:05 UTC + BOOST_CHECK_EQUAL("2006-01-02 15:04:05", Utility::FormatDateTime("%F %T", ts)); + BOOST_CHECK_EQUAL("2006", Utility::FormatDateTime("%Y", ts)); + BOOST_CHECK_EQUAL("2006#2006", Utility::FormatDateTime("%Y#%Y", ts)); + BOOST_CHECK_EQUAL("%", Utility::FormatDateTime("%%", ts)); + BOOST_CHECK_EQUAL("%Y", Utility::FormatDateTime("%%Y", ts)); + BOOST_CHECK_EQUAL("", Utility::FormatDateTime("", ts)); + BOOST_CHECK_EQUAL("1970-01-01 00:00:00", Utility::FormatDateTime("%F %T", 0.0)); + BOOST_CHECK_EQUAL("2038-01-19 03:14:07", Utility::FormatDateTime("%F %T", 2147483647.0)); // 2^31 - 1 + if constexpr (sizeof(time_t) > sizeof(int32_t)) { + BOOST_CHECK_EQUAL("2100-03-14 13:37:42", Utility::FormatDateTime("%F %T", 4108714662.0)); // Past year 2038 + } else { + BOOST_WARN_MESSAGE(false, "skipping test with past 2038 input due to 32 bit time_t"); + } + + // Negative (pre-1970) timestamps. +#ifdef _MSC_VER + // localtime_s() on Windows doesn't seem to like them and always errors out. + BOOST_CHECK_THROW(Utility::FormatDateTime("%F %T", -1.0), posix_error); + BOOST_CHECK_THROW(Utility::FormatDateTime("%F %T", -2147483648.0), posix_error); // -2^31 +#else /* _MSC_VER */ + BOOST_CHECK_EQUAL("1969-12-31 23:59:59", Utility::FormatDateTime("%F %T", -1.0)); + BOOST_CHECK_EQUAL("1901-12-13 20:45:52", Utility::FormatDateTime("%F %T", -2147483648.0)); // -2^31 +#endif /* _MSC_VER */ + + // Values right at the limits of time_t. + // + // With 64 bit time_t, there may not be an exact double representation of its min/max value, std::nextafter() is + // used to move the value towards 0 so that it's within the range of doubles that can be represented as time_t. + // + // These are expected to result in an error due to the intermediate struct tm not being able to represent these + // 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); + + // Excessive format strings can result in something too large for the buffer, errors out to the empty string. + // Note: both returning the proper result or throwing an exception would be fine too, unfortunately, that's + // not really possible due to limitations in strftime() error handling, see comment in the implementation. + BOOST_CHECK_EQUAL("", Utility::FormatDateTime(repeat("%Y", 1000).c_str(), ts)); + + // Invalid format strings. + for (const char* format : {"%", "x % y", "x %! y"}) { + std::string result = Utility::FormatDateTime(format, ts); + + // Implementations of strftime() seem to either keep invalid format specifiers and return them in the output, or + // treat them as an error which our implementation currently maps to the empty string due to strftime() not + // properly reporting errors. If this limitation of our implementation is lifted, other behavior like throwing + // an exception would also be valid. + BOOST_CHECK_MESSAGE(result.empty() || result == format, + "FormatDateTime(" << std::quoted(format) << ", " << ts << ") = " << std::quoted(result) << + " should be one of [\"\", " << std::quoted(format) << "]"); + } + + // 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() diff --git a/test/icinga-legacytimeperiod.cpp b/test/icinga-legacytimeperiod.cpp index 900bb6578..8661b75f7 100644 --- a/test/icinga-legacytimeperiod.cpp +++ b/test/icinga-legacytimeperiod.cpp @@ -527,16 +527,7 @@ struct Segment std::string pretty_time(const tm& t) { -#if defined(__GNUC__) && __GNUC__ < 5 - // GCC did not implement std::put_time() until version 5 - char buf[128]; - size_t n = strftime(buf, sizeof(buf), "%Y-%m-%d %H:%M:%S %Z", &t); - return std::string(buf, n); -#else /* defined(__GNUC__) && __GNUC__ < 5 */ - std::ostringstream stream; - stream << std::put_time(&t, "%Y-%m-%d %H:%M:%S %Z"); - return stream.str(); -#endif /* defined(__GNUC__) && __GNUC__ < 5 */ + return Utility::FormatDateTime("%Y-%m-%d %H:%M:%S %Z", &t); } std::string pretty_time(time_t t)