Fix problems with offset-derived ids for QTimeZone

When creating a time-zone from a UTC+offset name that isn't known to
the system, QTimeZone (since the fix to QTBUG-77738 in 5.15.0) falls
back to constructing a suitable UTC-offset backend; however, the id of
this is not guaranteed to match the id passed in to the constructor.
In all other cases, the id of a QTimeZone does match the id passed to
its constructor.

Some utcOffsetId testcases had different id() than the id passed to
the constructor, due to mismatches where a zone was constructed using
the fall-back but the generated id included its minutes (as :00) or
omitted its seconds. The omission of seconds is clearly a bug, but we
also don't want to include :00 for seconds when it's not needed. So
change QTimeZonePrivate::isoOffsetFormat() to accept a
QTimeZone::NameType to configure how much we include in an id. Its
callers other than the relevant constructor (from offset) still get
minutes, even when :00, but will also get seconds added if that isn't
zero; and the constructor from offset now gets the short form obtained
by omitting all trailing zeros.

Since all valid whole-hour offset names that do include :00 for the
minutes field are in fact known standard offset names, the elision of
minutes will only affect zones created by ID in the case of a
whole-hour offset given without :00 minutes specifier, so these shall
necessarily in fact get the ID passed to the constructor. Creating by
UTC-offset with a name that specifies zero seconds will result in a
QTimeZone instance whose id() differs from what was passed to its
constructor (eliding the :00 seconds and potentially also minutes, if
also zero) but this should be the only case where a QTimeZone's id
doesn't match the one passed to the constructor, when constructed by
id.

Fixed inconsistency between the offset-constructor's declaration
(taking offset as int) and definition (taking qint32) in the process.
Added an id check to the utcOffsetId() testcase. Amended two tests of
offset-derived time-zones' IDs, added comments to make clear how one
of those differs from a matching standard name test and converted two
uses of QCOMPARE(, true) to QVERIFY().

[ChangeLog][QtCore][QTimeZone] QTimeZone instances created by offset
from UTC (in seconds) shall now only include minutes in their ID when
the offset is not a whole number of hours. They shall also include the
seconds in their ID when the offset is not a whole number of minutes.

Task-number: QTBUG-87435
Change-Id: I610e0a78e2aca51e12bfe003497434a998e93dc7
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit 50c63446f525a8625b6315597cb0897d89908d6b)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

4 files changed, 32 insertions, 22 deletions

