Skip to content

Commit 8c51b15

Browse files
julianbrostjulianbrost
committed
DST inconsistency fix
1 parent 57c0762 commit 8c51b15

File tree

6 files changed

+168
-29
lines changed

6 files changed

+168
-29
lines changed

‎lib/base/datetime.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ DateTime::DateTime(const std::vector<Value>& args)
3535

3636
tms.tm_isdst = -1;
3737

38-
m_Value = mktime(&tms);
38+
m_Value = Utility::TmToTimestamp(&tms);
3939
} else if (args.size() == 1)
4040
m_Value = args[0];
4141
else

‎lib/base/utility.cpp

+48
Original file line numberDiff line numberDiff line change
@@ -1958,3 +1958,51 @@ bool Utility::ComparePasswords(const String& enteredPassword, const String& actu
19581958

19591959
return result;
19601960
}
1961+
1962+
/**
1963+
* Normalizes the given struct tm like mktime() from libc does with some exception for DST handling: If the given time
1964+
* 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
1965+
* interpreted using the UTC offset of the standard timezone and then normalized.
1966+
*
1967+
* This is done in order to provide consistent behavior across operating systems. Historically, Icinga 2 just relied on
1968+
* whatever mktime() of the operating system did and this function mimics what glibc does as that's what most systems
1969+
* use.
1970+
*
1971+
* @param t tm struct to be normalized
1972+
* @return time_t representing the timestamp given by t
1973+
*/
1974+
time_t Utility::NormalizeTm(tm *t)
1975+
{
1976+
// If tm_isdst already specifies the timezone (0 or 1), just use the mktime() behavior.
1977+
if (t->tm_isdst >= 0) {
1978+
return mktime(t);
1979+
}
1980+
1981+
const tm copy = *t;
1982+
1983+
t->tm_isdst = 1;
1984+
time_t result = mktime(t);
1985+
if (result != -1 && t->tm_isdst == 1) {
1986+
return result;
1987+
}
1988+
1989+
// Restore the original input. mktime() can (and does) change more fields than just tm_isdst by converting from
1990+
// daylight saving time to standard time (it moves the contents by (typically) an hour, which can move across
1991+
// days/weeks/months/years changing all other fields).
1992+
*t = copy;
1993+
1994+
t->tm_isdst = 0;
1995+
return mktime(t);
1996+
}
1997+
1998+
/**
1999+
* Returns the same as NormalizeTm() but takes a const pointer as argument and thus does not modify it.
2000+
*
2001+
* @param t struct tm to convert to time_t
2002+
* @return time_t representing the timestamp given by t
2003+
*/
2004+
time_t Utility::TmToTimestamp(const tm *t)
2005+
{
2006+
tm copy = *t;
2007+
return NormalizeTm(&copy);
2008+
}

‎lib/base/utility.hpp

+3
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,9 @@ class Utility
185185
return in.SubStr(0, maxLength - sha1HexLength - strlen(trunc)) + trunc + SHA1(in);
186186
}
187187

188+
static time_t NormalizeTm(tm *t);
189+
static time_t TmToTimestamp(const tm *t);
190+
188191
private:
189192
Utility();
190193

‎lib/icinga/legacytimeperiod.cpp

+17-28
Original file line numberDiff line numberDiff line change
@@ -13,23 +13,12 @@ using namespace icinga;
1313

1414
REGISTER_FUNCTION_NONCONST(Internal, LegacyTimePeriod, &LegacyTimePeriod::ScriptFunc, "tp:begin:end");
1515

