diff --git a/lib/base/datetime.cpp b/lib/base/datetime.cpp index aa7b5e59e..3e443e613 100644 --- a/lib/base/datetime.cpp +++ b/lib/base/datetime.cpp @@ -35,7 +35,7 @@ DateTime::DateTime(const std::vector& args) tms.tm_isdst = -1; - m_Value = mktime(&tms); + m_Value = Utility::TmToTimestamp(&tms); } else if (args.size() == 1) m_Value = args[0]; else diff --git a/lib/base/utility.cpp b/lib/base/utility.cpp index a2e461e5c..2b4e32def 100644 --- a/lib/base/utility.cpp +++ b/lib/base/utility.cpp @@ -1958,3 +1958,51 @@ bool Utility::ComparePasswords(const String& enteredPassword, const String& actu return result; } + +/** + * Normalizes the given struct tm like mktime() from libc does with some exception for DST handling: If the given time + * exists twice on a day, the instance in the DST timezone is picked. If the time does not actually exist on a day, it's + * interpreted using the UTC offset of the standard timezone and then normalized. + * + * This is done in order to provide consistent behavior across operating systems. Historically, Icinga 2 just relied on + * whatever mktime() of the operating system did and this function mimics what glibc does as that's what most systems + * use. + * + * @param t tm struct to be normalized + * @return time_t representing the timestamp given by t + */ +time_t Utility::NormalizeTm(tm *t) +{ + // If tm_isdst already specifies the timezone (0 or 1), just use the mktime() behavior. + if (t->tm_isdst >= 0) { + return mktime(t); + } + + const tm copy = *t; + + t->tm_isdst = 1; + time_t result = mktime(t); + if (result != -1 && t->tm_isdst == 1) { + return result; + } + + // Restore the original input. mktime() can (and does) change more fields than just tm_isdst by converting from + // daylight saving time to standard time (it moves the contents by (typically) an hour, which can move across + // days/weeks/months/years changing all other fields). + *t = copy; + + t->tm_isdst = 0; + return mktime(t); +} + +/** + * Returns the same as NormalizeTm() but takes a const pointer as argument and thus does not modify it. + * + * @param t struct tm to convert to time_t + * @return time_t representing the timestamp given by t + */ +time_t Utility::TmToTimestamp(const tm *t) +{ + tm copy = *t; + return NormalizeTm(©); +} diff --git a/lib/base/utility.hpp b/lib/base/utility.hpp index 5132673ca..2a2fe0c1b 100644 --- a/lib/base/utility.hpp +++ b/lib/base/utility.hpp @@ -185,6 +185,9 @@ public: return in.SubStr(0, maxLength - sha1HexLength - strlen(trunc)) + trunc + SHA1(in); } + static time_t NormalizeTm(tm *t); + static time_t TmToTimestamp(const tm *t); + private: Utility(); diff --git a/lib/icinga/legacytimeperiod.cpp b/lib/icinga/legacytimeperiod.cpp index 55bc58ed3..6d2433758 100644 --- a/lib/icinga/legacytimeperiod.cpp +++ b/lib/icinga/legacytimeperiod.cpp @@ -13,23 +13,12 @@ using namespace icinga; REGISTER_FUNCTION_NONCONST(Internal, LegacyTimePeriod, &LegacyTimePeriod::ScriptFunc, "tp:begin:end"); -/** - * Returns the same as mktime() but does not modify its argument and takes a const pointer. - * - * @param t struct tm to convert to time_t - * @return time_t representing the timestamp given by t - */ -static time_t mktime_const(const tm *t) { - tm copy = *t; - return mktime(©); -} - bool LegacyTimePeriod::IsInTimeRange(const tm *begin, const tm *end, int stride, const tm *reference) { time_t tsbegin, tsend, tsref; - tsbegin = mktime_const(begin); - tsend = mktime_const(end); - tsref = mktime_const(reference); + tsbegin = Utility::TmToTimestamp(begin); + tsend = Utility::TmToTimestamp(end); + tsref = Utility::TmToTimestamp(reference); if (tsref < tsbegin || tsref >= tsend) return false; @@ -85,7 +74,7 @@ void LegacyTimePeriod::FindNthWeekday(int wday, int n, tm *reference) t.tm_sec = 0; t.tm_isdst = -1; - mktime(&t); + Utility::NormalizeTm(&t); if (t.tm_wday == wday) { seen++; @@ -398,8 +387,8 @@ bool LegacyTimePeriod::IsInDayDefinition(const String& daydef, const tm *referen ParseTimeRange(daydef, &begin, &end, &stride, reference); Log(LogDebug, "LegacyTimePeriod") - << "ParseTimeRange: '" << daydef << "' => " << mktime(&begin) - << " -> " << mktime(&end) << ", stride: " << stride; + << "ParseTimeRange: '" << daydef << "' => " << Utility::TmToTimestamp(&begin) + << " -> " << Utility::TmToTimestamp(&end) << ", stride: " << stride; return IsInTimeRange(&begin, &end, stride, reference); } @@ -448,8 +437,8 @@ Dictionary::Ptr LegacyTimePeriod::ProcessTimeRange(const String& timestamp, cons ProcessTimeRangeRaw(timestamp, reference, &begin, &end); return new Dictionary({ - { "begin", (long)mktime(&begin) }, - { "end", (long)mktime(&end) } + { "begin", (long)Utility::TmToTimestamp(&begin) }, + { "end", (long)Utility::TmToTimestamp(&end) } }); } @@ -480,13 +469,13 @@ Dictionary::Ptr LegacyTimePeriod::FindRunningSegment(const String& daydef, const time_t tsend, tsiter, tsref; int stride; - tsref = mktime_const(reference); + tsref = Utility::TmToTimestamp(reference); ParseTimeRange(daydef, &begin, &end, &stride, reference); iter = begin; - tsend = mktime(&end); + tsend = Utility::NormalizeTm(&end); do { if (IsInTimeRange(&begin, &end, stride, &iter)) { @@ -518,7 +507,7 @@ Dictionary::Ptr LegacyTimePeriod::FindRunningSegment(const String& daydef, const iter.tm_hour = 0; iter.tm_min = 0; iter.tm_sec = 0; - tsiter = mktime(&iter); + tsiter = Utility::NormalizeTm(&iter); } while (tsiter < tsend); return nullptr; @@ -538,13 +527,13 @@ Dictionary::Ptr LegacyTimePeriod::FindNextSegment(const String& daydef, const St ref.tm_mday++; } - tsref = mktime(&ref); + tsref = Utility::NormalizeTm(&ref); ParseTimeRange(daydef, &begin, &end, &stride, &ref); iter = begin; - tsend = mktime(&end); + tsend = Utility::NormalizeTm(&end); do { if (IsInTimeRange(&begin, &end, stride, &iter)) { @@ -575,7 +564,7 @@ Dictionary::Ptr LegacyTimePeriod::FindNextSegment(const String& daydef, const St iter.tm_hour = 0; iter.tm_min = 0; iter.tm_sec = 0; - tsiter = mktime(&iter); + tsiter = Utility::NormalizeTm(&iter); } while (tsiter < tsend); } @@ -607,17 +596,17 @@ Array::Ptr LegacyTimePeriod::ScriptFunc(const TimePeriod::Ptr& tp, double begin, t->tm_isdst = -1; // Normalize fields using mktime. - mktime(t); + Utility::NormalizeTm(t); // Reset tm_isdst so that future calls figure out the correct time zone after setting tm_hour/tm_min/tm_sec. t->tm_isdst = -1; }; - for (tm reference = tm_begin; mktime_const(&reference) <= end; advance_to_next_day(&reference)) { + for (tm reference = tm_begin; Utility::TmToTimestamp(&reference) <= end; advance_to_next_day(&reference)) { #ifdef I2_DEBUG Log(LogDebug, "LegacyTimePeriod") - << "Checking reference time " << mktime_const(&reference); + << "Checking reference time " << Utility::TmToTimestamp(&reference); #endif /* I2_DEBUG */ ObjectLock olock(ranges); diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 8ed300c1f..0e2a2976e 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -186,6 +186,7 @@ add_boost_test(base base_utility/EscapeCreateProcessArg base_utility/TruncateUsingHash base_utility/FormatDateTime + base_utility/NormalizeTm base_value/scalar base_value/convert base_value/format diff --git a/test/base-utility.cpp b/test/base-utility.cpp index 2273e1b8c..0e6e0c645 100644 --- a/test/base-utility.cpp +++ b/test/base-utility.cpp @@ -1,6 +1,7 @@ /* Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+ */ #include "base/utility.hpp" +#include "test/utils.hpp" #include #include @@ -230,4 +231,86 @@ BOOST_AUTO_TEST_CASE(FormatDateTime) { BOOST_CHECK_THROW(Utility::FormatDateTime("%Y", positive_out_of_range), positive_overflow); } +BOOST_AUTO_TEST_CASE(NormalizeTm) +{ + GlobalTimezoneFixture tz(GlobalTimezoneFixture::TestTimezoneWithDST); + + auto normalize = [](const std::string_view& input) { + tm t = make_tm(std::string(input)); + return Utility::NormalizeTm(&t); + }; + + auto is_dst = [](const std::string_view& input) { + tm t = make_tm(std::string(input)); + Utility::NormalizeTm(&t); + BOOST_CHECK_GE(t.tm_isdst, 0); + return t.tm_isdst > 0; + }; + + // The whole day 2021-01-01 uses PST (24h day) + BOOST_CHECK(!is_dst("2021-01-01 10:00:00")); + BOOST_CHECK_EQUAL(normalize("2021-01-01 10:00:00"), 1609524000); + BOOST_CHECK_EQUAL(normalize("2021-01-01 10:00:00 PST"), 1609524000); + BOOST_CHECK_EQUAL(normalize("2021-01-01 11:00:00 PDT"), 1609524000); // normalized to 10:00 PST + BOOST_CHECK_EQUAL(normalize("2021-01-02 00:00:00") - normalize("2021-01-01 00:00:00"), 24*60*60); + + // The whole day 2021-07-01 uses PDT (24h day) + BOOST_CHECK(is_dst("2021-07-01 10:00:00")); + BOOST_CHECK_EQUAL(normalize("2021-07-01 10:00:00"), 1625158800); + BOOST_CHECK_EQUAL(normalize("2021-07-01 10:00:00 PDT"), 1625158800); + BOOST_CHECK_EQUAL(normalize("2021-07-01 09:00:00 PST"), 1625158800); // normalized to 10:00 PDT + BOOST_CHECK_EQUAL(normalize("2021-07-02 00:00:00") - normalize("2021-07-01 00:00:00"), 24*60*60); + + // On 2021-03-14, PST changes to PDT (23h day) + BOOST_CHECK(!is_dst("2021-03-14 00:00:00")); + BOOST_CHECK(is_dst("2021-03-14 23:59:59")); + BOOST_CHECK_EQUAL(normalize("2021-03-15 00:00:00") - normalize("2021-03-14 00:00:00"), 23*60*60); + + BOOST_CHECK_EQUAL(normalize("2021-03-14 01:59:59 PST"), 1615715999); + // The following three times do not exist on that day in that timezone. + // They are interpreted as UTC-8, which is the offset of PST. + BOOST_CHECK_EQUAL(normalize("2021-03-14 02:00:00 PST"), 1615716000); + BOOST_CHECK_EQUAL(normalize("2021-03-14 02:30:00 PST"), 1615717800); + BOOST_CHECK_EQUAL(normalize("2021-03-14 03:00:00 PST"), 1615719600); + + BOOST_CHECK_EQUAL(normalize("2021-03-14 03:00:00 PDT"), 1615716000); + // The following three times do not exist on that day in that timezone. + // They are interpreted as UTC-7, which is the offset of PDT. + BOOST_CHECK_EQUAL(normalize("2021-03-14 01:59:59 PDT"), 1615712399); + BOOST_CHECK_EQUAL(normalize("2021-03-14 02:00:00 PDT"), 1615712400); + BOOST_CHECK_EQUAL(normalize("2021-03-14 02:30:00 PDT"), 1615714200); + + BOOST_CHECK_EQUAL(normalize("2021-03-14 01:59:59"), 1615715999); + BOOST_CHECK_EQUAL(normalize("2021-03-14 03:00:00"), 1615716000); + // The following two times don't exist on that day, they are within the hour that is skipped. + // They are interpreted as UTC-8 (offset of PST) and then normalized to PDT. + BOOST_CHECK_EQUAL(normalize("2021-03-14 02:00:00"), 1615716000); + BOOST_CHECK_EQUAL(normalize("2021-03-14 02:30:00"), 1615717800); + + // On 2021-11-07, PDT changes to PST (25h day) + BOOST_CHECK(is_dst("2021-11-07 00:00:00")); + BOOST_CHECK(!is_dst("2021-11-07 23:59:59")); + BOOST_CHECK_EQUAL(normalize("2021-11-08 00:00:00") - normalize("2021-11-07 00:00:00"), 25*60*60); + + BOOST_CHECK_EQUAL(normalize("2021-11-07 00:59:59 PDT"), 1636271999); + BOOST_CHECK_EQUAL(normalize("2021-11-07 01:00:00 PDT"), 1636272000); + BOOST_CHECK_EQUAL(normalize("2021-11-07 01:30:00 PDT"), 1636273800); + BOOST_CHECK_EQUAL(normalize("2021-11-07 01:59:59 PDT"), 1636275599); + // The following time does not exist on that day in that timezone, it's interpreted as 01:00:00 PST. + BOOST_CHECK_EQUAL(normalize("2021-11-07 02:00:00 PDT"), 1636275600); + + // The following time does not exist on that day in that timezone, it's interpreted as 01:59:59 PDT. + BOOST_CHECK_EQUAL(normalize("2021-11-07 00:59:59 PST"), 1636275599); + BOOST_CHECK_EQUAL(normalize("2021-11-07 01:00:00 PST"), 1636275600); + BOOST_CHECK_EQUAL(normalize("2021-11-07 01:30:00 PST"), 1636277400); + BOOST_CHECK_EQUAL(normalize("2021-11-07 01:59:59 PST"), 1636279199); + BOOST_CHECK_EQUAL(normalize("2021-11-07 02:00:00 PST"), 1636279200); + + BOOST_CHECK_EQUAL(normalize("2021-11-07 00:59:59"), 1636271999); // unambiguous: PDT + BOOST_CHECK_EQUAL(normalize("2021-11-07 01:00:00"), 1636272000); // exists twice, interpreted as PDT + BOOST_CHECK_EQUAL(normalize("2021-11-07 01:30:00"), 1636273800); // exists twice, interpreted as PDT + BOOST_CHECK_EQUAL(normalize("2021-11-07 01:59:59"), 1636275599); // exists twice, interpreted as PDT + BOOST_CHECK_EQUAL(normalize("2021-11-07 02:00:00"), 1636279200); // unambiguous: PST +} + BOOST_AUTO_TEST_SUITE_END()