Fix corner case in QTimeZonePrivate::dataForLocalTime()

If the local time for which we want data is after the last known
transition, the two transitions we get to bracket it are the last
known and an invalid one. The code checked the former was valid, but
neglected to check the latter, leading to nonsense arithmetic later in
the function. In this situation we unequivocally want the last known
transition, so the problem is easily solved.

Fixes: QTBUG-96152
Change-Id: I6fc830ce538e8a572093cd8dfe832e10689bf904
Reviewed-by: Andrei Golubev <andrei.golubev@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
(cherry picked from commit b656cea5deccab352b7c4c56d7023f5108578654)

1 files changed, 20 insertions, 13 deletions

diff --git a/src/corelib/tools/qtimezoneprivate.cpp b/src/corelib/tools/qtimezoneprivate.cpp
index 569b343187..6e6a23a9f4 100644
--- a/src/corelib/tools/qtimezoneprivate.cpp
+++ b/src/corelib/tools/qtimezoneprivate.cpp
@@ -1,5 +1,6 @@
 /****************************************************************************
 **
+** Copyright (C) 2021 The Qt Company Ltd.
 ** Copyright (C) 2013 John Layt <jlayt@kde.org>
 ** Contact: https://www.qt.io/licensing/
 **
@@ -355,26 +356,33 @@ QTimeZonePrivate::Data QTimeZonePrivate::dataForLocalTime(qint64 forLocalMSecs,
 
         // Check we do *really* have transitions for this zone:
         if (tran.atMSecsSinceEpoch != invalidMSecs()) {
-
-            /*
-              So now tran is definitely before and nextTran is either after or only
-              slightly before.  One is standard time; we interpret the other as DST
-              (although the transition might in fact by a change in standard offset).  Our
-              hint tells us which of those to use (defaulting to standard if no hint): try
-              it first; if that fails, try the other; if both fail, life's tricky.
-            */
+            /* So now tran is definitely before ... */
             Q_ASSERT(forLocalMSecs < 0
                      || forLocalMSecs - tran.offsetFromUtc * 1000 > tran.atMSecsSinceEpoch);
+            // Work out the UTC value it would make sense to return if using tran:
+            tran.atMSecsSinceEpoch = forLocalMSecs - tran.offsetFromUtc * 1000;
+            // If we know of no transition after it, the answer is easy:
             const qint64 nextStart = nextTran.atMSecsSinceEpoch;
-            // Work out the UTC values it might make sense to return:
+            if (nextStart == invalidMSecs())
+                return tran;
+
+            /*
+              ... and nextTran is either after or only slightly before. We're
+              going to interpret one as standard time, the other as DST
+              (although the transition might in fact be a change in standard
+              offset, or a change in DST offset, e.g. to/from double-DST). Our
+              hint tells us which of those to use (defaulting to standard if no
+              hint): try it first; if that fails, try the other; if both fail,
+              life's tricky.
+            */
+            // Work out the UTC value it would make sense to return if using nextTran:
             nextTran.atMSecsSinceEpoch = forLocalMSecs - nextTran.offsetFromUtc * 1000;
-            tran.atMSecsSinceEpoch = forLocalMSecs - tran.offsetFromUtc * 1000;
 
             // If both or neither have zero DST, treat the one with lower offset as standard:
             const bool nextIsDst = !nextTran.daylightTimeOffset == !tran.daylightTimeOffset
                 ? tran.offsetFromUtc < nextTran.offsetFromUtc : nextTran.daylightTimeOffset;
             // If that agrees with hint > 0, our first guess is to use nextTran; else tran.
-            const bool nextFirst = nextIsDst == (hint > 0) && nextStart != invalidMSecs();
+            const bool nextFirst = nextIsDst == (hint > 0);
             for (int i = 0; i < 2; i++) {
                 /*
                   On the first pass, the case we consider is what hint told us to expect
@@ -386,12 +394,11 @@ QTimeZonePrivate::Data QTimeZonePrivate::dataForLocalTime(qint64 forLocalMSecs,
                   than that.
                 */
                 if (nextFirst ? i == 0 : i) {
-                    Q_ASSERT(nextStart != invalidMSecs());
                     if (Q_LIKELY(nextStart <= nextTran.atMSecsSinceEpoch))
                         return nextTran;
                 } else {
                     // If next is invalid, nextFirst is false, to route us here first:
-                    if (nextStart == invalidMSecs() || Q_LIKELY(nextStart > tran.atMSecsSinceEpoch))
+                    if (Q_LIKELY(nextStart > tran.atMSecsSinceEpoch))
                         return tran;
                 }
             }