qiurui
/
qtdeclarative
mirror of https://github.com/qt/qtdeclarative.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

QML: Type-check objects passed to QmlListWrapper 

So far we could assign QObjects of any type into any QmlListWrapper of
another type. This is UB, since you may be holding the underlying
QQmlListProperty in C++, and it's also not very consistent.

Assign null instead if the type doesn't match and emit a warning.
Likewise, also warn when initializing a list with mismatched elements.

[ChangeLog][QtQml][Important Behavior Changes] Assignments to list
properties in QML are now type-checked. Before you could, for example,
insert a plain QObject into list<Item>, producing undefined behavior.
Now it instead inserts null and warns. In particular, if you assign a
JavaScript array of random objects to a list property, QML will check
each individual element, and insert null as well as warn when
encountering type incompatibilities.

Pick-to: 6.8 6.7 6.5
Fixes: QTBUG-127343
Change-Id: I9b78afcd3ed40c80175a99861373588f56683954
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
This commit is contained in:
Ulf Hermann 2024-08-14 11:14:08 +02:00
parent d9b13de4e8
commit aeabdb9388
7 changed files with 141 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

82
src/qml/qml/qqmllistwrapper.cpp
View File

@ -17,6 +17,8 @@
QT_BEGIN_NAMESPACE
Q_LOGGING_CATEGORY(lcIncompatibleElement, "qt.qml.list.incompatible")
using namespace QV4;
using namespace Qt::StringLiterals;
@ -229,7 +231,22 @@ bool QmlListWrapper::virtualPut(Managed *m, PropertyKey id, const Value &value,
QV4::Scope scope(v4);
QV4::ScopedObject so(scope, value.toObject(scope.engine));
if (auto *wrapper = so->as<QV4::QObjectWrapper>()) {
prop->replace(prop, index, wrapper->object());
QObject *object = wrapper->object();
if (!object) {
prop->replace(prop, index, object);
return true;
}
const QMetaType elementType = w->d()->elementType();
const QMetaObject *elementMeta = elementType.metaObject();
if (Q_UNLIKELY(!elementMeta || !QQmlMetaObject::canConvert(object, elementMeta))) {
qCWarning(lcIncompatibleElement)
<< "Cannot insert" << object << "into a QML list of" << elementType.name();
prop->replace(prop, index, nullptr);
return true;
}
prop->replace(prop, index, object);
return true;
}
@ -346,11 +363,28 @@ ReturnedValue PropertyListPrototype::method_push(const FunctionObject *b, const
if (!qIsAtMostUintLimit(length, std::numeric_limits<uint>::max() - argc))
return scope.engine->throwRangeError(QString::fromLatin1("List length out of range."));
const QMetaType elementType = w->d()->elementType();
const QMetaObject *elementMeta = elementType.metaObject();
for (int i = 0; i < argc; ++i) {
if (argv[i].isNull())
if (argv[i].isNull()) {
property->append(property, nullptr);
else
property->append(property, argv[i].as<QV4::QObjectWrapper>()->object());
continue;
}
QObject *object = argv[i].as<QV4::QObjectWrapper>()->object();
if (!object) {
property->append(property, nullptr);
continue;
}
if (Q_UNLIKELY(!elementMeta || !QQmlMetaObject::canConvert(object, elementMeta))) {
qCWarning(lcIncompatibleElement)
<< "Cannot append" << object << "to a QML list of" << elementType.name();
property->append(property, nullptr);
continue;
}
property->append(property, object);
}
const auto actualLength = property->count(property);
@ -474,12 +508,29 @@ ReturnedValue PropertyListPrototype::method_splice(const FunctionObject *b, cons
}
}
const QMetaType elementType = w->d()->elementType();
const QMetaObject *elementMeta = elementType.metaObject();
for (qsizetype i = 0; i < itemCount; ++i) {
const auto arg = argv[i + 2];
if (arg.isNull())
if (arg.isNull()) {
property->replace(property, start + i, nullptr);
else
property->replace(property, start + i, arg.as<QObjectWrapper>()->object());
continue;
}
QObject *object = arg.as<QObjectWrapper>()->object();
if (!object) {
property->replace(property, start + i, nullptr);
continue;
}
if (Q_UNLIKELY(!elementMeta || !QQmlMetaObject::canConvert(object, elementMeta))) {
qCWarning(lcIncompatibleElement)
<< "Cannot splice" << object << "into a QML list of" << elementType.name();
property->replace(property, start + i, nullptr);
continue;
}
property->replace(property, start + i, object);
}
return newArray->asReturnedValue();
@ -524,9 +575,24 @@ ReturnedValue PropertyListPrototype::method_unshift(const FunctionObject *b, con
for (qsizetype k = len; k > 0; --k)
property->replace(property, k + argc - 1, property->at(property, k - 1));
const QMetaType elementType = w->d()->elementType();
const QMetaObject *elementMeta = elementType.metaObject();
for (int i = 0; i < argc; ++i) {
const auto *wrapper = argv[i].as<QObjectWrapper>();
property->replace(property, i, wrapper ? wrapper->object() : nullptr);
QObject *object = wrapper ? wrapper->object() : nullptr;
if (!object) {
property->replace(property, i, object);
continue;
}
if (Q_UNLIKELY(!elementMeta || !QQmlMetaObject::canConvert(object, elementMeta))) {
qCWarning(lcIncompatibleElement)
<< "Cannot unshift" << object << "into a QML list of" << elementType.name();
property->replace(property, i, nullptr);
continue;
}
property->replace(property, i, object);
}
return Encode(uint(len + argc));

3
src/qml/qml/qqmllistwrapper_p.h
View File

@ -25,6 +25,8 @@
QT_BEGIN_NAMESPACE
Q_DECLARE_LOGGING_CATEGORY(lcIncompatibleElement)
namespace QV4 {
namespace Heap {
@ -38,6 +40,7 @@ struct QmlListWrapper : Object
QObject *object() const { return m_object.data(); }
QMetaType propertyType() const { return QMetaType(m_propertyType); }
QMetaType elementType() const { return QQmlMetaType::listValueType(propertyType()); }
const QQmlListProperty<QObject> *property() const
{

6
src/qml/qml/qqmlproperty.cpp
View File

@ -12,6 +12,7 @@
#include <private/qqmlengine_p.h>
#include <private/qqmlirbuilder_p.h>
#include <private/qqmllist_p.h>
#include <private/qqmllistwrapper_p.h>
#include <private/qqmlproperty_p.h>
#include <private/qqmlsignalnames_p.h>
#include <private/qqmlstringconverters_p.h>
@ -1589,8 +1590,11 @@ bool QQmlPropertyPrivate::write(
propClear(&prop);
const auto doAppend = [&](QObject *o) {
if (o && !QQmlMetaObject::canConvert(o, valueMetaObject))
if (Q_UNLIKELY(o && !QQmlMetaObject::canConvert(o, valueMetaObject))) {
qCWarning(lcIncompatibleElement)
<< "Cannot append" << o << "to a QML list of" << listValueType.name();
o = nullptr;
}
propAppend(&prop, o);
};

23
src/quicktemplates/qquickbuttongroup.cpp
View File

@ -84,6 +84,29 @@ QT_BEGIN_NAMESPACE
}
\endcode
Another option is to filter the list of children. This is especially handy
if you're using a repeater to populate it, since the repeater will also be
a child of the parent layout:
\code
ButtonGroup {
buttons: column.children.filter((child) => child !== repeater)
}
Column {
id: column
Repeater {
id: repeater
model: [ qsTr("DAB"), qsTr("AM"), qsTr("FM") ]
RadioButton {
required property string modelData
text: modelData
}
}
}
\endcode
More advanced use cases can be handled using the \c addButton() and
\c removeButton() methods.

11
tests/auto/qml/qqmllistreference/data/compositeListProp.qml
View File

@ -1,5 +1,14 @@
import QtQml
QtObject {
property list<AListItem> items
id: self
property list<AListItem> items: [ self ]
Component.onCompleted: {
items.push(self);
items.push(null);
items[2] = self;
items.splice(1, 0, self);
items.unshift(self);
}
}

23
tests/auto/qml/qqmllistreference/tst_qqmllistreference.cpp
View File

@ -863,9 +863,31 @@ void tst_qqmllistreference::compositeListProperty()
{
QQmlEngine engine;
QQmlComponent component(&engine, testFileUrl("compositeListProp.qml"));
QTest::ignoreMessage(
QtWarningMsg, QRegularExpression("Cannot append QObject_QML_[0-9]+\\(0x[0-9a-f]+\\) "
"to a QML list of AListItem_QMLTYPE_[0-9]+\\*"));
QTest::ignoreMessage(
QtWarningMsg, QRegularExpression("Cannot append QObject_QML_[0-9]+\\(0x[0-9a-f]+\\) "
"to a QML list of AListItem_QMLTYPE_[0-9]+\\*"));
QTest::ignoreMessage(
QtWarningMsg, QRegularExpression("Cannot insert QObject_QML_[0-9]+\\(0x[0-9a-f]+\\) "
"into a QML list of AListItem_QMLTYPE_[0-9]+\\*"));
QTest::ignoreMessage(
QtWarningMsg, QRegularExpression("Cannot splice QObject_QML_[0-9]+\\(0x[0-9a-f]+\\) "
"into a QML list of AListItem_QMLTYPE_[0-9]+\\*"));
QTest::ignoreMessage(
QtWarningMsg, QRegularExpression("Cannot unshift QObject_QML_[0-9]+\\(0x[0-9a-f]+\\) "
"into a QML list of AListItem_QMLTYPE_[0-9]+\\*"));
QScopedPointer<QObject> object(component.create());
QVERIFY(!object.isNull());
QQmlListReference list1(object.data(), "items");
QCOMPARE(list1.size(), 5);
for (qsizetype i = 0; i < 5; ++i)
QCOMPARE(list1.at(i), nullptr);
QQmlComponent item(&engine, testFileUrl("AListItem.qml"));
QScopedPointer<QObject> i1(item.create());
QScopedPointer<QObject> i2(item.create());
@ -873,7 +895,6 @@ void tst_qqmllistreference::compositeListProperty()
QVERIFY(!i2.isNull());
// We know the element type now.
QQmlListReference list1(object.data(), "items");
QVERIFY(list1.listElementType() != nullptr);
QVERIFY(list1.append(i1.data()));
QVERIFY(list1.replace(0, i2.data()));

6
tests/auto/quickcontrols/controls/data/tst_buttongroup.qml
View File

@ -304,7 +304,9 @@ TestCase {
id: repeater
Column {
id: column
property ButtonGroup group: ButtonGroup { buttons: column.children }
property ButtonGroup group: ButtonGroup {
buttons: column.children.filter((child) => child !== r)
}
property alias repeater: r
Repeater {
id: r
@ -392,7 +394,7 @@ TestCase {
id: checkedButtonColumn
Column {
id: column
ButtonGroup { buttons: column.children }
ButtonGroup { buttons: column.children.filter((child) => child !== repeater) }
Repeater {
id: repeater
delegate: Button {