Merge pull request #10165 from Icinga/formatdatetime-2.13

Overhaul Utility::FormatDateTime()

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 <boost/thread/tss.hpp>
 #include <boost/algorithm/string/trim.hpp>
 #include <boost/algorithm/string/replace.hpp>
+#include <boost/numeric/conversion/cast.hpp>
 #include <boost/uuid/uuid_io.hpp>
 #include <boost/uuid/uuid_generators.hpp>
 #include <boost/regex.hpp>
@@ -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<time_t>(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);
+}
+
+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 */
 
-	return timestamp;
+	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) {

lib/base/utility.hpp

@@ -77,7 +77,8 @@ class Utility
 	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

test/CMakeLists.txt

@@ -125,6 +125,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

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<time_t>;
+	using double_limit = std::numeric_limits<double>;
+	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(\"" << format << "\", " << ts << ") = \"" << result <<
+			"\" should be one of [\"\", \"" << 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()

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)