From fce5fdb4d1c0e1d2db28d7a8dfb03e916dfc232f Mon Sep 17 00:00:00 2001 From: Ahmad Samir Date: Wed, 5 Feb 2025 22:48:09 +0200 Subject: [PATCH] QTimer: clamp too-large milliseconds intervals to INT_MAX ms QTimer stores the interval in an int, a milliseconds interval that is too-large will overflow and the value becomes negative. Clamp the interval to INT_MAX ms. This also matches what QTimer::from_msecs() does. Amends f1f610bc67bfd5c2ef31270a6945e7bae93b5e4a. Change-Id: Id925827f632e8a8d8b8b65e6a41e8f8722344c26 Reviewed-by: Thiago Macieira --- src/corelib/kernel/qtimer.cpp | 11 +++-- .../auto/corelib/kernel/qtimer/tst_qtimer.cpp | 42 +++++++++++++++++++ 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/src/corelib/kernel/qtimer.cpp b/src/corelib/kernel/qtimer.cpp index 796a82afe0b..74d38faa214 100644 --- a/src/corelib/kernel/qtimer.cpp +++ b/src/corelib/kernel/qtimer.cpp @@ -246,9 +246,14 @@ void QTimer::start(int msec) static std::chrono::milliseconds checkInterval(const char *caller, std::chrono::milliseconds interval) { + constexpr auto maxInterval = INT_MAX * 1ms; if (interval < 0ms) { qWarning("%s: negative intervals aren't allowed; the interval will be set to 1ms.", caller); interval = 1ms; + } else if (interval > maxInterval) { + qWarning("%s: interval exceeds maximum allowed interval, it will be clamped to " + "INT_MAX ms (about 24 days).", caller); + interval = maxInterval; } return interval; } @@ -276,9 +281,6 @@ void QTimer::start(std::chrono::milliseconds interval) Q_D(QTimer); interval = checkInterval("QTimer::start", interval); - - // This could be narrowing as the interval is stored in an `int` QProperty, - // and the type can't be changed in Qt6. const int msec = interval.count(); const bool intervalChanged = msec != d->inter; d->inter.setValue(msec); @@ -645,9 +647,6 @@ void QTimer::setInterval(std::chrono::milliseconds interval) Q_D(QTimer); interval = checkInterval("QTimer::setInterval", interval); - - // This could be narrowing as the interval is stored in an `int` QProperty, - // and the type can't be changed in Qt6. const int msec = interval.count(); d->inter.removeBindingUnlessInWrapper(); const bool intervalChanged = msec != d->inter.valueBypassingBindings(); diff --git a/tests/auto/corelib/kernel/qtimer/tst_qtimer.cpp b/tests/auto/corelib/kernel/qtimer/tst_qtimer.cpp index aff1ae07008..4d9aae81f38 100644 --- a/tests/auto/corelib/kernel/qtimer/tst_qtimer.cpp +++ b/tests/auto/corelib/kernel/qtimer/tst_qtimer.cpp @@ -99,6 +99,8 @@ private slots: void negativeInterval(); void testTimerId(); + + void intervalOverflow(); }; void tst_QTimer::zeroTimer() @@ -1434,6 +1436,46 @@ void tst_QTimer::negativeInterval() QCOMPARE(timer.intervalAsDuration(), 1ms); } +void tst_QTimer::intervalOverflow() +{ + QTimer timer; + constexpr auto maxInterval = INT_MAX * 1ms; + constexpr auto tooBig = maxInterval + 1ms; + auto ignoreStartWarningMsg = [] { + QTest::ignoreMessage(QtWarningMsg, + "QTimer::start: interval exceeds maximum allowed interval, it will " + "be clamped to INT_MAX ms (about 24 days)."); + }; + auto ignoreSetIntervalWarningMsg = [] { + QTest::ignoreMessage(QtWarningMsg, + "QTimer::setInterval: interval exceeds maximum allowed interval, " + "it will be clamped to INT_MAX ms (about 24 days)."); + }; + + ignoreStartWarningMsg(); + timer.start(tooBig); + QVERIFY(timer.isActive()); + // Interval clamped to INT_MAX * 1ms + QCOMPARE(timer.intervalAsDuration(), maxInterval); + + timer.stop(); + ignoreSetIntervalWarningMsg(); + timer.setInterval(tooBig); // The same but with setInterval() + QCOMPARE(timer.intervalAsDuration(), maxInterval); + timer.start(); + QVERIFY(timer.isActive()); + + timer.stop(); + ignoreStartWarningMsg(); + timer.start(tooBig + 10min); + QVERIFY(timer.isActive()); + QCOMPARE(timer.intervalAsDuration(), maxInterval); + ignoreSetIntervalWarningMsg(); + timer.setInterval(tooBig + 1h); // With an already running timer + QVERIFY(timer.isActive()); + QCOMPARE(timer.intervalAsDuration(), maxInterval); +} + class OrderHelper : public QObject { Q_OBJECT