From de4b58a04fdc53a1a678c49d819f75e24ffe5fff Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Fri, 25 Apr 2025 16:09:54 +0200 Subject: [PATCH 1/6] tests: Move make_tm helper function to utils file Preparation to be able to use the function from different test files later on. --- test/CMakeLists.txt | 1 + test/icinga-legacytimeperiod.cpp | 30 +------------------------- test/utils.cpp | 36 ++++++++++++++++++++++++++++++++ test/utils.hpp | 8 +++++++ 4 files changed, 46 insertions(+), 29 deletions(-) create mode 100644 test/utils.cpp create mode 100644 test/utils.hpp diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 810b35bef..8ed300c1f 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -57,6 +57,7 @@ add_boost_test(types set(base_test_SOURCES icingaapplication-fixture.cpp + utils.cpp base-array.cpp base-base64.cpp base-convert.cpp diff --git a/test/icinga-legacytimeperiod.cpp b/test/icinga-legacytimeperiod.cpp index 8661b75f7..d279a1cbb 100644 --- a/test/icinga-legacytimeperiod.cpp +++ b/test/icinga-legacytimeperiod.cpp @@ -2,6 +2,7 @@ #include "base/utility.hpp" #include "icinga/legacytimeperiod.hpp" +#include "test/utils.hpp" #include #include #include @@ -471,35 +472,6 @@ BOOST_AUTO_TEST_CASE(advanced) AdvancedHelper("09:00:03-30:00:04", {{2014, 9, 24}, {9, 0, 3}}, {{2014, 9, 25}, {6, 0, 4}}); } -tm make_tm(std::string s) -{ - int dst = -1; - size_t l = strlen("YYYY-MM-DD HH:MM:SS"); - if (s.size() > l) { - std::string zone = s.substr(l); - if (zone == " PST") { - dst = 0; - } else if (zone == " PDT") { - dst = 1; - } else { - // tests should only use PST/PDT (for now) - BOOST_CHECK_MESSAGE(false, "invalid or unknown time time: " << zone); - } - } - - std::tm t = {}; -#if defined(__GNUC__) && __GNUC__ < 5 - // GCC did not implement std::get_time() until version 5 - strptime(s.c_str(), "%Y-%m-%d %H:%M:%S", &t); -#else /* defined(__GNUC__) && __GNUC__ < 5 */ - std::istringstream stream(s); - stream >> std::get_time(&t, "%Y-%m-%d %H:%M:%S"); -#endif /* defined(__GNUC__) && __GNUC__ < 5 */ - t.tm_isdst = dst; - - return t; -} - time_t make_time_t(const tm* t) { tm copy = *t; diff --git a/test/utils.cpp b/test/utils.cpp new file mode 100644 index 000000000..4008db4a1 --- /dev/null +++ b/test/utils.cpp @@ -0,0 +1,36 @@ +/* Icinga 2 | (c) 2025 Icinga GmbH | GPLv2+ */ + +#include "utils.hpp" +#include +#include +#include +#include + +tm make_tm(std::string s) +{ + int dst = -1; + size_t l = strlen("YYYY-MM-DD HH:MM:SS"); + if (s.size() > l) { + std::string zone = s.substr(l); + if (zone == " PST") { + dst = 0; + } else if (zone == " PDT") { + dst = 1; + } else { + // tests should only use PST/PDT (for now) + BOOST_CHECK_MESSAGE(false, "invalid or unknown time time: " << zone); + } + } + + std::tm t = {}; +#if defined(__GNUC__) && __GNUC__ < 5 + // GCC did not implement std::get_time() until version 5 + strptime(s.c_str(), "%Y-%m-%d %H:%M:%S", &t); +#else /* defined(__GNUC__) && __GNUC__ < 5 */ + std::istringstream stream(s); + stream >> std::get_time(&t, "%Y-%m-%d %H:%M:%S"); +#endif /* defined(__GNUC__) && __GNUC__ < 5 */ + t.tm_isdst = dst; + + return t; +} diff --git a/test/utils.hpp b/test/utils.hpp new file mode 100644 index 000000000..213c10e4b --- /dev/null +++ b/test/utils.hpp @@ -0,0 +1,8 @@ +/* Icinga 2 | (c) 2025 Icinga GmbH | GPLv2+ */ + +#pragma once + +#include +#include + +tm make_tm(std::string s); From 2458f686db271b6307cec98fd71573015c0e63ac Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Fri, 25 Apr 2025 16:17:28 +0200 Subject: [PATCH 2/6] tests: Remove GCC compatibility from make_tm We're using C++17, GCC only started implementing that in version 5, so there's no need for compatibility code for older versions any more. --- test/utils.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/utils.cpp b/test/utils.cpp index 4008db4a1..41b6a1067 100644 --- a/test/utils.cpp +++ b/test/utils.cpp @@ -23,13 +23,8 @@ tm make_tm(std::string s) } std::tm t = {}; -#if defined(__GNUC__) && __GNUC__ < 5 - // GCC did not implement std::get_time() until version 5 - strptime(s.c_str(), "%Y-%m-%d %H:%M:%S", &t); -#else /* defined(__GNUC__) && __GNUC__ < 5 */ std::istringstream stream(s); stream >> std::get_time(&t, "%Y-%m-%d %H:%M:%S"); -#endif /* defined(__GNUC__) && __GNUC__ < 5 */ t.tm_isdst = dst; return t; From 17c96783cf433e1a49ded86a1a19850b15889bf5 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Thu, 24 Apr 2025 17:05:31 +0200 Subject: [PATCH 3/6] tests: Move GlobalTimezoneFixture to utils file Prepare for adding test cases for DST changes in other files as well. --- test/icinga-legacytimeperiod.cpp | 48 ++------------------------------ test/utils.cpp | 36 ++++++++++++++++++++++++ test/utils.hpp | 17 +++++++++++ 3 files changed, 55 insertions(+), 46 deletions(-) diff --git a/test/icinga-legacytimeperiod.cpp b/test/icinga-legacytimeperiod.cpp index d279a1cbb..cecef81e5 100644 --- a/test/icinga-legacytimeperiod.cpp +++ b/test/icinga-legacytimeperiod.cpp @@ -16,52 +16,8 @@ using namespace icinga; BOOST_AUTO_TEST_SUITE(icinga_legacytimeperiod); -struct GlobalTimezoneFixture -{ - char *tz; - - GlobalTimezoneFixture(const char *fixed_tz = "") - { - tz = getenv("TZ"); -#ifdef _WIN32 - _putenv_s("TZ", fixed_tz == "" ? "UTC" : fixed_tz); -#else - setenv("TZ", fixed_tz, 1); -#endif - tzset(); - } - - ~GlobalTimezoneFixture() - { -#ifdef _WIN32 - if (tz) - _putenv_s("TZ", tz); - else - _putenv_s("TZ", ""); -#else - if (tz) - setenv("TZ", tz, 1); - else - unsetenv("TZ"); -#endif - tzset(); - } -}; - BOOST_GLOBAL_FIXTURE(GlobalTimezoneFixture); -// DST changes in America/Los_Angeles: -// 2021-03-14: 01:59:59 PST (UTC-8) -> 03:00:00 PDT (UTC-7) -// 2021-11-07: 01:59:59 PDT (UTC-7) -> 01:00:00 PST (UTC-8) -#ifndef _WIN32 -static const char *dst_test_timezone = "America/Los_Angeles"; -#else /* _WIN32 */ -// Tests are using pacific time because Windows only really supports timezones following US DST rules with the TZ -// environment variable. Format is "[Standard TZ][negative UTC offset][DST TZ]". -// https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/tzset?view=msvc-160#remarks -static const char *dst_test_timezone = "PST8PDT"; -#endif /* _WIN32 */ - BOOST_AUTO_TEST_CASE(simple) { tm tm_beg, tm_end, tm_ref; @@ -523,7 +479,7 @@ std::ostream& operator<<(std::ostream& o, const boost::optional& s) BOOST_AUTO_TEST_CASE(dst) { - GlobalTimezoneFixture tz(dst_test_timezone); + GlobalTimezoneFixture tz(GlobalTimezoneFixture::TestTimezoneWithDST); // Self-tests for helper functions BOOST_CHECK_EQUAL(make_tm("2021-11-07 02:30:00").tm_isdst, -1); @@ -802,7 +758,7 @@ BOOST_AUTO_TEST_CASE(dst) // This tests checks that TimePeriod::IsInside() always returns true for a 24x7 period, even around DST changes. BOOST_AUTO_TEST_CASE(dst_isinside) { - GlobalTimezoneFixture tz(dst_test_timezone); + GlobalTimezoneFixture tz(GlobalTimezoneFixture::TestTimezoneWithDST); Function::Ptr update = new Function("LegacyTimePeriod", LegacyTimePeriod::ScriptFunc, {"tp", "begin", "end"}); Dictionary::Ptr ranges = new Dictionary({ diff --git a/test/utils.cpp b/test/utils.cpp index 41b6a1067..316763670 100644 --- a/test/utils.cpp +++ b/test/utils.cpp @@ -29,3 +29,39 @@ tm make_tm(std::string s) return t; } + +#ifndef _WIN32 +const char *GlobalTimezoneFixture::TestTimezoneWithDST = "America/Los_Angeles"; +#else /* _WIN32 */ +// Tests are using pacific time because Windows only really supports timezones following US DST rules with the TZ +// environment variable. Format is "[Standard TZ][negative UTC offset][DST TZ]". +// https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/tzset?view=msvc-160#remarks +const char *GlobalTimezoneFixture::TestTimezoneWithDST = "PST8PDT"; +#endif /* _WIN32 */ + +GlobalTimezoneFixture::GlobalTimezoneFixture(const char *fixed_tz) +{ + tz = getenv("TZ"); +#ifdef _WIN32 + _putenv_s("TZ", fixed_tz == "" ? "UTC" : fixed_tz); +#else + setenv("TZ", fixed_tz, 1); +#endif + tzset(); +} + +GlobalTimezoneFixture::~GlobalTimezoneFixture() +{ +#ifdef _WIN32 + if (tz) + _putenv_s("TZ", tz); + else + _putenv_s("TZ", ""); +#else + if (tz) + setenv("TZ", tz, 1); + else + unsetenv("TZ"); +#endif + tzset(); +} diff --git a/test/utils.hpp b/test/utils.hpp index 213c10e4b..af44d3b88 100644 --- a/test/utils.hpp +++ b/test/utils.hpp @@ -6,3 +6,20 @@ #include tm make_tm(std::string s); + +struct GlobalTimezoneFixture +{ + /** + * Timezone used for testing DST changes. + * + * DST changes in America/Los_Angeles: + * 2021-03-14: 01:59:59 PST (UTC-8) -> 03:00:00 PDT (UTC-7) + * 2021-11-07: 01:59:59 PDT (UTC-7) -> 01:00:00 PST (UTC-8) + */ + static const char *TestTimezoneWithDST; + + GlobalTimezoneFixture(const char *fixed_tz = ""); + ~GlobalTimezoneFixture(); + + char *tz; +}; From 5404143dee89e7e4f338dfcfffca2446b249f633 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Thu, 24 Apr 2025 17:15:51 +0200 Subject: [PATCH 4/6] Ensure consistent mktime() DST behavior across different implementations There are inputs to mktime() where the behavior is not specified and there's also no single obviously correct behavior. In particular, this affects how auto-detection of whether DST is in effect is done when tm_isdst = -1 is set and the time specified does not exist at all or exists twice on that day. If different implementations are used within an Icinga 2 cluster, that can lead to inconsistent behavior because different nodes may interpret the same TimePeriod differently. This commit introduces a wrapper to mktime(), namely Utility::NormalizeTm() that implements the behavior provided by glibc. The choice for glibc's behavior is pretty arbitrary, it was simply picked because most systems that are officially/fully supported use it (with the only exception being Windows), so this should give the least possible amount of user-visible changes. As part of this commit, the closely related helper function mktime_const() is also moved to Utility::TmToTimestamp() and made a wrapper around the newly introduced NormalizeTm(). --- lib/base/datetime.cpp | 2 +- lib/base/utility.cpp | 48 +++++++++++++++++++ lib/base/utility.hpp | 3 ++ lib/icinga/legacytimeperiod.cpp | 45 +++++++----------- test/CMakeLists.txt | 1 + test/base-utility.cpp | 83 +++++++++++++++++++++++++++++++++ 6 files changed, 153 insertions(+), 29 deletions(-) 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() From 379d7638ed686c39edaeae6844bcb96c1b71e482 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Tue, 22 Apr 2025 17:07:41 +0200 Subject: [PATCH 5/6] tests: Remove special cases for Windows in icinga_legacytimeperiod/dst Ideally, Icinga 2 should behave consistenly across platforms. These special cases only existed because mktime() on Windows behaved differently than the implementation in glibc. With the introduction of Utility::NormalizeTm(), there's now consistent behavior and the other expected results for windows are no longer necessary (ideally, they shouldn't have existed in the first place). --- test/icinga-legacytimeperiod.cpp | 55 -------------------------------- 1 file changed, 55 deletions(-) diff --git a/test/icinga-legacytimeperiod.cpp b/test/icinga-legacytimeperiod.cpp index cecef81e5..4eb5fb92e 100644 --- a/test/icinga-legacytimeperiod.cpp +++ b/test/icinga-legacytimeperiod.cpp @@ -517,13 +517,8 @@ BOOST_AUTO_TEST_CASE(dst) day, "01:30-02:30", {make_tm("2021-03-14 01:00:00 PST")}, {make_tm("2021-03-14 01:59:59 PST")}, -#ifndef _WIN32 // As 02:30 does not exist on this day, it is parsed as if it was 02:30 PST which is actually 03:30 PDT. Segment("2021-03-14 01:30:00 PST", "2021-03-14 03:30:00 PDT"), -#else - // Windows interpretes 02:30 as 01:30 PST, so it is an empty segment. - boost::none, -#endif }); } @@ -533,14 +528,9 @@ BOOST_AUTO_TEST_CASE(dst) day, "02:30-03:30", {make_tm("2021-03-14 01:00:00 PST")}, {make_tm("2021-03-14 03:00:00 PDT")}, -#ifndef _WIN32 // As 02:30 does not exist on this day, it is parsed as if it was 02:30 PST which is actually 03:30 PDT. // Therefore, the result is a segment from 03:30 PDT to 03:30 PDT with a duration of 0, i.e. no segment. boost::none, -#else - // Windows parses non-existing 02:30 as 01:30 PST, resulting in an 1 hour segment. - Segment("2021-03-14 01:30:00 PST", "2021-03-14 03:30:00 PDT"), -#endif }); } @@ -549,13 +539,8 @@ BOOST_AUTO_TEST_CASE(dst) day, "02:15-03:45", {make_tm("2021-03-14 01:00:00 PST")}, {make_tm("2021-03-14 03:30:00 PDT")}, -#ifndef _WIN32 // As 02:15 does not exist on this day, it is parsed as if it was 02:15 PST which is actually 03:15 PDT. Segment("2021-03-14 03:15:00 PDT", "2021-03-14 03:45:00 PDT"), -#else - // Windows interprets 02:15 as 01:15 PST though. - Segment("2021-03-14 01:15:00 PST", "2021-03-14 03:45:00 PDT"), -#endif }); // range after DST change @@ -587,7 +572,6 @@ BOOST_AUTO_TEST_CASE(dst) if (day.find("sunday") == std::string::npos) { // skip for non-absolute day specs (would find another sunday) // range existing twice during DST change (first instance) -#ifndef _WIN32 tests.push_back(TestData{ day, "01:15-01:45", {make_tm("2021-11-07 01:00:00 PDT")}, @@ -595,15 +579,6 @@ BOOST_AUTO_TEST_CASE(dst) // Duplicate times are interpreted as the first occurrence. Segment("2021-11-07 01:15:00 PDT", "2021-11-07 01:45:00 PDT"), }); -#else - tests.push_back(TestData{ - day, "01:15-01:45", - {make_tm("2021-11-07 01:00:00 PDT")}, - {make_tm("2021-11-07 01:30:00 PST")}, - // However, Windows always uses the second occurrence. - Segment("2021-11-07 01:15:00 PST", "2021-11-07 01:45:00 PST"), - }); -#endif } if (day.find("sunday") == std::string::npos) { // skip for non-absolute day specs (would find another sunday) @@ -612,13 +587,8 @@ BOOST_AUTO_TEST_CASE(dst) day, "01:15-01:45", {make_tm("2021-11-07 01:00:00 PST")}, {make_tm("2021-11-07 01:30:00 PST")}, -#ifndef _WIN32 // Interpreted as the first occurrence, so it's in the past. boost::none, -#else - // On Windows, it's the second occurrence, so it's still in the present/future and is found. - Segment("2021-11-07 01:15:00 PST", "2021-11-07 01:45:00 PST"), -#endif }); } @@ -644,13 +614,8 @@ BOOST_AUTO_TEST_CASE(dst) day, "00:30-01:30", {make_tm("2021-11-07 00:00:00 PDT")}, {make_tm("2021-11-07 01:00:00 PDT")}, -#ifndef _WIN32 // Both times are interpreted as the first instance on that day (i.e both PDT). Segment("2021-11-07 00:30:00 PDT", "2021-11-07 01:30:00 PDT") -#else - // Windows interprets duplicate times as the second instance (i.e. both PST). - Segment("2021-11-07 00:30:00 PDT", "2021-11-07 01:30:00 PST") -#endif }); // range beginning during duplicate DST hour (first instance) @@ -658,18 +623,12 @@ BOOST_AUTO_TEST_CASE(dst) day, "01:30-02:30", {make_tm("2021-11-07 01:00:00 PDT")}, {make_tm("2021-11-07 02:00:00 PST")}, -#ifndef _WIN32 // 01:30 is interpreted as the first occurrence (PDT) but since there's no 02:30 PDT, it's PST. Segment("2021-11-07 01:30:00 PDT", "2021-11-07 02:30:00 PST") -#else - // Windows interprets both as PST though. - Segment("2021-11-07 01:30:00 PST", "2021-11-07 02:30:00 PST") -#endif }); if (day.find("sunday") == std::string::npos) { // skip for non-absolute day specs (would find another sunday) // range ending during duplicate DST hour (second instance) -#ifndef _WIN32 tests.push_back(TestData{ day, "00:30-01:30", {make_tm("2021-11-07 00:00:00 PST")}, @@ -678,15 +637,6 @@ BOOST_AUTO_TEST_CASE(dst) // 01:00 PST (02:00 PDT) is after the segment. boost::none, }); -#else - tests.push_back(TestData{ - day, "00:30-01:30", - {make_tm("2021-11-07 00:00:00 PDT")}, - {make_tm("2021-11-07 01:00:00 PST")}, - // As Windows interprets the end as PST, it's still in the future and the segment is found. - Segment("2021-11-07 00:30:00 PDT", "2021-11-07 01:30:00 PST"), - }); -#endif } // range beginning during duplicate DST hour (second instance) @@ -694,13 +644,8 @@ BOOST_AUTO_TEST_CASE(dst) day, "01:30-02:30", {make_tm("2021-11-07 01:00:00 PDT")}, {make_tm("2021-11-07 02:00:00 PST")}, -#ifndef _WIN32 // As 01:30 always refers to the first occurrence (PDT), this is actually a 2 hour segment. Segment("2021-11-07 01:30:00 PDT", "2021-11-07 02:30:00 PST"), -#else - // On Windows, it refers t the second occurrence (PST), therefore it's an 1 hour segment. - Segment("2021-11-07 01:30:00 PST", "2021-11-07 02:30:00 PST"), -#endif }); } From 3c9e06972d094d079fd8856aea43ab78b10d8802 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Tue, 22 Apr 2025 17:28:39 +0200 Subject: [PATCH 6/6] Don't disable icinga_legacytimeperiod/dst test on Alpine The test was disabled due to a difference in behavior of mktime() between glibc and musl. This inconsistency was fixed with the introduction of Utility::NormalizeTm() and Alpine no longer needs this special case. --- .github/workflows/linux.bash | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.github/workflows/linux.bash b/.github/workflows/linux.bash index 6c4240d1a..7eabec9a3 100755 --- a/.github/workflows/linux.bash +++ b/.github/workflows/linux.bash @@ -14,11 +14,6 @@ case "$DISTRO" in # https://gitlab.alpinelinux.org/alpine/aports/-/blob/master/community/icinga2/APKBUILD apk add bison boost-dev ccache cmake flex g++ libedit-dev libressl-dev ninja-build tzdata ln -vs /usr/lib/ninja-build/bin/ninja /usr/local/bin/ninja - - # This test fails due to some glibc/musl mismatch regarding timezone PST/PDT. - # - https://www.openwall.com/lists/musl/2024/03/05/2 - # - https://gitlab.alpinelinux.org/alpine/aports/-/blob/b3ea02e2251451f9511086e1970f21eb640097f7/community/icinga2/disable-failing-tests.patch - sed -i '/icinga_legacytimeperiod\/dst$/d' /icinga2/test/CMakeLists.txt ;; amazonlinux:2)