16-
/**
17-
* Returns the same as mktime() but does not modify its argument and takes a const pointer.
18-
*
19-
* @param t struct tm to convert to time_t
20-
* @return time_t representing the timestamp given by t
21-
*/
22-
static time_t mktime_const(const tm *t) {
23-
tm copy = *t;
24-
return mktime(&copy);
25-
}
26-
2716
bool LegacyTimePeriod::IsInTimeRange(const tm *begin, const tm *end, int stride, const tm *reference)
2817
{
2918
time_t tsbegin, tsend, tsref;
30-
tsbegin = mktime_const(begin);
31-
tsend = mktime_const(end);
32-
tsref = mktime_const(reference);
19+
tsbegin = Utility::TmToTimestamp(begin);
20+
tsend = Utility::TmToTimestamp(end);
21+
tsref = Utility::TmToTimestamp(reference);
3322

3423
if (tsref < tsbegin || tsref >= tsend)
3524
return false;
@@ -85,7 +74,7 @@ void LegacyTimePeriod::FindNthWeekday(int wday, int n, tm *reference)
8574
t.tm_sec = 0;
8675
t.tm_isdst = -1;
8776

88-
mktime(&t);
77+
Utility::NormalizeTm(&t);
8978

9079
if (t.tm_wday == wday) {
9180
seen++;
@@ -398,8 +387,8 @@ bool LegacyTimePeriod::IsInDayDefinition(const String& daydef, const tm *referen
398387
ParseTimeRange(daydef, &begin, &end, &stride, reference);
399388

400389
Log(LogDebug, "LegacyTimePeriod")
401-
<< "ParseTimeRange: '" << daydef << "' => " << mktime(&begin)
402-
<< " -> " << mktime(&end) << ", stride: " << stride;
390+
<< "ParseTimeRange: '" << daydef << "' => " << Utility::TmToTimestamp(&begin)
391+
<< " -> " << Utility::TmToTimestamp(&end) << ", stride: " << stride;
403392

404393
return IsInTimeRange(&begin, &end, stride, reference);
405394
}
@@ -448,8 +437,8 @@ Dictionary::Ptr LegacyTimePeriod::ProcessTimeRange(const String& timestamp, cons
448437
ProcessTimeRangeRaw(timestamp, reference, &begin, &end);
449438

450439
return new Dictionary({
451-
{ "begin", (long)mktime(&begin) },
452-
{ "end", (long)mktime(&end) }
440+
{ "begin", (long)Utility::TmToTimestamp(&begin) },
441+
{ "end", (long)Utility::TmToTimestamp(&end) }
453442
});
454443
}
455444

@@ -480,13 +469,13 @@ Dictionary::Ptr LegacyTimePeriod::FindRunningSegment(const String& daydef, const
480469
time_t tsend, tsiter, tsref;
481470
int stride;
482471

483-
tsref = mktime_const(reference);
472+
tsref = Utility::TmToTimestamp(reference);
484473

485474
ParseTimeRange(daydef, &begin, &end, &stride, reference);
486475

487476
iter = begin;
488477

489-
tsend = mktime(&end);
478+
tsend = Utility::NormalizeTm(&end);
490479

491480
do {
492481
if (IsInTimeRange(&begin, &end, stride, &iter)) {
@@ -518,7 +507,7 @@ Dictionary::Ptr LegacyTimePeriod::FindRunningSegment(const String& daydef, const
518507
iter.tm_hour = 0;
519508
iter.tm_min = 0;
520509
iter.tm_sec = 0;
521-
tsiter = mktime(&iter);
510+
tsiter = Utility::NormalizeTm(&iter);
522511
} while (tsiter < tsend);
523512

524513
return nullptr;
@@ -538,13 +527,13 @@ Dictionary::Ptr LegacyTimePeriod::FindNextSegment(const String& daydef, const St
538527
ref.tm_mday++;
539528
}
540529

541-
tsref = mktime(&ref);
530+
tsref = Utility::NormalizeTm(&ref);
542531

543532
ParseTimeRange(daydef, &begin, &end, &stride, &ref);
544533

545534
iter = begin;
546535

547-
tsend = mktime(&end);
536+
tsend = Utility::NormalizeTm(&end);
548537

549538
do {
550539
if (IsInTimeRange(&begin, &end, stride, &iter)) {
@@ -575,7 +564,7 @@ Dictionary::Ptr LegacyTimePeriod::FindNextSegment(const String& daydef, const St
575564
iter.tm_hour = 0;
576565
iter.tm_min = 0;
577566
iter.tm_sec = 0;
578-
tsiter = mktime(&iter);
567+
tsiter = Utility::NormalizeTm(&iter);
579568
} while (tsiter < tsend);
580569
}
581570

@@ -607,17 +596,17 @@ Array::Ptr LegacyTimePeriod::ScriptFunc(const TimePeriod::Ptr& tp, double begin,
607596
t->tm_isdst = -1;
608597

609598
// Normalize fields using mktime.
610-
mktime(t);
599+
Utility::NormalizeTm(t);
611600

612601
// Reset tm_isdst so that future calls figure out the correct time zone after setting tm_hour/tm_min/tm_sec.
613602
t->tm_isdst = -1;
614603
};
615604

616-
for (tm reference = tm_begin; mktime_const(&reference) <= end; advance_to_next_day(&reference)) {
605+
for (tm reference = tm_begin; Utility::TmToTimestamp(&reference) <= end; advance_to_next_day(&reference)) {
617606

618607
#ifdef I2_DEBUG
619608
Log(LogDebug, "LegacyTimePeriod")
620-
<< "Checking reference time " << mktime_const(&reference);
609+
<< "Checking reference time " << Utility::TmToTimestamp(&reference);
621610
#endif /* I2_DEBUG */
622611

623612
ObjectLock olock(ranges);

‎test/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ add_boost_test(base
186186
base_utility/EscapeCreateProcessArg
187187
base_utility/TruncateUsingHash
188188
base_utility/FormatDateTime
189+
base_utility/NormalizeTm
189190
base_value/scalar
190191
base_value/convert
191192
base_value/format

‎test/base-utility.cpp

+98
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include "base/utility.hpp"
44
#include <chrono>
55
#include <BoostTestTargetConfig.h>
6+
#include "timezone-fixture.hpp"
67

78
#ifdef _WIN32
89
# include <windows.h>
@@ -230,4 +231,101 @@ BOOST_AUTO_TEST_CASE(FormatDateTime) {
230231
BOOST_CHECK_THROW(Utility::FormatDateTime("%Y", positive_out_of_range), positive_overflow);
231232
}
232233

234+
// TODO: deduplicate with timeperiod tests
235+
static tm make_tm(std::string s)
236+
{
237+
std::tm t = {};
238+
std::istringstream stream(s);
239+
stream >> std::get_time(&t, "%Y-%m-%d %H:%M:%S");
240+
t.tm_isdst = -1;
241+
size_t l = strlen("YYYY-MM-DD HH:MM:SS");
242+
if (s.size() > l) {
243+
std::string zone = s.substr(l);
244+
if (zone == " PST") {
245+
t.tm_isdst = 0;
246+
} else if (zone == " PDT") {
247+
t.tm_isdst = 1;
248+
} else {
249+
// tests should only use PST/PDT (for now)
250+
BOOST_CHECK_MESSAGE(false, "invalid or unknown time time: " << zone);
251+
}
252+
}
253+
254+
return t;
255+
}
256+
257+
BOOST_AUTO_TEST_CASE(NormalizeTm)
258+
{
259+
GlobalTimezoneFixture tz(GlobalTimezoneFixture::TestTimezoneWithDST);
260+
261+
auto normalize = [](const std::string_view& input) {
262+
tm t = make_tm(std::string(input));
263+
return Utility::NormalizeTm(&t);
264+
};
265+
266+
auto is_dst = [](const std::string_view& input) {
267+
tm t = make_tm(std::string(input));
268+
Utility::NormalizeTm(&t);
269+
BOOST_CHECK_GE(t.tm_isdst, 0);
270+
return t.tm_isdst > 0;
271+
};
272+
273+
// The whole day 2021-01-01 uses PST (24h day)
274+
BOOST_CHECK(!is_dst("2021-01-01 10:00:00"));
275+
BOOST_CHECK_EQUAL(normalize("2021-01-01 10:00:00"), 1609524000);
276+
BOOST_CHECK_EQUAL(normalize("2021-01-01 10:00:00 PST"), 1609524000);
277+
BOOST_CHECK_EQUAL(normalize("2021-01-01 11:00:00 PDT"), 1609524000); // normalized to 10:00 PST
278+
BOOST_CHECK_EQUAL(normalize("2021-01-02 00:00:00") - normalize("2021-01-01 00:00:00"), 24*60*60);
279+
280+
// The whole day 2021-07-01 uses PDT (24h day)
281+
BOOST_CHECK(is_dst("2021-07-01 10:00:00"));
282+
BOOST_CHECK_EQUAL(normalize("2021-07-01 10:00:00"), 1625158800);
283+
BOOST_CHECK_EQUAL(normalize("2021-07-01 10:00:00 PDT"), 1625158800);
284+
BOOST_CHECK_EQUAL(normalize("2021-07-01 09:00:00 PST"), 1625158800); // normalized to 10:00 PDT
285+
BOOST_CHECK_EQUAL(normalize("2021-07-02 00:00:00") - normalize("2021-07-01 00:00:00"), 24*60*60);
286+
287+
// On 2021-03-14, PST changes to PDT (23h day)
288+
BOOST_CHECK(!is_dst("2021-03-14 00:00:00"));
289+
BOOST_CHECK(is_dst("2021-03-14 23:59:59"));
290+
BOOST_CHECK_EQUAL(normalize("2021-03-15 00:00:00") - normalize("2021-03-14 00:00:00"), 23*60*60);
291+
292+
BOOST_CHECK_EQUAL(normalize("2021-03-14 01:59:59 PST"), 1615715999);
293+
BOOST_CHECK_EQUAL(normalize("2021-03-14 02:00:00 PST"), 1615716000); // does not actually exist, UTC-8
294+
BOOST_CHECK_EQUAL(normalize("2021-03-14 02:30:00 PST"), 1615717800); // does not actually exist, UTC-8
295+
BOOST_CHECK_EQUAL(normalize("2021-03-14 03:00:00 PST"), 1615719600); // does not actually exist, UTC-8
296+
297+
BOOST_CHECK_EQUAL(normalize("2021-03-14 01:59:59 PDT"), 1615712399); // does not actually exist, UTC-7
298+
BOOST_CHECK_EQUAL(normalize("2021-03-14 02:00:00 PDT"), 1615712400); // does not actually exist, UTC-7
299+
BOOST_CHECK_EQUAL(normalize("2021-03-14 02:30:00 PDT"), 1615714200); // does not actually exist, UTC-7
300+
BOOST_CHECK_EQUAL(normalize("2021-03-14 03:00:00 PDT"), 1615716000);
301+
302+
BOOST_CHECK_EQUAL(normalize("2021-03-14 01:59:59"), 1615715999);
303+
BOOST_CHECK_EQUAL(normalize("2021-03-14 02:00:00"), 1615716000); // does not actually exist, 03:00 PDT
304+
BOOST_CHECK_EQUAL(normalize("2021-03-14 02:30:00"), 1615717800); // does not actually exist, 03:30 PDT
305+
BOOST_CHECK_EQUAL(normalize("2021-03-14 03:00:00"), 1615716000);
306+
307+
// On 2021-11-07, PDT changes to PST (25h day)
308+
BOOST_CHECK(is_dst("2021-11-07 00:00:00"));
309+
BOOST_CHECK(!is_dst("2021-11-07 23:59:59"));
310+
BOOST_CHECK_EQUAL(normalize("2021-11-08 00:00:00") - normalize("2021-11-07 00:00:00"), 25*60*60);
311+
312+
BOOST_CHECK_EQUAL(normalize("2021-11-07 00:59:59 PDT"), 1636271999);
313+
BOOST_CHECK_EQUAL(normalize("2021-11-07 01:00:00 PDT"), 1636272000);
314+
BOOST_CHECK_EQUAL(normalize("2021-11-07 01:30:00 PDT"), 1636273800);
315+
BOOST_CHECK_EQUAL(normalize("2021-11-07 01:59:59 PDT"), 1636275599);
316+
BOOST_CHECK_EQUAL(normalize("2021-11-07 02:00:00 PDT"), 1636275600); // does not actually exist, 1:00 PST
317+
318+
BOOST_CHECK_EQUAL(normalize("2021-11-07 00:59:59 PST"), 1636275599); // does not actually exist, 1:59 PDT
319+
BOOST_CHECK_EQUAL(normalize("2021-11-07 01:00:00 PST"), 1636275600);
320+
BOOST_CHECK_EQUAL(normalize("2021-11-07 01:30:00 PST"), 1636277400);
321+
BOOST_CHECK_EQUAL(normalize("2021-11-07 01:59:59 PST"), 1636279199);
322+
BOOST_CHECK_EQUAL(normalize("2021-11-07 02:00:00 PST"), 1636279200);
323+
324+
BOOST_CHECK_EQUAL(normalize("2021-11-07 00:59:59"), 1636271999); // unambiguous: PDT
325+
BOOST_CHECK_EQUAL(normalize("2021-11-07 01:00:00"), 1636272000); // exists twice, normalized to PDT
326+
BOOST_CHECK_EQUAL(normalize("2021-11-07 01:30:00"), 1636273800); // exists twice, normalized to PDT
327+
BOOST_CHECK_EQUAL(normalize("2021-11-07 01:59:59"), 1636275599); // exists twice, normalized to PDT
328+
BOOST_CHECK_EQUAL(normalize("2021-11-07 02:00:00"), 1636279200); // unambiguous: PST
329+
}
330+
233331
BOOST_AUTO_TEST_SUITE_END()

0 commit comments

Comments
 (0)