QIcuTimeZonePrivate constructor: save iteration over all zone IDs

ICU returns a "valid" representation of GMT when given an unrecognised
ID, so QTZ's constructor has been checking the ID is available before
passing it to the backend constructor. That availability check was
done by generating the list of available IDs to see if the given ID
was in it; this is very inefficient. Furthermore, the QTZ constructor
was also checking availability, to work round the same issue in only
this one backend, making the check redundant.

So overide isTimeZoneIdAvailable() in the ICU backend, calling
ucal_getCanonicalTimeZoneID(), which answers the question directly;
and drop the duplicate check in the QTZ constructor. Expand a test to
verify an invalid name is rejected.

Fixes: QTBUG-121807
Pick-to: 6.7 6.6 6.5
Change-Id: I34f996b607b958d12607a94eb273bb1b406cca1a
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

3 files changed, 26 insertions, 14 deletions

diff --git a/src/corelib/time/qtimezone.cpp b/src/corelib/time/qtimezone.cpp
index 5f5d104baf..ea6ca942bb 100644
--- a/src/corelib/time/qtimezone.cpp
+++ b/src/corelib/time/qtimezone.cpp
@@ -448,23 +448,15 @@ QTimeZone::Data &QTimeZone::Data::operator=(QTimeZonePrivate *dptr) noexcept
 
 QTimeZone::QTimeZone(const QByteArray &ianaId)
 {
-    // Try and see if it's a CLDR UTC offset ID - just as quick by creating as
-    // by looking up.
+    // Try and see if it's a recognized UTC offset ID - just as quick by
+    // creating as by looking up.
     d = new QUtcTimeZonePrivate(ianaId);
-    // If not a CLDR UTC offset ID then try creating it with the system backend.
-    // Relies on backend not creating valid TZ with invalid name.
+    // If not recognized, try creating it with the system backend.
     if (!d->isValid()) {
         if (ianaId.isEmpty())
             d = newBackendTimeZone();
-#ifdef Q_OS_ANDROID
-        // on Android the isTimeZoneIdAvailable() implementation is vastly more
-        // expensive than just trying to create a timezone
-        else
+        else // Constructor MUST produce invalid for unsupported ID.
             d = newBackendTimeZone(ianaId);
-#else
-        else if (global_tz->backend->isTimeZoneIdAvailable(ianaId))
-            d = newBackendTimeZone(ianaId);
-#endif
         // else: No such ID, avoid creating a TZ cache entry for it.
     }
     // Can also handle UTC with arbitrary (valid) offset, but only do so as
diff --git a/src/corelib/time/qtimezoneprivate_icu.cpp b/src/corelib/time/qtimezoneprivate_icu.cpp
index d2e2ba98ac..fb8c620f32 100644
--- a/src/corelib/time/qtimezoneprivate_icu.cpp
+++ b/src/corelib/time/qtimezoneprivate_icu.cpp
@@ -254,8 +254,8 @@ QIcuTimeZonePrivate::QIcuTimeZonePrivate()
 QIcuTimeZonePrivate::QIcuTimeZonePrivate(const QByteArray &ianaId)
     : m_ucal(nullptr)
 {
-    // Need to check validity here as ICu will create a GMT tz if name is invalid
-    if (availableTimeZoneIds().contains(ianaId))
+    // ICU misleadingly maps invalid IDs to GMT.
+    if (isTimeZoneIdAvailable(ianaId))
         init(ianaId);
 }
 
@@ -433,6 +433,25 @@ QByteArray QIcuTimeZonePrivate::systemTimeZoneId() const
     return ucalDefaultTimeZoneId();
 }
 
+bool QIcuTimeZonePrivate::isTimeZoneIdAvailable(const QByteArray &ianaId) const
+{
+    const QString ianaStr = QString::fromUtf8(ianaId);
+    const UChar *const name = reinterpret_cast<const UChar *>(ianaStr.constData());
+    // We are not interested in the value, but we have to pass something.
+    // No known IANA zone name is (up to 2023) longer than 30 characters.
+    constexpr size_t size = 64;
+    UChar buffer[size];
+
+    // TODO: convert to ucal_getIanaTimeZoneID(), new draft in ICU 74, once we
+    // can rely on its availability, assuming it works the same once not draft.
+    UErrorCode status = U_ZERO_ERROR;
+    UBool isSys = false;
+    // Returns the length of the IANA zone name (but we don't care):
+    ucal_getCanonicalTimeZoneID(name, ianaStr.size(), buffer, size, &isSys, &status);
+    // We're only interested if the result is a "system" (i.e. IANA) ID:
+    return isSys;
+}
+
 QList<QByteArray> QIcuTimeZonePrivate::availableTimeZoneIds() const
 {
     UErrorCode status = U_ZERO_ERROR;
diff --git a/src/corelib/time/qtimezoneprivate_p.h b/src/corelib/time/qtimezoneprivate_p.h
index 555114f140..bef9c11d43 100644
--- a/src/corelib/time/qtimezoneprivate_p.h
+++ b/src/corelib/time/qtimezoneprivate_p.h
@@ -232,6 +232,7 @@ public:
 
     QByteArray systemTimeZoneId() const override;
 
+    bool isTimeZoneIdAvailable(const QByteArray &ianaId) const override;
     QList<QByteArray> availableTimeZoneIds() const override;
     QList<QByteArray> availableTimeZoneIds(QLocale::Territory territory) const override;
     QList<QByteArray> availableTimeZoneIds(int offsetFromUtc) const override;