Utility::FormatDateTime(): handle errors from strftime()

So far, the return value of strftime() was simply ignored and the output buffer
passed to the icinga::String constructor. However, there are error conditions
where strftime() returns 0 to signal an error, like if the buffer was too small
for the output. In that case, there's no guarantee on the buffer contents and
reading it can result in undefined behavior. Unfortunately, returning 0 can
also indicate success and strftime() doesn't set errno, so there's no reliable
way to distinguish both situations. Thus, the implementation now returns the
empty string in both cases.

I attempted to use std::put_time() at first as that allows for better error
handling, however, there were problems with the implementation on Windows (see
inline comment), so I put that plan on hold at left strftime() there for the
time being.
This commit is contained in:
Julian Brost 2024-08-21 11:55:19 +02:00
parent c2c66908f6
commit 0285028689
2 changed files with 30 additions and 4 deletions

View File

@ -1052,7 +1052,27 @@ String Utility::FormatDuration(double duration)
String Utility::FormatDateTime(const char *format, double ts)
{
char timestamp[128];
/* 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.
*/
// Sub-second precision is removed, strftime() has no format specifiers for that anyway.
auto tempts = boost::numeric_cast<time_t>(ts);
tm tmthen;
@ -1072,9 +1092,10 @@ String Utility::FormatDateTime(const char *format, double ts)
}
#endif /* _MSC_VER */
strftime(timestamp, sizeof(timestamp), format, &tmthen);
return timestamp;
char buf[128];
size_t n = strftime(buf, sizeof(buf), format, &tmthen);
// On error, n == 0 and an empty string is returned.
return std::string(buf, n);
}
String Utility::FormatErrorNumber(int code) {

View File

@ -186,6 +186,11 @@ BOOST_AUTO_TEST_CASE(FormatDateTime) {
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));
// 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);