mirror of
https://github.com/status-im/QtModelsToolkit.git
synced 2026-08-31 01:51:08 +00:00
fix(ModelQuery): pin CppOwnership on QObject role values returned to JS
A Q_INVOKABLE returning a QVariant holding a QObject* makes the object destructible by the JS GC (qv4qobjectwrapper transfers ownership unless explicitly set). ModelQuery::get(model, row, roleName) returns nested submodels (e.g. QQmlListModel's cached nested models) this way; the GC could delete them while the source model still owns them, causing a double-free/UAF in ListModel::destroy() on model teardown.
This commit is contained in:
@@ -1,6 +1,11 @@
|
||||
#include <QTest>
|
||||
|
||||
#include <QCoreApplication>
|
||||
#include <QPointer>
|
||||
#include <QQmlContext>
|
||||
#include <QQmlEngine>
|
||||
#include <QQmlExpression>
|
||||
#include <QRegularExpression>
|
||||
|
||||
#include <qtmodelstoolkit/modelquery.h>
|
||||
#include <qtmodelstoolkit/testing/listmodelwrapper.h>
|
||||
@@ -46,6 +51,63 @@ private slots:
|
||||
QCOMPARE(utils.indexOf(model, "balance", 4), 3);
|
||||
QCOMPARE(utils.indexOf(model, "balance", "4"), 3);
|
||||
}
|
||||
|
||||
void testGetDoesNotHandSubmodelToJsGc()
|
||||
{
|
||||
QQmlEngine engine;
|
||||
|
||||
ListModelWrapper model(engine, R"([
|
||||
{ name: "A", balances: [ { chainId: 1 }, { chainId: 2 } ] }
|
||||
])");
|
||||
|
||||
ModelQuery utils;
|
||||
|
||||
// The submodel wrapper is cached, so the direct call returns the same
|
||||
// object the JS call below returns.
|
||||
QPointer<QObject> subModel = utils.get(model, 0, "balances").value<QObject *>();
|
||||
QVERIFY(subModel);
|
||||
|
||||
// Fetch through the JS engine: a QVariant(QObject*) returned from a
|
||||
// Q_INVOKABLE is handed to the JS GC unless ownership is pinned.
|
||||
engine.rootContext()->setContextProperty("modelQuery", &utils);
|
||||
engine.rootContext()->setContextProperty("sourceModel", model.model());
|
||||
QQmlExpression expression(engine.rootContext(), nullptr,
|
||||
QStringLiteral("modelQuery.get(sourceModel, 0, 'balances')"));
|
||||
QVERIFY(!expression.evaluate().isNull());
|
||||
|
||||
QCOMPARE(QQmlEngine::objectOwnership(subModel), QQmlEngine::CppOwnership);
|
||||
|
||||
// The source model owns the submodel; a GC pass must not delete it.
|
||||
engine.collectGarbage();
|
||||
QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete);
|
||||
QVERIFY(subModel);
|
||||
}
|
||||
|
||||
void testGetRespectsExplicitJsOwnershipAndWarns()
|
||||
{
|
||||
QQmlEngine engine;
|
||||
|
||||
ListModelWrapper model(engine, R"([
|
||||
{ name: "A", balances: [ { chainId: 1 } ] }
|
||||
])");
|
||||
|
||||
ModelQuery utils;
|
||||
|
||||
QObject *subModel = utils.get(model, 0, "balances").value<QObject *>();
|
||||
QVERIFY(subModel);
|
||||
QQmlEngine::setObjectOwnership(subModel, QQmlEngine::JavaScriptOwnership);
|
||||
|
||||
engine.rootContext()->setContextProperty("modelQuery", &utils);
|
||||
engine.rootContext()->setContextProperty("sourceModel", model.model());
|
||||
QQmlExpression expression(engine.rootContext(), nullptr,
|
||||
QStringLiteral("modelQuery.get(sourceModel, 0, 'balances')"));
|
||||
|
||||
QTest::ignoreMessage(QtWarningMsg,
|
||||
QRegularExpression(QStringLiteral("ModelQuery::get:.*JS-owned")));
|
||||
QVERIFY(!expression.evaluate().isNull());
|
||||
|
||||
QCOMPARE(QQmlEngine::objectOwnership(subModel), QQmlEngine::JavaScriptOwnership);
|
||||
}
|
||||
};
|
||||
|
||||
QTEST_MAIN(TestModelQuery)
|
||||
|
||||
Reference in New Issue
Block a user