From cc62a35bbebc9d069c9dbaeb703dfd6afea548e3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Vr=C3=A1til?= <daniel.vratil@kdab.com>
Date: Tue, 20 Mar 2018 18:14:20 +0100
Subject: Fix QQmlListModel crash when appending an empty array in debug mode

Calling QQmlListModel::append() with an empty JS array triggers an assert
in QAbstractItemModel::beginInsertRows() because it's called with
negative "last" parameter.

Change-Id: I202da260d79f2e6677c663c5785ff754c715fef8
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>
---
 src/qml/types/qqmllistmodel.cpp                    | 23 +++++++++++-----------
 tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp | 22 +++++++++++++++++++++
 2 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/src/qml/types/qqmllistmodel.cpp b/src/qml/types/qqmllistmodel.cpp
index d4fc02cd3e..c4e33d572d 100644
--- a/src/qml/types/qqmllistmodel.cpp
+++ b/src/qml/types/qqmllistmodel.cpp
@@ -2351,21 +2351,22 @@ void QQmlListModel::append(QQmlV4Function *args)
             QV4::ScopedObject argObject(scope);
 
             int objectArrayLength = objectArray->getLength();
+            if (objectArrayLength > 0) {
+                int index = count();
+                emitItemsAboutToBeInserted(index, objectArrayLength);
 
-            int index = count();
-            emitItemsAboutToBeInserted(index, objectArrayLength);
+                for (int i=0 ; i < objectArrayLength ; ++i) {
+                    argObject = objectArray->getIndexed(i);
 
-            for (int i=0 ; i < objectArrayLength ; ++i) {
-                argObject = objectArray->getIndexed(i);
-
-                if (m_dynamicRoles) {
-                    m_modelObjects.append(DynamicRoleModelNode::create(scope.engine->variantMapFromJS(argObject), this));
-                } else {
-                    m_listModel->append(argObject);
+                    if (m_dynamicRoles) {
+                        m_modelObjects.append(DynamicRoleModelNode::create(scope.engine->variantMapFromJS(argObject), this));
+                    } else {
+                        m_listModel->append(argObject);
+                    }
                 }
-            }
 
-            emitItemsInserted();
+                emitItemsInserted();
+            }
         } else if (argObject) {
             int index;
 
diff --git a/tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp b/tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp
index 4618ea4071..9fdc54f067 100644
--- a/tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp
+++ b/tests/auto/qml/qqmllistmodel/tst_qqmllistmodel.cpp
@@ -125,6 +125,7 @@ private slots:
     void bindingsOnGetResult();
     void stringifyModelEntry();
     void qobjectTrackerForDynamicModelObjects();
+    void crash_append_empty_array();
 };
 
 bool tst_qqmllistmodel::compareVariantList(const QVariantList &testList, QVariant object)
@@ -1534,6 +1535,27 @@ void tst_qqmllistmodel::qobjectTrackerForDynamicModelObjects()
     QVERIFY(!ddata->jsWrapper.isNullOrUndefined());
 }
 
+void tst_qqmllistmodel::crash_append_empty_array()
+{
+    QQmlEngine engine;
+    QQmlComponent component(&engine);
+    component.setData(
+                "import QtQuick 2.0\n"
+                "Item {\n"
+                "   ListModel {\n"
+                "      id: testModel\n"
+                "      objectName: \"testModel\""
+                "   }\n"
+                "}\n", QUrl());
+    QScopedPointer<QObject> scene(component.create());
+    QQmlListModel *model = scene->findChild<QQmlListModel*>("testModel");
+    QSignalSpy spy(model, &QQmlListModel::rowsAboutToBeInserted);
+    QQmlExpression expr(engine.rootContext(), model, "append(new Array())");
+    expr.evaluate();
+    QVERIFY2(!expr.hasError(), QTest::toString(expr.error().toString()));
+    QCOMPARE(spy.count(), 0);
+}
+
 QTEST_MAIN(tst_qqmllistmodel)
 
 #include "tst_qqmllistmodel.moc"
-- 
cgit v1.2.3