Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-60595

UTC-offset QTimeZones sent through QDataStream become invalid

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: P2: Important
    • Resolution: Done
    • Affects Version/s: 5.2.0, 5.8.0, 5.9.0 Beta 3
    • Fix Version/s: 5.9.1
    • Component/s: Core: Date/Time
    • Labels:
      None
    • Environment:
      Linux and Windows tested
    • Commits:
      147aa291620d9e2533bbea536a748a8f8a7ed14b

      Description

      Creating a QTimeZone("UTC") appears to be a valid action. It's not clear whether "UTC" is a valid IANA ID, but it does appear in the list of availableTimeZoneIds(). tst_QTimeZone::dataStreamTest() also does this.

      Its internal representation, as created by QTimeZone::QTimeZone(const QByteArray &ianaId) is a QUtcTimeZonePrivate which isValid().

      When QTimeZone is serialised into a QDataStream, it calls tz.d->serialize(ds); which is QUtcTimeZonePrivate::serialize. This writes QStringLiteral("OffsetFromUtc") followed by the IANA ID and the offset (etc.) to the datastream.

      When QTimeZone is deserialised it looks for this marker string, and if present, passes all of the parameters to the QTimeZone constructor (not just the name):

      QDataStream &operator>>(QDataStream &ds, QTimeZone &tz)
      {
          QString ianaId;
          ds >> ianaId;
          if (ianaId == QLatin1String("OffsetFromUtc")) {
              int utcOffset;
              QString name;
              QString abbreviation;
              int country;
              QString comment;
              ds >> ianaId >> utcOffset >> name >> abbreviation >> country >> comment;
              tz = QTimeZone(ianaId.toUtf8(), utcOffset, name, abbreviation, (QLocale::Country) country, comment);
          } else {
              tz = QTimeZone(ianaId.toUtf8());
          }
          return ds;
      }
      

      This appears to be designed to support non-standard zones, but instead it fails to support standard ones. The docstring of this constructor states:

      The \a ianaId must not be one of the available system IDs returned by availableTimeZoneIds().

      And the code also checks this, leaving d unset for standard zones, which results in the QTimeZone being invalid:

      QTimeZone::QTimeZone(const QByteArray &ianaId, int offsetSeconds, const QString &name,
                           const QString &abbreviation, QLocale::Country country, const QString &comment)
          : d()
      {
          if (!isTimeZoneIdAvailable(ianaId))
              d = new QUtcTimeZonePrivate(ianaId, offsetSeconds, name, abbreviation, country, comment);
      }
      

      So the valid timezone which was serialised and then deserialised has become invalid. This also affects serialisation of QDateTimes with timezones.

      My guess is that we should check on deserialisation whether the stored timezone is supported by our system, and if so, call the single-argument constructor instead, like this:

      QDataStream &operator>>(QDataStream &ds, QTimeZone &tz)
      {
          QString ianaId;
          ds >> ianaId;
          if (ianaId == QLatin1String("OffsetFromUtc")) {
              int utcOffset;
              QString name;
              QString abbreviation;
              int country;
              QString comment;
              ds >> ianaId >> utcOffset >> name >> abbreviation >> country >> comment;
              tz = QTimeZone(ianaId.toUtf8());
              if(!tz.isValid())
                  tz = QTimeZone(ianaId.toUtf8(), utcOffset, name, abbreviation, (QLocale::Country) country, comment);
          } else {
              tz = QTimeZone(ianaId.toUtf8());
          }
          return ds;
      }
      

        Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

            People

            • Assignee:
              qris0 Chris Wilson
              Reporter:
              qris0 Chris Wilson
            • Votes:
              1 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Gerrit Reviews

                There are no open Gerrit changes