From 2a6f2fe9ef9a5d0755443ba94183d97b2fac1a28 Mon Sep 17 00:00:00 2001 From: Edward Welbourne Date: Mon, 7 Sep 2020 14:48:57 +0200 Subject: [PATCH] Check against {und,ov}erflow in more QDateTime methods QDateTime's range of possible values is wider than anyone generally needs, but let's not do confusing things when someone does overflow it. Change-Id: Ifbaf7a0f02cd3afe7d3d13c829bf0887eba29f7f Reviewed-by: Thiago Macieira Reviewed-by: Andrei Golubev --- src/corelib/time/qdatetime.cpp | 50 +++++++++--- .../corelib/time/qdatetime/tst_qdatetime.cpp | 78 ++++++++++++++----- 2 files changed, 99 insertions(+), 29 deletions(-) diff --git a/src/corelib/time/qdatetime.cpp b/src/corelib/time/qdatetime.cpp index 559b3764a1..0be4f290be 100644 --- a/src/corelib/time/qdatetime.cpp +++ b/src/corelib/time/qdatetime.cpp @@ -3959,7 +3959,15 @@ void QDateTime::setMSecsSinceEpoch(qint64 msecs) */ void QDateTime::setSecsSinceEpoch(qint64 secs) { - setMSecsSinceEpoch(secs * 1000); + qint64 msecs; + if (!mul_overflow(secs, std::integral_constant(), &msecs)) { + setMSecsSinceEpoch(msecs); + } else if (d.isShort()) { + d.data.status &= ~QDateTimePrivate::ValidWhenMask; + } else { + d.detach(); + d->m_status &= ~QDateTimePrivate::ValidWhenMask; + } } #if QT_CONFIG(datestring) // depends on, so implies, textdate @@ -4208,7 +4216,10 @@ QDateTime QDateTime::addYears(int nyears) const QDateTime QDateTime::addSecs(qint64 s) const { - return addMSecs(s * 1000); + qint64 msecs; + if (mul_overflow(s, std::integral_constant(), &msecs)) + return QDateTime(); + return addMSecs(msecs); } /*! @@ -4226,15 +4237,31 @@ QDateTime QDateTime::addMSecs(qint64 msecs) const return QDateTime(); QDateTime dt(*this); - auto spec = getSpec(d); - if (spec == Qt::LocalTime || spec == Qt::TimeZone) { - // Convert to real UTC first in case crosses DST transition - dt.setMSecsSinceEpoch(toMSecsSinceEpoch() + msecs); - } else { + switch (getSpec(d)) { + case Qt::LocalTime: + case Qt::TimeZone: + // Convert to real UTC first in case this crosses a DST transition: + if (!add_overflow(toMSecsSinceEpoch(), msecs, &msecs)) { + dt.setMSecsSinceEpoch(msecs); + } else if (dt.d.isShort()) { + dt.d.data.status &= ~QDateTimePrivate::ValidWhenMask; + } else { + dt.d.detach(); + dt.d->m_status &= ~QDateTimePrivate::ValidWhenMask; + } + break; + case Qt::UTC: + case Qt::OffsetFromUTC: // No need to convert, just add on - if (d.isShort()) { + if (add_overflow(getMSecs(d), msecs, &msecs)) { + if (dt.d.isShort()) { + dt.d.data.status &= ~QDateTimePrivate::ValidWhenMask; + } else { + dt.d.detach(); + dt.d->m_status &= ~QDateTimePrivate::ValidWhenMask; + } + } else if (d.isShort()) { // need to check if we need to enlarge first - msecs += dt.d.data.msecs; if (msecsCanBeSmall(msecs)) { dt.d.data.msecs = qintptr(msecs); } else { @@ -4243,8 +4270,9 @@ QDateTime QDateTime::addMSecs(qint64 msecs) const } } else { dt.d.detach(); - dt.d->m_msecs += msecs; + dt.d->m_msecs = msecs; } + break; } return dt; } @@ -4289,7 +4317,7 @@ qint64 QDateTime::daysTo(const QDateTime &other) const qint64 QDateTime::secsTo(const QDateTime &other) const { - return (msecsTo(other) / 1000); + return msecsTo(other) / 1000; } /*! diff --git a/tests/auto/corelib/time/qdatetime/tst_qdatetime.cpp b/tests/auto/corelib/time/qdatetime/tst_qdatetime.cpp index d66ee5c832..9d80583e72 100644 --- a/tests/auto/corelib/time/qdatetime/tst_qdatetime.cpp +++ b/tests/auto/corelib/time/qdatetime/tst_qdatetime.cpp @@ -564,6 +564,19 @@ void tst_QDateTime::setSecsSinceEpoch() QCOMPARE(dt1, QDateTime(QDate(1970, 1, 2), QTime(10, 17, 36), Qt::UTC)); QCOMPARE(dt1.timeSpec(), Qt::OffsetFromUTC); QCOMPARE(dt1.offsetFromUtc(), 60 * 60); + + // Only testing UTC; see fromSecsSinceEpoch() for fuller test. + const qint64 maxSeconds = std::numeric_limits::max() / 1000; + dt1.setSecsSinceEpoch(maxSeconds); + QVERIFY(dt1.isValid()); + dt1.setSecsSinceEpoch(-maxSeconds); + QVERIFY(dt1.isValid()); + dt1.setSecsSinceEpoch(maxSeconds + 1); + QVERIFY(!dt1.isValid()); + dt1.setSecsSinceEpoch(0); + QVERIFY(dt1.isValid()); + dt1.setSecsSinceEpoch(-maxSeconds - 1); + QVERIFY(!dt1.isValid()); } void tst_QDateTime::setMSecsSinceEpoch_data() @@ -754,6 +767,7 @@ void tst_QDateTime::fromMSecsSinceEpoch() void tst_QDateTime::fromSecsSinceEpoch() { + // Compare setSecsSinceEpoch() const qint64 maxSeconds = std::numeric_limits::max() / 1000; const QDateTime early = QDateTime::fromSecsSinceEpoch(-maxSeconds, Qt::UTC); const QDateTime late = QDateTime::fromSecsSinceEpoch(maxSeconds, Qt::UTC); @@ -1204,7 +1218,7 @@ void tst_QDateTime::addYears() QCOMPARE(end.offsetFromUtc(), 60 * 60); } -void tst_QDateTime::addSecs_data() +void tst_QDateTime::addMSecs_data() { QTest::addColumn("dt"); QTest::addColumn("nsecs"); @@ -1300,6 +1314,31 @@ void tst_QDateTime::addSecs_data() QTest::newRow("epoch-1s-local") << QDateTime(QDate(1970, 1, 1), QTime(0, 0)) << qint64(-1) << QDateTime(QDate(1969, 12, 31), QTime(23, 59, 59)); + + // Overflow and Underflow + const qint64 maxSeconds = std::numeric_limits::max() / 1000; + QTest::newRow("after-last") + << QDateTime::fromSecsSinceEpoch(maxSeconds, Qt::UTC) << qint64(1) << QDateTime(); + QTest::newRow("to-last") + << QDateTime::fromSecsSinceEpoch(maxSeconds - 1, Qt::UTC) << qint64(1) + << QDateTime::fromSecsSinceEpoch(maxSeconds, Qt::UTC); + QTest::newRow("before-first") + << QDateTime::fromSecsSinceEpoch(-maxSeconds, Qt::UTC) << qint64(-1) << QDateTime(); + QTest::newRow("to-first") + << QDateTime::fromSecsSinceEpoch(1 - maxSeconds, Qt::UTC) << qint64(-1) + << QDateTime::fromSecsSinceEpoch(-maxSeconds, Qt::UTC); +} + +void tst_QDateTime::addSecs_data() +{ + addMSecs_data(); + + const qint64 maxSeconds = std::numeric_limits::max() / 1000; + // Results would be representable, but the step isn't + QTest::newRow("leap-up") + << QDateTime::fromSecsSinceEpoch(-1, Qt::UTC) << 1 + maxSeconds << QDateTime(); + QTest::newRow("leap-down") + << QDateTime::fromSecsSinceEpoch(1, Qt::UTC) << -1 - maxSeconds << QDateTime(); } void tst_QDateTime::addSecs() @@ -1308,16 +1347,15 @@ void tst_QDateTime::addSecs() QFETCH(const qint64, nsecs); QFETCH(const QDateTime, result); QDateTime test = dt.addSecs(nsecs); - QCOMPARE(test, result); - QCOMPARE(test.timeSpec(), dt.timeSpec()); - if (test.timeSpec() == Qt::OffsetFromUTC) - QCOMPARE(test.offsetFromUtc(), dt.offsetFromUtc()); - QCOMPARE(result.addSecs(-nsecs), dt); -} - -void tst_QDateTime::addMSecs_data() -{ - addSecs_data(); + if (!result.isValid()) { + QVERIFY(!test.isValid()); + } else { + QCOMPARE(test, result); + QCOMPARE(test.timeSpec(), dt.timeSpec()); + if (test.timeSpec() == Qt::OffsetFromUTC) + QCOMPARE(test.offsetFromUtc(), dt.offsetFromUtc()); + QCOMPARE(result.addSecs(-nsecs), dt); + } } void tst_QDateTime::addMSecs() @@ -1327,11 +1365,15 @@ void tst_QDateTime::addMSecs() QFETCH(const QDateTime, result); QDateTime test = dt.addMSecs(qint64(nsecs) * 1000); - QCOMPARE(test, result); - QCOMPARE(test.timeSpec(), dt.timeSpec()); - if (test.timeSpec() == Qt::OffsetFromUTC) - QCOMPARE(test.offsetFromUtc(), dt.offsetFromUtc()); - QCOMPARE(result.addMSecs(qint64(-nsecs) * 1000), dt); + if (!result.isValid()) { + QVERIFY(!test.isValid()); + } else { + QCOMPARE(test, result); + QCOMPARE(test.timeSpec(), dt.timeSpec()); + if (test.timeSpec() == Qt::OffsetFromUTC) + QCOMPARE(test.offsetFromUtc(), dt.offsetFromUtc()); + QCOMPARE(result.addMSecs(qint64(-nsecs) * 1000), dt); + } } void tst_QDateTime::toTimeSpec_data() @@ -1540,7 +1582,7 @@ void tst_QDateTime::secsTo() QFETCH(const qint64, nsecs); QFETCH(const QDateTime, result); - if (dt.isValid()) { + if (result.isValid()) { QCOMPARE(dt.secsTo(result), (qint64)nsecs); QCOMPARE(result.secsTo(dt), (qint64)-nsecs); QVERIFY((dt == result) == (0 == nsecs)); @@ -1566,7 +1608,7 @@ void tst_QDateTime::msecsTo() QFETCH(const qint64, nsecs); QFETCH(const QDateTime, result); - if (dt.isValid()) { + if (result.isValid()) { QCOMPARE(dt.msecsTo(result), qint64(nsecs) * 1000); QCOMPARE(result.msecsTo(dt), -qint64(nsecs) * 1000); QVERIFY((dt == result) == (0 == (qint64(nsecs) * 1000)));