QString/QByteArray: fix append() wrt. raw data

When appending to an empty string or byte array, we optimize and
copy the internal pointer. But if the other string/byte array was
created with fromRawData this might be temporary data on the stack/heap
and might be de-allocated or overwritten before the string/byte array
is used or is forced to make a deep-copy. This would lead to incorrect
data being used.

This is easy to overlook if you plan to append multiple strings
together, potentially supplied through an argument. Upon appending a
second string it would make a full copy, but there might not be a
guarantee for that. So, it's hard for users to avoid this pitfall!

Fixes: QTBUG-115752
Pick-to: 6.6 6.5 6.2
Change-Id: Ia9aa5f463121c2ce2e0e8eee8a6c8612b7297f2b
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

src/corelib/text/qbytearray.cpp

@@ -2039,7 +2039,10 @@ QByteArray &QByteArray::append(const QByteArray &ba)
 {
     if (!ba.isNull()) {
         if (isNull()) {
-            operator=(ba);
+            if (Q_UNLIKELY(!ba.d.isMutable()))
+                assign(ba); // fromRawData, so we do a deep copy
+            else
+                operator=(ba);
         } else if (ba.size()) {
             append(QByteArrayView(ba));
         }

src/corelib/text/qstring.cpp

@@ -3144,7 +3144,10 @@ QString &QString::append(const QString &str)
 {
     if (!str.isNull()) {
         if (isNull()) {
-            operator=(str);
+            if (Q_UNLIKELY(!str.d.isMutable()))
+                assign(str); // fromRawData, so we do a deep copy
+            else
+                operator=(str);
         } else if (str.size()) {
             append(str.constData(), str.size());
         }

tests/auto/corelib/text/qbytearray/tst_qbytearray.cpp

@@ -51,6 +51,7 @@ private slots:
     void prependExtended_data();
     void prependExtended();
     void append();
+    void appendFromRawData();
     void appendExtended_data();
     void appendExtended();
     void appendEmptyNull();
@@ -914,6 +915,20 @@ void tst_QByteArray::append()
     }
 }
 
+void tst_QByteArray::appendFromRawData()
+{
+    char rawData[] = "Hello World!";
+    QByteArray ba = QByteArray::fromRawData(rawData, std::size(rawData) - 1);
+
+    QByteArray copy;
+    copy.append(ba);
+    QCOMPARE(copy, ba);
+    // We make an _actual_ copy, because appending a byte array
+    // created with fromRawData() might be optimized to copy the DataPointer,
+    // which means we may point to temporary stack data.
+    QCOMPARE_NE((void *)copy.constData(), (void *)ba.constData());
+}
+
 void tst_QByteArray::appendExtended_data()
 {
     prependExtended_data();

tests/auto/corelib/text/qstring/tst_qstring.cpp

@@ -469,6 +469,8 @@ private slots:
     void append_bytearray_special_cases();
 #endif
 
+    void appendFromRawData();
+
     void operator_pluseq_qstring()            { operator_pluseq_impl<QString>(); }
     void operator_pluseq_qstring_data()       { operator_pluseq_data(); }
     void operator_pluseq_qstringview()        { operator_pluseq_impl<QStringView, QString &(QString::*)(QStringView)>(); }
@@ -3404,6 +3406,21 @@ void tst_QString::append_bytearray_special_cases()
 }
 #endif // !defined(QT_RESTRICTED_CAST_FROM_ASCII) && !defined(QT_NO_CAST_FROM_ASCII)
 
+void tst_QString::appendFromRawData()
+{
+    const char16_t utf[] = u"Hello World!";
+    auto *rawData = reinterpret_cast<const QChar *>(utf);
+    QString str = QString::fromRawData(rawData, std::size(utf) - 1);
+
+    QString copy;
+    copy.append(str);
+    QCOMPARE(copy, str);
+    // We make an _actual_ copy, because appending a byte array
+    // created with fromRawData() might be optimized to copy the DataPointer,
+    // which means we may point to temporary stack data.
+    QCOMPARE_NE((void *)copy.constData(), (void *)str.constData());
+}
+
 void tst_QString::assign()
 {
     // QString &assign(QAnyStringView)