QJsonObject: add missing detach2() calls

The refactoring to use CBOR missed two places where we could assign from
the same object and thus cause corruption. In fixing this issue, I found
a design flaw in QJsonObject, see Q_EXPECT_FAILing unit test and task
QTBUG-83398.

[ChangeLog][QtCore][QJsonObject] Fixed a regression from 5.13 that
incorrect results when assigning elements from an object to itself.

Fixes: QTBUG-83366
Change-Id: Ibdc95e9af7bd456a94ecfffd1603df24b06713aa
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

2 files changed, 100 insertions, 0 deletions

diff --git a/src/corelib/serialization/qjsonobject.cpp b/src/corelib/serialization/qjsonobject.cpp
index b76e50e2d2..850e878571 100644
--- a/src/corelib/serialization/qjsonobject.cpp
+++ b/src/corelib/serialization/qjsonobject.cpp
@@ -452,9 +452,11 @@ QJsonValueRef QJsonObject::atImpl(T key)
     bool keyExists = false;
     int index = indexOf(o, key, &keyExists);
     if (!keyExists) {
+        detach2(o->elements.length() / 2 + 1);
         o->insertAt(index, key);
         o->insertAt(index + 1, QCborValue::fromJsonValue(QJsonValue()));
     }
+    // detaching will happen if and when this QJsonValueRef is assigned to
     return QJsonValueRef(this, index / 2);
 }
 
@@ -1469,6 +1471,7 @@ QJsonValue QJsonObject::valueAt(int i) const
 void QJsonObject::setValueAt(int i, const QJsonValue &val)
 {
     Q_ASSERT(o && i >= 0 && 2 * i + 1 < o->elements.length());
+    detach2();
     if (val.isUndefined()) {
         o->removeAt(2 * i + 1);
         o->removeAt(2 * i);
diff --git a/tests/auto/corelib/serialization/json/tst_qtjson.cpp b/tests/auto/corelib/serialization/json/tst_qtjson.cpp
index 45f815f810..7da20772f8 100644
--- a/tests/auto/corelib/serialization/json/tst_qtjson.cpp
+++ b/tests/auto/corelib/serialization/json/tst_qtjson.cpp
@@ -54,7 +54,9 @@ private Q_SLOTS:
 
     void testObjectSimple();
     void testObjectSmallKeys();
+    void testObjectInsertCopies();
     void testArraySimple();
+    void testArrayInsertCopies();
     void testValueObject();
     void testValueArray();
     void testObjectNested();
@@ -531,6 +533,75 @@ void tst_QtJson::testObjectSmallKeys()
     QCOMPARE(data1.end() - data1.begin(), 3);
 }
 
+void tst_QtJson::testObjectInsertCopies()
+{
+    {
+        QJsonObject obj;
+        obj["prop1"] = "TEST";
+        QCOMPARE(obj.size(), 1);
+        QCOMPARE(obj.value("prop1"), "TEST");
+
+        obj["prop2"] = obj.value("prop1");
+        QCOMPARE(obj.size(), 2);
+        QCOMPARE(obj.value("prop1"), "TEST");
+        QCOMPARE(obj.value("prop2"), "TEST");
+    }
+    {
+        // see QTBUG-83366
+        QJsonObject obj;
+        obj["value"] = "TEST";
+        QCOMPARE(obj.size(), 1);
+        QCOMPARE(obj.value("value"), "TEST");
+
+        obj["prop2"] = obj.value("value");
+        QCOMPARE(obj.size(), 2);
+        QCOMPARE(obj.value("value"), "TEST");
+        QCOMPARE(obj.value("prop2"), "TEST");
+    }
+    {
+        QJsonObject obj;
+        obj["value"] = "TEST";
+        QCOMPARE(obj.size(), 1);
+        QCOMPARE(obj.value("value"), "TEST");
+
+        // same as previous, but this is a QJsonValueRef
+        QJsonValueRef rv = obj["prop2"];
+        rv = obj["value"];
+        QCOMPARE(obj.size(), 2);
+        QCOMPARE(obj.value("value"), "TEST");
+        QCOMPARE(obj.value("prop2"), "TEST");
+    }
+    {
+        QJsonObject obj;
+        obj["value"] = "TEST";
+        QCOMPARE(obj.size(), 1);
+        QCOMPARE(obj.value("value"), "TEST");
+
+        // same as previous, but this is a QJsonValueRef
+        QJsonValueRef rv = obj["value"];
+        obj["prop2"] = rv;
+        QCOMPARE(obj.size(), 2);
+        QCOMPARE(obj.value("value"), "TEST");
+        QEXPECT_FAIL("", "QTBUG-83398: design flaw: the obj[] call invalidates the QJsonValueRef", Continue);
+        QCOMPARE(obj.value("prop2"), "TEST");
+    }
+    {
+        QJsonObject obj;
+        obj["value"] = "TEST";
+        QCOMPARE(obj.size(), 1);
+        QCOMPARE(obj.value("value"), "TEST");
+
+        QJsonValueRef v = obj["value"];
+        QJsonObject obj2 = obj;
+        obj.insert("prop2", v);
+        QCOMPARE(obj.size(), 2);
+        QCOMPARE(obj.value("value"), "TEST");
+        QCOMPARE(obj.value("prop2"), "TEST");
+        QCOMPARE(obj2.size(), 1);
+        QCOMPARE(obj2.value("value"), "TEST");
+    }
+}
+
 void tst_QtJson::testArraySimple()
 {
     QJsonArray array;
@@ -584,6 +655,32 @@ void tst_QtJson::testArraySimple()
     QCOMPARE(array.at(1), QJsonValue(QLatin1String("test")));
 }
 
+void tst_QtJson::testArrayInsertCopies()
+{
+    {
+        QJsonArray array;
+        array.append("TEST");
+        QCOMPARE(array.size(), 1);
+        QCOMPARE(array.at(0), "TEST");
+
+        array.append(array.at(0));
+        QCOMPARE(array.size(), 2);
+        QCOMPARE(array.at(0), "TEST");
+        QCOMPARE(array.at(1), "TEST");
+    }
+    {
+        QJsonArray array;
+        array.append("TEST");
+        QCOMPARE(array.size(), 1);
+        QCOMPARE(array.at(0), "TEST");
+
+        array.prepend(array.at(0));
+        QCOMPARE(array.size(), 2);
+        QCOMPARE(array.at(0), "TEST");
+        QCOMPARE(array.at(1), "TEST");
+    }
+}
+
 void tst_QtJson::testValueObject()
 {
     QJsonObject object;