QCborStreamReader::next: don't allocate too much memory in a QBA

Because CBOR strings are encoded in UTF-8, it's possible that the string
that won't fit a QString in UTF-16 would still fit QByteArray in UTF-8
(e.g., anything US-ASCII and most Latin text).

The previous solution was an improvement because we used to read into a
QByteArray then convert the QByteArray to QString, thus using 3x the
amount of memory (1x in QByteArray, 2x in QString). The previous commit
skipped the middle allocation and made the regular readString() function
do the decoding either directly on source memory or by reading in small
chunks (16 kB).

Future improvement for Qt 6.1: add readStringChunk(char16_t *, qsizetype)
so we can do the validation / skipping at O(1) memory.

Pick-to: 5.15 6.0
Change-Id: I7b9b97ae9b32412abdc6fffd1645458c655cc566
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

3 files changed, 31 insertions, 25 deletions

diff --git a/src/corelib/serialization/qcborstreamreader.cpp b/src/corelib/serialization/qcborstreamreader.cpp
index ef2ec08b88..f1bea6a1f1 100644
--- a/src/corelib/serialization/qcborstreamreader.cpp
+++ b/src/corelib/serialization/qcborstreamreader.cpp
@@ -1076,19 +1076,18 @@ bool QCborStreamReader::next(int maxRecursion)
             next(maxRecursion - 1);
         if (lastError() == QCborError::NoError)
             leaveContainer();
-    } else if (isString() || isByteArray()) {
-        auto r = _readByteArray_helper();
-        while (r.status == Ok) {
-            if (isString() && r.data.size() > MaxStringSize) {
-                d->handleError(CborErrorDataTooLarge);
-                break;
-            }
-            if (isString() && !QUtf8::isValidUtf8(r.data).isValidUtf8) {
-                d->handleError(CborErrorInvalidUtf8TextString);
-                break;
-            }
-            r = _readByteArray_helper();
-        }
+    } else if (isByteArray()) {
+        char c;
+        StringResult<qsizetype> r;
+        do {
+            r = readStringChunk(&c, 1);
+        } while (r.status == Ok);
+    } else if (isString()) {
+        // we need to use actual readString so we get UTF-8 validation
+        StringResult<QString> r;
+        do {
+            r = readString();
+        } while (r.status == Ok);
     } else {
         // fixed types
         CborError err = cbor_value_advance_fixed(&d->currentElement);
diff --git a/tests/auto/corelib/serialization/cborlargedatavalidation.cpp b/tests/auto/corelib/serialization/cborlargedatavalidation.cpp
index c5cf1dc04e..a9a31ddf57 100644
--- a/tests/auto/corelib/serialization/cborlargedatavalidation.cpp
+++ b/tests/auto/corelib/serialization/cborlargedatavalidation.cpp
@@ -117,6 +117,7 @@ void addValidationHugeDevice(qsizetype byteArrayInvalid, qsizetype stringInvalid
     qRegisterMetaType<QSharedPointer<QIODevice>>();
     QTest::addColumn<QSharedPointer<QIODevice>>("device");
     QTest::addColumn<CborError>("expectedError");
+    QTest::addColumn<CborError>("expectedValidationError");
 
     char buf[1 + sizeof(quint64)];
     auto device = [&buf](QCborStreamReader::Type t, quint64 size) {
@@ -130,21 +131,21 @@ void addValidationHugeDevice(qsizetype byteArrayInvalid, qsizetype stringInvalid
 
     // do the exact limits
     QTest::newRow("bytearray-just-too-big")
-            << device(QCborStreamReader::ByteArray, byteArrayInvalid) << CborErrorDataTooLarge;
-
-    // TODO: Fix this to work for 64-bit. The test tries to allocate too much data and fails.
-    if (sizeof(size_t) == 4) {
-        QTest::newRow("string-just-too-big")
-                << device(QCborStreamReader::String, stringInvalid) << CborErrorDataTooLarge;
-    }
+            << device(QCborStreamReader::ByteArray, byteArrayInvalid)
+            << CborErrorDataTooLarge << CborNoError;
+    QTest::newRow("string-just-too-big")
+            << device(QCborStreamReader::String, stringInvalid)
+            << CborErrorDataTooLarge << CborErrorDataTooLarge;
 
     auto addSize = [=](const char *sizename, qint64 size) {
         if (byteArrayInvalid < size)
             QTest::addRow("bytearray-%s", sizename)
-                << device(QCborStreamReader::ByteArray, size) << CborErrorDataTooLarge;
+                << device(QCborStreamReader::ByteArray, size)
+                << CborErrorDataTooLarge << CborNoError;
         if (stringInvalid < size)
             QTest::addRow("string-%s", sizename)
-                << device(QCborStreamReader::String, size) << CborErrorDataTooLarge;
+                << device(QCborStreamReader::String, size)
+                << CborErrorDataTooLarge << CborErrorDataTooLarge;
     };
     addSize("1GB", quint64(1) << 30);
     addSize("2GB", quint64(1) << 31);
diff --git a/tests/auto/corelib/serialization/qcborstreamreader/tst_qcborstreamreader.cpp b/tests/auto/corelib/serialization/qcborstreamreader/tst_qcborstreamreader.cpp
index 437d8c40ce..f04f26b42f 100644
--- a/tests/auto/corelib/serialization/qcborstreamreader/tst_qcborstreamreader.cpp
+++ b/tests/auto/corelib/serialization/qcborstreamreader/tst_qcborstreamreader.cpp
@@ -958,6 +958,7 @@ void tst_QCborStreamReader::hugeDeviceValidation()
 
     QFETCH(QSharedPointer<QIODevice>, device);
     QFETCH(CborError, expectedError);
+    QFETCH(CborError, expectedValidationError);
     QCborError error = { QCborError::Code(expectedError) };
 
     device->open(QIODevice::ReadOnly | QIODevice::Unbuffered);
@@ -966,10 +967,15 @@ void tst_QCborStreamReader::hugeDeviceValidation()
     QVERIFY(parseOne(reader).isEmpty());
     QCOMPARE(reader.lastError(), error);
 
-    // next() should fail
     reader.reset();
-    QVERIFY(!reader.next());
-    QCOMPARE(reader.lastError(), error);
+    error = { QCborError::Code(expectedValidationError) };
+    if (error == QCborError{}) {
+        // this test actually succeeds, so don't do it to avoid large memory consumption
+    } else {
+        // next() should fail
+        QVERIFY(!reader.next());
+        QCOMPARE(reader.lastError(), error);
+    }
 }
 
 static const int Recursions = 3;