diff --git a/src/corelib/time/qtimezoneprivate.cpp b/src/corelib/time/qtimezoneprivate.cpp
index 873b05295d..832f8f3e59 100644
--- a/src/corelib/time/qtimezoneprivate.cpp
+++ b/src/corelib/time/qtimezoneprivate.cpp
@@ -1,6 +1,6 @@
 /****************************************************************************
 **
-** Copyright (C) 2019 The Qt Company Ltd.
+** Copyright (C) 2021 The Qt Company Ltd.
 ** Copyright (C) 2013 John Layt <jlayt@kde.org>
 ** Contact: https://www.qt.io/licensing/
 **
@@ -663,12 +663,25 @@ bool QTimeZonePrivate::isValidId(const QByteArray &ianaId)
     return true;
 }
 
-QString QTimeZonePrivate::isoOffsetFormat(int offsetFromUtc)
+QString QTimeZonePrivate::isoOffsetFormat(int offsetFromUtc, QTimeZone::NameType mode)
 {
-    const int mins = offsetFromUtc / 60;
-    return QString::fromUtf8("UTC%1%2:%3").arg(mins >= 0 ? QLatin1Char('+') : QLatin1Char('-'))
-                                          .arg(qAbs(mins) / 60, 2, 10, QLatin1Char('0'))
-                                          .arg(qAbs(mins) % 60, 2, 10, QLatin1Char('0'));
+    if (mode == QTimeZone::ShortName && !offsetFromUtc)
+        return utcQString();
+
+    char sign = '+';
+    if (offsetFromUtc < 0) {
+        sign = '-';
+        offsetFromUtc = -offsetFromUtc;
+    }
+    const int secs = offsetFromUtc % 60;
+    const int mins = (offsetFromUtc / 60) % 60;
+    const int hour = offsetFromUtc / 3600;
+    QString result = QString::asprintf("UTC%c%02d", sign, hour);
+    if (mode != QTimeZone::ShortName || secs || mins)
+        result += QString::asprintf(":%02d", mins);
+    if (mode == QTimeZone::LongName || secs)
+        result += QString::asprintf(":%02d", secs);
+    return result;
 }
 
 QByteArray QTimeZonePrivate::ianaIdToWindowsId(const QByteArray &id)
@@ -801,13 +814,7 @@ qint64 QUtcTimeZonePrivate::offsetFromUtcString(const QByteArray &id)
 // Create offset from UTC
 QUtcTimeZonePrivate::QUtcTimeZonePrivate(qint32 offsetSeconds)
 {
-    QString utcId;
-
-    if (offsetSeconds == 0)
-        utcId = utcQString();
-    else
-        utcId = isoOffsetFormat(offsetSeconds);
-
+    QString utcId = isoOffsetFormat(offsetSeconds, QTimeZone::ShortName);
     init(utcId.toUtf8(), offsetSeconds, utcId, utcId, QLocale::AnyCountry, utcId);
 }
 
diff --git a/src/corelib/time/qtimezoneprivate_p.h b/src/corelib/time/qtimezoneprivate_p.h
index 625aba2079..d15d7ccfea 100644
--- a/src/corelib/time/qtimezoneprivate_p.h
+++ b/src/corelib/time/qtimezoneprivate_p.h
@@ -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/
 **
@@ -144,7 +145,8 @@ public:
     static QTimeZone::OffsetData invalidOffsetData();
     static QTimeZone::OffsetData toOffsetData(const Data &data);
     static bool isValidId(const QByteArray &ianaId);
-    static QString isoOffsetFormat(int offsetFromUtc);
+    static QString isoOffsetFormat(int offsetFromUtc,
+                                   QTimeZone::NameType mode = QTimeZone::OffsetName);
 
     static QByteArray ianaIdToWindowsId(const QByteArray &ianaId);
     static QByteArray windowsIdToDefaultIanaId(const QByteArray &windowsId);
@@ -180,7 +182,7 @@ public:
     // Create named time zone
     QUtcTimeZonePrivate(const QByteArray &utcId);
     // Create offset from UTC
-    QUtcTimeZonePrivate(int offsetSeconds);
+    QUtcTimeZonePrivate(qint32 offsetSeconds);
     // Create custom offset from UTC
     QUtcTimeZonePrivate(const QByteArray &zoneId, int offsetSeconds, const QString &name,
                         const QString &abbreviation, QLocale::Country country,
diff --git a/tests/auto/corelib/time/qdatetime/tst_qdatetime.cpp b/tests/auto/corelib/time/qdatetime/tst_qdatetime.cpp
index 7815183d55..146c6b37a5 100644
--- a/tests/auto/corelib/time/qdatetime/tst_qdatetime.cpp
+++ b/tests/auto/corelib/time/qdatetime/tst_qdatetime.cpp
@@ -3628,7 +3628,7 @@ void tst_QDateTime::timeZones() const
     QCOMPARE(nzStdOffset.date(), QDate(2012, 6, 1));
     QCOMPARE(nzStdOffset.time(), QTime(12, 0));
     QVERIFY(nzStdOffset.timeZone() == nzTzOffset);
-    QCOMPARE(nzStdOffset.timeZone().id(), QByteArray("UTC+12:00"));
+    QCOMPARE(nzStdOffset.timeZone().id(), QByteArray("UTC+12"));
     QCOMPARE(nzStdOffset.offsetFromUtc(), 43200);
     QCOMPARE(nzStdOffset.isDaylightTime(), false);
     QCOMPARE(nzStdOffset.toMSecsSinceEpoch(), utcStd.toMSecsSinceEpoch());
diff --git a/tests/auto/corelib/time/qtimezone/tst_qtimezone.cpp b/tests/auto/corelib/time/qtimezone/tst_qtimezone.cpp
index eca258c52e..0103f92434 100644
--- a/tests/auto/corelib/time/qtimezone/tst_qtimezone.cpp
+++ b/tests/auto/corelib/time/qtimezone/tst_qtimezone.cpp
@@ -1,6 +1,6 @@
 /****************************************************************************
 **
-** Copyright (C) 2020 The Qt Company Ltd.
+** Copyright (C) 2021 The Qt Company Ltd.
 ** Contact: https://www.qt.io/licensing/
 **
 ** This file is part of the test suite of the Qt Toolkit.
@@ -499,6 +499,7 @@ void tst_QTimeZone::utcOffsetId()
         QFETCH(int, offset);
         QCOMPARE(zone.offsetFromUtc(epoch), offset);
         QVERIFY(!zone.hasDaylightTime());
+        QCOMPARE(zone.id(), id);
     }
 }
 
@@ -977,11 +978,11 @@ void tst_QTimeZone::utcTest()
     QCOMPARE(tzp.hasDaylightTime(), false);
     QCOMPARE(tzp.hasTransitions(), false);
 
-    // Test create from UTC Offset
+    // Test create from UTC Offset (uses minimal id, skipping minutes if 0)
     QDateTime now = QDateTime::currentDateTime();
     QTimeZone tz(36000);
-    QCOMPARE(tz.isValid(),   true);
-    QCOMPARE(tz.id(), QByteArray("UTC+10:00"));
+    QVERIFY(tz.isValid());
+    QCOMPARE(tz.id(), QByteArray("UTC+10"));
     QCOMPARE(tz.offsetFromUtc(now), 36000);
     QCOMPARE(tz.standardTimeOffset(now), 36000);
     QCOMPARE(tz.daylightTimeOffset(now), 0);
@@ -996,9 +997,9 @@ void tst_QTimeZone::utcTest()
     QCOMPARE(QTimeZone(max).isValid(), true);
     QCOMPARE(QTimeZone(max + 1).isValid(), false);
 
-    // Test create from standard name
+    // Test create from standard name (preserves :00 for minutes in id):
     tz = QTimeZone("UTC+10:00");
-    QCOMPARE(tz.isValid(),   true);
+    QVERIFY(tz.isValid());
     QCOMPARE(tz.id(), QByteArray("UTC+10:00"));
     QCOMPARE(tz.offsetFromUtc(now), 36000);
     QCOMPARE(tz.standardTimeOffset(now), 36000);