Skip to content

Commit

Permalink
Fix crash when a date-time has an invalid time-zone
Browse files Browse the repository at this point in the history 
QDateTime is a friend of QTimeZone, so can access its internals; but
it must check the zone is valid before doing so.

Expanded tst_QDateTime::invalid() and made it data-driven to catch the
failure cases.

Commented on a test-case that caught a mistake in my first attempt at
this, and on QDateTimeParser's surprising reliance on a quirk of
QDateTime::toMSecsSinceEpoch()'s behavior.

Fixes: QTBUG-80146
Change-Id: I24856e19ff9bf402152d17d71f83be84e366faad
Reviewed-by: Alex Blasche <[email protected]>
  • Loading branch information
@ediosyncratic
ediosyncratic committed Dec 6, 2019
1 parent da94625 commit 49063c3
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 25 deletions.
32 changes: 22 additions & 10 deletions src/corelib/time/qdatetime.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3410,6 +3410,7 @@ inline qint64 QDateTimePrivate::zoneMSecsToEpochMSecs(qint64 zoneMSecs, const QT
DaylightStatus hint,
QDate *zoneDate, QTime *zoneTime)
{
Q_ASSERT(zone.isValid());
// Get the effective data from QTimeZone
QTimeZonePrivate::Data data = zone.d->dataForLocalTime(zoneMSecs, int(hint));
// Docs state any time before 1970-01-01 will *not* have any DST applied
Expand Down Expand Up @@ -3799,8 +3800,9 @@ QTimeZone QDateTime::timeZone() const
case Qt::OffsetFromUTC:
return QTimeZone(d->m_offsetFromUtc);
case Qt::TimeZone:
Q_ASSERT(d->m_timeZone.isValid());
return d->m_timeZone;
if (d->m_timeZone.isValid())
return d->m_timeZone;
break;
case Qt::LocalTime:
return QTimeZone::systemTimeZone();
}
Expand Down Expand Up @@ -3884,6 +3886,7 @@ QString QDateTime::timeZoneAbbreviation() const
#if !QT_CONFIG(timezone)
break;
#else
Q_ASSERT(d->m_timeZone.isValid());
return d->m_timeZone.d->abbreviation(toMSecsSinceEpoch());
#endif // timezone
case Qt::LocalTime: {
Expand Down Expand Up @@ -3920,6 +3923,7 @@ bool QDateTime::isDaylightTime() const
#if !QT_CONFIG(timezone)
break;
#else
Q_ASSERT(d->m_timeZone.isValid());
return d->m_timeZone.d->isDaylightTime(toMSecsSinceEpoch());
#endif // timezone
case Qt::LocalTime: {
Expand Down Expand Up @@ -4044,6 +4048,10 @@ void QDateTime::setTimeZone(const QTimeZone &toZone)
*/
qint64 QDateTime::toMSecsSinceEpoch() const
{
// Note: QDateTimeParser relies on this producing a useful result, even when
// !isValid(), at least when the invalidity is a time in a fall-back (that
// we'll have adjusted to lie outside it, but marked invalid because it's
// not what was asked for). Other things may be doing similar.
switch (getSpec(d)) {
case Qt::UTC:
return getMSecs(d);
Expand All @@ -4058,12 +4066,13 @@ qint64 QDateTime::toMSecsSinceEpoch() const
}

case Qt::TimeZone:
#if !QT_CONFIG(timezone)
return 0;
#else
return QDateTimePrivate::zoneMSecsToEpochMSecs(d->m_msecs, d->m_timeZone,
extractDaylightStatus(getStatus(d)));
#if QT_CONFIG(timezone)
if (d->m_timeZone.isValid()) {
return QDateTimePrivate::zoneMSecsToEpochMSecs(d->m_msecs, d->m_timeZone,
extractDaylightStatus(getStatus(d)));
}
#endif
return 0;
}
Q_UNREACHABLE();
return 0;
Expand Down Expand Up @@ -4158,9 +4167,11 @@ void QDateTime::setMSecsSinceEpoch(qint64 msecs)
case Qt::TimeZone:
Q_ASSERT(!d.isShort());
#if QT_CONFIG(timezone)
d.detach();
if (!d->m_timeZone.isValid())
break;
// Docs state any LocalTime before 1970-01-01 will *not* have any DST applied
// but all affected times afterwards will have DST applied.
d.detach();
if (msecs >= 0) {
status = mergeDaylightStatus(status,
d->m_timeZone.d->isDaylightTime(msecs)
Expand Down Expand Up @@ -4433,7 +4444,7 @@ static inline void massageAdjustedDateTime(const QDateTimeData &d, QDate *date,
QDateTimePrivate::DaylightStatus status = QDateTimePrivate::UnknownDaylightTime;
localMSecsToEpochMSecs(timeToMSecs(*date, *time), &status, date, time);
#if QT_CONFIG(timezone)
} else if (spec == Qt::TimeZone) {
} else if (spec == Qt::TimeZone && d->m_timeZone.isValid()) {
QDateTimePrivate::zoneMSecsToEpochMSecs(timeToMSecs(*date, *time),
d->m_timeZone,
QDateTimePrivate::UnknownDaylightTime,
Expand Down Expand Up @@ -5094,7 +5105,8 @@ QDateTime QDateTime::fromMSecsSinceEpoch(qint64 msecs, const QTimeZone &timeZone
{
QDateTime dt;
dt.setTimeZone(timeZone);
dt.setMSecsSinceEpoch(msecs);
if (timeZone.isValid())
dt.setMSecsSinceEpoch(msecs);
return dt;
}

Expand Down
2 changes: 1 addition & 1 deletion src/corelib/time/qdatetimeparser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1363,7 +1363,7 @@ QDateTimeParser::scanString(const QDateTime &defaultValue,
// given date (which might be a spring-forward, skipping an hour).
if (parserType == QVariant::DateTime && !(isSet & HourSectionMask) && !when.isValid()) {
qint64 msecs = when.toMSecsSinceEpoch();
// Fortunately, that gets a useful answer ...
// Fortunately, that gets a useful answer, even though when is invalid ...
const QDateTime replace =
#if QT_CONFIG(timezone)
tspec == Qt::TimeZone
Expand Down
52 changes: 38 additions & 14 deletions tests/auto/corelib/time/qdatetime/tst_qdatetime.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ private slots:
void timeZones() const;
void systemTimeZoneChange() const;

void invalid_data() const;
void invalid() const;
void range() const;

Expand Down Expand Up @@ -2458,6 +2459,7 @@ void tst_QDateTime::fromStringStringFormat_data()
if (southBrazil.isValid()) {
QTest::newRow("spring-forward-midnight")
<< QString("2008-10-19 23:45.678 America/Sao_Paulo") << QString("yyyy-MM-dd mm:ss.zzz t")
// That's in the hour skipped - expect the matching time after the spring-forward, in DST:
<< QDateTime(QDate(2008, 10, 19), QTime(1, 23, 45, 678), southBrazil);
}
#endif
Expand Down Expand Up @@ -3359,6 +3361,9 @@ void tst_QDateTime::timeZones() const
QCOMPARE(utc.date(), utcDst.date());
QCOMPARE(utc.time(), utcDst.time());

// Crash test, QTBUG-80146:
QVERIFY(!nzStd.toTimeZone(QTimeZone()).isValid());

// Sydney is 2 hours behind New Zealand
QTimeZone ausTz = QTimeZone("Australia/Sydney");
QDateTime aus = nzStd.toTimeZone(ausTz);
Expand Down Expand Up @@ -3528,23 +3533,42 @@ void tst_QDateTime::systemTimeZoneChange() const
QCOMPARE(tzDate.toMSecsSinceEpoch(), tzMsecs);
}

void tst_QDateTime::invalid() const
void tst_QDateTime::invalid_data() const
{
QDateTime invalidDate = QDateTime(QDate(0, 0, 0), QTime(-1, -1, -1));
QCOMPARE(invalidDate.isValid(), false);
QCOMPARE(invalidDate.timeSpec(), Qt::LocalTime);

QDateTime utcDate = invalidDate.toUTC();
QCOMPARE(utcDate.isValid(), false);
QCOMPARE(utcDate.timeSpec(), Qt::UTC);
QTest::addColumn<QDateTime>("when");
QTest::addColumn<Qt::TimeSpec>("spec");
QTest::addColumn<bool>("goodZone");
QTest::newRow("default") << QDateTime() << Qt::LocalTime << true;

QDateTime offsetDate = invalidDate.toOffsetFromUtc(3600);
QCOMPARE(offsetDate.isValid(), false);
QCOMPARE(offsetDate.timeSpec(), Qt::OffsetFromUTC);
QDateTime invalidDate = QDateTime(QDate(0, 0, 0), QTime(-1, -1, -1));
QTest::newRow("simple") << invalidDate << Qt::LocalTime << true;
QTest::newRow("UTC") << invalidDate.toUTC() << Qt::UTC << true;
QTest::newRow("offset")
<< invalidDate.toOffsetFromUtc(3600) << Qt::OffsetFromUTC << true;
QTest::newRow("CET")
<< invalidDate.toTimeZone(QTimeZone("Europe/Oslo")) << Qt::TimeZone << true;

// Crash tests, QTBUG-80146:
QTest::newRow("nozone+construct")
<< QDateTime(QDate(1970, 1, 1), QTime(12, 0), QTimeZone()) << Qt::TimeZone << false;
QTest::newRow("nozone+fromMSecs")
<< QDateTime::fromMSecsSinceEpoch(42, QTimeZone()) << Qt::TimeZone << false;
QDateTime valid(QDate(1970, 1, 1), QTime(12, 0), Qt::UTC);
QTest::newRow("tonozone") << valid.toTimeZone(QTimeZone()) << Qt::TimeZone << false;
}

QDateTime tzDate = invalidDate.toTimeZone(QTimeZone("Europe/Oslo"));
QCOMPARE(tzDate.isValid(), false);
QCOMPARE(tzDate.timeSpec(), Qt::TimeZone);
void tst_QDateTime::invalid() const
{
QFETCH(QDateTime, when);
QFETCH(Qt::TimeSpec, spec);
QFETCH(bool, goodZone);
QVERIFY(!when.isValid());
QCOMPARE(when.timeSpec(), spec);
QCOMPARE(when.timeZoneAbbreviation(), QString());
if (!goodZone)
QCOMPARE(when.toMSecsSinceEpoch(), 0);
QVERIFY(!when.isDaylightTime());
QCOMPARE(when.timeZone().isValid(), goodZone);
}

void tst_QDateTime::range() const
Expand Down

0 comments on commit 49063c3

Please sign in to comment.