From 5964d7cf7837c3f516617fdf0c2557cd08756906 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Klemens=20Sch=C3=B6lhorn?= Date: Wed, 21 Aug 2019 15:06:14 +0200 Subject: [PATCH 2/2] CachedDateFormat: Fix handling of ts with 654 as ms part findMillisecondStart tries to find the position of the milliseconds in a formatted timestamp by comparing the formatting to one where the millisecond part of the timestamp was replaced with 654. If the timestamp already has this millisecond part, it will switch to a different value: 987. However the check if it should switch to 987 was broken, as it was comparing the magic number in microseconds to the millisecond part of the timestamp in milliseconds, so this always results in false. This leads to the comparison timestamp being identical and the function falsely returning NO_MILLISECONDS. The end result is that if the first timestamp we format since the beginning of the current second has 654 as milliseconds, for all following formatted timestamps the milliseconds will be identical when within the same second, as the code never tries to find the millisecond start again, but still happily uses the cache within one second without updating milliseconds. The fix itself is pretty easy: We just need to compare using the correct units in findMillisecondStart. This commit also adds a testcase that demonstrates the issue. Fixes LOGCXX-506. --- src/main/cpp/cacheddateformat.cpp | 7 ++++--- src/test/cpp/helpers/cacheddateformattestcase.cpp | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/main/cpp/cacheddateformat.cpp b/src/main/cpp/cacheddateformat.cpp index 45b374ab..c813eceb 100644 --- a/src/main/cpp/cacheddateformat.cpp +++ b/src/main/cpp/cacheddateformat.cpp @@ -124,12 +124,13 @@ int CachedDateFormat::findMillisecondStart( slotBegin -= 1000000; } - int millis = (int) (time - slotBegin) / 1000; + int micros = (int) (time - slotBegin); + // the magic numbers are in microseconds int magic = magic1; LogString magicString(magicString1); - if (millis == magic1) + if (micros == magic1) { magic = magic2; magicString = magicString2; @@ -157,7 +158,7 @@ int CachedDateFormat::findMillisecondStart( // determine the expected digits for the base time const logchar abc[] = { 0x41, 0x42, 0x43, 0 }; LogString formattedMillis(abc); - millisecondFormat(millis, formattedMillis, 0); + millisecondFormat(micros / 1000, formattedMillis, 0); LogString plusZero; formatter->format(plusZero, slotBegin, pool); diff --git a/src/test/cpp/helpers/cacheddateformattestcase.cpp b/src/test/cpp/helpers/cacheddateformattestcase.cpp index 777680d3..7148834a 100644 --- a/src/test/cpp/helpers/cacheddateformattestcase.cpp +++ b/src/test/cpp/helpers/cacheddateformattestcase.cpp @@ -18,6 +18,7 @@ #include #include "../logunit.h" #include +#include #include #include #include @@ -76,6 +77,7 @@ LOGUNIT_CLASS(CachedDateFormatTestCase) LOGUNIT_TEST(test19); LOGUNIT_TEST(test20); LOGUNIT_TEST(test21); + LOGUNIT_TEST(test22); LOGUNIT_TEST_SUITE_END(); #define MICROSECONDS_PER_DAY APR_INT64_C(86400000000) @@ -642,6 +644,26 @@ void test21() { LOGUNIT_ASSERT_EQUAL(1000, maxValid); } +/** + * Check that findMillisecondStart correctly handles timestamps + * which use the magic millisecond value 654. + */ +void test22() { + DateFormatPtr baseFormatter(new ISO8601DateFormat()); + CachedDateFormat isoFormat(baseFormatter, 1000000); + isoFormat.setTimeZone(TimeZone::getGMT()); + + Pool p; + LogString formatted; + + isoFormat.format(formatted, 654000, p); + LOGUNIT_ASSERT_EQUAL(LOG4CXX_STR("1970-01-01 00:00:00,654"), formatted); + + formatted.clear(); + isoFormat.format(formatted, 999000, p); + LOGUNIT_ASSERT_EQUAL(LOG4CXX_STR("1970-01-01 00:00:00,999"), formatted); +} + }; LOGUNIT_TEST_SUITE_REGISTRATION(CachedDateFormatTestCase); -- 2.16.1.2.g9d582f143