From d6bca7b1982338927fa9e20c8e9cb08dcdb2fcbc Mon Sep 17 00:00:00 2001 From: Edward Welbourne Date: Tue, 12 Jan 2021 19:39:04 +0100 Subject: 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 (cherry picked from commit 50c63446f525a8625b6315597cb0897d89908d6b) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/time/qtimezoneprivate.cpp | 33 +++++++++++++--------- src/corelib/time/qtimezoneprivate_p.h | 6 ++-- .../auto/corelib/time/qdatetime/tst_qdatetime.cpp | 2 +- .../auto/corelib/time/qtimezone/tst_qtimezone.cpp | 13 +++++---- 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 ** 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 ** 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); -- cgit v1.2.3