1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
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>
This commit is contained in:
Thiago Macieira 2020-11-07 08:08:31 -08:00
parent bab2cd1125
commit 58bea7f3a2
3 changed files with 31 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
src/corelib/serialization/qcborstreamreader.cpp
View File

@ -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);

19
tests/auto/corelib/serialization/cborlargedatavalidation.cpp
View File

@ -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);

12
tests/auto/corelib/serialization/qcborstreamreader/tst_qcborstreamreader.cpp
View File

@ -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;