Uploaded image for project: 'Log4cxx'
  1. Log4cxx
  2. LOGCXX-506

CachedDateFormat reuses timestamps without updating milliseconds after formatting timestamp with ms == 654

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 0.10.0, 0.11.0
    • Fix Version/s: 0.11.0
    • Component/s: None
    • Labels:
      None
    • Flags:
      Patch

      Description

      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 is broken, as it is 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.

       

      I could reproduce this on the v0_10_0 tag and on latest master under RHEL 7.4. Patches with a testcase and the fix against master are attached (they can basically be also applied to 0.10, although the code formatting changed...).

      The first patch removes the magic numbers from the header and puts them next to the magic strings. This is not necessary for the fix, but it improves readability a lot (and maybe breaks abi?).

        Attachments

          Activity

            People

            • Assignee:
              tschoening Thorsten Schöning
              Reporter:
              klemens Klemens Schölhorn
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: