From 7ccd63fd895ae292dcde805c0cad38c734963c77 Mon Sep 17 00:00:00 2001 From: Ivan Solovev Date: Wed, 24 Jan 2024 17:30:58 +0100 Subject: [PATCH] QDataStream: use SizeLimitExceeded status in write operations Set this status when the stream tries to write more data than the current serialization format supports. Update the methods that write containers to return early if they fail to write the container size, and do not try to serialize the elements. Convert the manual tst_manualqdatastream test into a data-driven test, allowing us to specify various stream versions. Adjust the test code to check that the SizeLimitExceeded status is set when the stream version is <= Qt_6_6. Amends fd48ce0b73c74dafd5db27bc1f2752ef665df7ef Found in 6.7 API review Change-Id: If4c62ea53ac9bccd423f00f0f03afd6ba6bdc4f5 Reviewed-by: Marc Mutz Reviewed-by: Thiago Macieira (cherry picked from commit dd514160ce169734e23a146a2c115fed55a69260) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/serialization/qdatastream.cpp | 4 +- src/corelib/serialization/qdatastream.h | 26 ++++++++----- .../qdatastream/tst_qdatastream.cpp | 14 +++++++ .../qdatastream/tst_manualqdatastream.cpp | 38 ++++++++++++++----- 4 files changed, 60 insertions(+), 22 deletions(-) diff --git a/src/corelib/serialization/qdatastream.cpp b/src/corelib/serialization/qdatastream.cpp index 27a2ee9fdd6..4239dc23977 100644 --- a/src/corelib/serialization/qdatastream.cpp +++ b/src/corelib/serialization/qdatastream.cpp @@ -1393,8 +1393,8 @@ QDataStream &QDataStream::writeBytes(const char *s, qint64 len) return *this; } CHECK_STREAM_WRITE_PRECOND(*this) - writeQSizeType(*this, len); // write length specifier - if (len > 0) + // Write length then, if any, content + if (writeQSizeType(*this, len) && len > 0) writeRawData(s, len); return *this; } diff --git a/src/corelib/serialization/qdatastream.h b/src/corelib/serialization/qdatastream.h index 80f4b234539..55299c6eff3 100644 --- a/src/corelib/serialization/qdatastream.h +++ b/src/corelib/serialization/qdatastream.h @@ -211,7 +211,7 @@ private: #endif qint64 readBlock(char *data, qint64 len); static inline qint64 readQSizeType(QDataStream &s); - static inline void writeQSizeType(QDataStream &s, qint64 value); + static inline bool writeQSizeType(QDataStream &s, qint64 value); enum class QDataStreamSizes : quint32 { NullCode = 0xffffffffu, ExtendedSize = 0xfffffffeu }; friend class QtPrivate::StreamStateSaver; @@ -338,7 +338,8 @@ QDataStream &readAssociativeContainer(QDataStream &s, Container &c) template QDataStream &writeSequentialContainer(QDataStream &s, const Container &c) { - QDataStream::writeQSizeType(s, c.size()); + if (!QDataStream::writeQSizeType(s, c.size())) + return s; for (const typename Container::value_type &t : c) s << t; @@ -348,7 +349,8 @@ QDataStream &writeSequentialContainer(QDataStream &s, const Container &c) template QDataStream &writeAssociativeContainer(QDataStream &s, const Container &c) { - QDataStream::writeQSizeType(s, c.size()); + if (!QDataStream::writeQSizeType(s, c.size())) + return s; auto it = c.constBegin(); auto end = c.constEnd(); while (it != end) { @@ -362,7 +364,8 @@ QDataStream &writeAssociativeContainer(QDataStream &s, const Container &c) template QDataStream &writeAssociativeMultiContainer(QDataStream &s, const Container &c) { - QDataStream::writeQSizeType(s, c.size()); + if (!QDataStream::writeQSizeType(s, c.size())) + return s; auto it = c.constBegin(); auto end = c.constEnd(); while (it != end) { @@ -424,16 +427,19 @@ qint64 QDataStream::readQSizeType(QDataStream &s) return extendedLen; } -void QDataStream::writeQSizeType(QDataStream &s, qint64 value) +bool QDataStream::writeQSizeType(QDataStream &s, qint64 value) { - if (value < qint64(QDataStreamSizes::ExtendedSize)) + if (value < qint64(QDataStreamSizes::ExtendedSize)) { s << quint32(value); - else if (s.version() >= QDataStream::Qt_6_7) + } else if (s.version() >= QDataStream::Qt_6_7) { s << quint32(QDataStreamSizes::ExtendedSize) << value; - else if (value == qint64(QDataStreamSizes::ExtendedSize)) + } else if (value == qint64(QDataStreamSizes::ExtendedSize)) { s << quint32(QDataStreamSizes::ExtendedSize); - else - s.setStatus(QDataStream::WriteFailed); // value is too big for old format + } else { + s.setStatus(QDataStream::SizeLimitExceeded); // value is too big for old format + return false; + } + return true; } inline QDataStream &QDataStream::operator>>(char &i) diff --git a/tests/auto/corelib/serialization/qdatastream/tst_qdatastream.cpp b/tests/auto/corelib/serialization/qdatastream/tst_qdatastream.cpp index 09f2bbc860f..b934e54f432 100644 --- a/tests/auto/corelib/serialization/qdatastream/tst_qdatastream.cpp +++ b/tests/auto/corelib/serialization/qdatastream/tst_qdatastream.cpp @@ -134,6 +134,7 @@ private slots: void stream_atEnd(); void stream_writeError(); + void stream_writeSizeLimitExceeded(); void stream_QByteArray2(); @@ -2190,6 +2191,19 @@ void tst_QDataStream::stream_writeError() TEST_WRITE_ERROR(.writeRawData("test", 4)) } +void tst_QDataStream::stream_writeSizeLimitExceeded() +{ + QByteArray ba; + QDataStream ds(&ba, QDataStream::ReadWrite); + // Set the version that supports only 32-bit data size + ds.setVersion(QDataStream::Qt_6_6); + QCOMPARE(ds.status(), QDataStream::Ok); + const qint64 size = qint64(std::numeric_limits::max()) + 1; + ds.writeBytes("", size); + QCOMPARE(ds.status(), QDataStream::SizeLimitExceeded); + QVERIFY(ba.isEmpty()); +} + void tst_QDataStream::stream_QByteArray2() { QByteArray ba; diff --git a/tests/manual/corelib/qdatastream/tst_manualqdatastream.cpp b/tests/manual/corelib/qdatastream/tst_manualqdatastream.cpp index 334f27dc936..4fa5ebb36b5 100644 --- a/tests/manual/corelib/qdatastream/tst_manualqdatastream.cpp +++ b/tests/manual/corelib/qdatastream/tst_manualqdatastream.cpp @@ -29,6 +29,7 @@ public slots: private slots: void stream_bigQString(); + void stream_bigQByteArray(); void stream_bigQList(); void stream_bigQSet(); void stream_bigQMap(); @@ -39,6 +40,10 @@ private slots: void tst_QDataStream::initTestCase() { qputenv("QTEST_FUNCTION_TIMEOUT", "9000000"); + + QTest::addColumn("streamVersion"); + QTest::addRow("current") << QDataStream::Qt_DefaultCompiledVersion; + QTest::addRow("Qt_6_6") << QDataStream::Qt_6_6; } template @@ -112,11 +117,13 @@ template void tst_QDataStream::stream_big() { #if QT_POINTER_SIZE > 4 + QFETCH_GLOBAL(const QDataStream::Version, streamVersion); QElapsedTimer timer; T input; fill(input); QByteArray ba; QDataStream inputstream(&ba, QIODevice::WriteOnly); + inputstream.setVersion(streamVersion); timer.start(); try { inputstream << input; @@ -124,17 +131,23 @@ void tst_QDataStream::stream_big() QSKIP("Not enough memory to copy into QDataStream."); } qDebug("Streamed into QDataStream in %lld ms", timer.elapsed()); - T output; - QDataStream outputstream(ba); - timer.start(); - try { - outputstream >> output; - } catch (const std::bad_alloc &) { - QSKIP("Not enough memory to copy out of QDataStream."); + if (streamVersion < QDataStream::Qt_6_7) { + // old versions do not support data size more than 4 GiB + QCOMPARE(inputstream.status(), QDataStream::SizeLimitExceeded); + QVERIFY(ba.isEmpty()); + } else { + T output; + QDataStream outputstream(ba); + timer.start(); + try { + outputstream >> output; + } catch (const std::bad_alloc &) { + QSKIP("Not enough memory to copy out of QDataStream."); + } + qDebug("Streamed out of QDataStream in %lld ms", timer.elapsed()); + QCOMPARE(input.size(), output.size()); + QCOMPARE(input, output); } - qDebug("Streamed out of QDataStream in %lld ms", timer.elapsed()); - QCOMPARE(input.size(), output.size()); - QCOMPARE(input, output); #else QSKIP("This test is 64-bit only."); #endif @@ -144,6 +157,11 @@ void tst_QDataStream::stream_bigQString() { stream_big(); } + +void tst_QDataStream::stream_bigQByteArray() +{ + stream_big(); +} void tst_QDataStream::stream_bigQList() { stream_big>();