From dd514160ce169734e23a146a2c115fed55a69260 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 Pick-to: 6.7 Change-Id: If4c62ea53ac9bccd423f00f0f03afd6ba6bdc4f5 Reviewed-by: Marc Mutz Reviewed-by: Thiago Macieira --- 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 7c4c5cc12af..d0421f70fad 100644 --- a/src/corelib/serialization/qdatastream.cpp +++ b/src/corelib/serialization/qdatastream.cpp @@ -1397,8 +1397,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 24c5b87ee00..217bdab7d34 100644 --- a/src/corelib/serialization/qdatastream.h +++ b/src/corelib/serialization/qdatastream.h @@ -212,7 +212,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; @@ -339,7 +339,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; @@ -349,7 +350,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) { @@ -363,7 +365,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) { @@ -425,16 +428,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 7cdd7390542..7aecdec5350 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 a7866f6bb74..7b242f4519b 100644 --- a/tests/manual/corelib/qdatastream/tst_manualqdatastream.cpp +++ b/tests/manual/corelib/qdatastream/tst_manualqdatastream.cpp @@ -28,6 +28,7 @@ public slots: private slots: void stream_bigQString(); + void stream_bigQByteArray(); void stream_bigQList(); void stream_bigQSet(); void stream_bigQMap(); @@ -37,6 +38,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 @@ -110,11 +115,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; @@ -122,17 +129,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 @@ -142,6 +155,11 @@ void tst_QDataStream::stream_bigQString() { stream_big(); } + +void tst_QDataStream::stream_bigQByteArray() +{ + stream_big(); +} void tst_QDataStream::stream_bigQList() { stream_big>();