From 902a5e7aaa0ec156d19b5a7988eff1809a6a2046 Mon Sep 17 00:00:00 2001 From: Alex Trotsenko Date: Fri, 17 Jun 2016 15:07:26 +0300 Subject: [PATCH] QDataStream: adjust containers' deserialization in transaction mode If an error occurs during the transaction, we should prevent the containers from being successfully read. So, check the status of the stream before reading the container, because the deserialization procedure temporarily resets it on entry. Task-number: QTBUG-54022 Change-Id: Ie955c2fa3e449374f0f8403f00e487efa2bfdaf3 Reviewed-by: Oswald Buddenhagen Reviewed-by: Edward Welbourne --- src/corelib/io/qdatastream.h | 7 +- .../io/qdatastream/tst_qdatastream.cpp | 144 +++++++++++------- 2 files changed, 97 insertions(+), 54 deletions(-) diff --git a/src/corelib/io/qdatastream.h b/src/corelib/io/qdatastream.h index 260dd519e3f..e8634fddef2 100644 --- a/src/corelib/io/qdatastream.h +++ b/src/corelib/io/qdatastream.h @@ -63,6 +63,9 @@ template class QMap; #if !defined(QT_NO_DATASTREAM) || defined(QT_BOOTSTRAPPED) class QDataStreamPrivate; +namespace QtPrivate { +class StreamStateSaver; +} class Q_CORE_EXPORT QDataStream { public: @@ -192,6 +195,7 @@ private: Status q_status; int readBlock(char *data, int len); + friend class QtPrivate::StreamStateSaver; }; namespace QtPrivate { @@ -201,7 +205,8 @@ class StreamStateSaver public: inline StreamStateSaver(QDataStream *s) : stream(s), oldStatus(s->status()) { - stream->resetStatus(); + if (!stream->dev || !stream->dev->isTransactionStarted()) + stream->resetStatus(); } inline ~StreamStateSaver() { diff --git a/tests/auto/corelib/io/qdatastream/tst_qdatastream.cpp b/tests/auto/corelib/io/qdatastream/tst_qdatastream.cpp index b7e7344b16a..447cf2845e6 100644 --- a/tests/auto/corelib/io/qdatastream/tst_qdatastream.cpp +++ b/tests/auto/corelib/io/qdatastream/tst_qdatastream.cpp @@ -2750,27 +2750,43 @@ void tst_QDataStream::status_QBitArray() } #define MAP_TEST(byteArray, initialStatus, expectedStatus, expectedHash) \ - { \ - QByteArray ba = byteArray; \ - QDataStream stream(&ba, QIODevice::ReadOnly); \ - stream.setStatus(initialStatus); \ - stream >> hash; \ - QCOMPARE((int)stream.status(), (int)expectedStatus); \ - QCOMPARE(hash.size(), expectedHash.size()); \ - QCOMPARE(hash, expectedHash); \ - } \ - { \ - QByteArray ba = byteArray; \ - StringMap expectedMap; \ - StringHash::const_iterator it = expectedHash.constBegin(); \ - for (; it != expectedHash.constEnd(); ++it) \ - expectedMap.insert(it.key(), it.value()); \ - QDataStream stream(&ba, QIODevice::ReadOnly); \ - stream.setStatus(initialStatus); \ - stream >> map; \ - QCOMPARE((int)stream.status(), (int)expectedStatus); \ - QCOMPARE(map.size(), expectedMap.size()); \ - QCOMPARE(map, expectedMap); \ + for (bool inTransaction = false;; inTransaction = true) { \ + { \ + QByteArray ba = byteArray; \ + QDataStream stream(&ba, QIODevice::ReadOnly); \ + if (inTransaction) \ + stream.startTransaction(); \ + stream.setStatus(initialStatus); \ + stream >> hash; \ + QCOMPARE((int)stream.status(), (int)expectedStatus); \ + if (!inTransaction || stream.commitTransaction()) { \ + QCOMPARE(hash.size(), expectedHash.size()); \ + QCOMPARE(hash, expectedHash); \ + } else { \ + QVERIFY(hash.isEmpty()); \ + } \ + } \ + { \ + QByteArray ba = byteArray; \ + StringMap expectedMap; \ + StringHash::const_iterator it = expectedHash.constBegin(); \ + for (; it != expectedHash.constEnd(); ++it) \ + expectedMap.insert(it.key(), it.value()); \ + QDataStream stream(&ba, QIODevice::ReadOnly); \ + if (inTransaction) \ + stream.startTransaction(); \ + stream.setStatus(initialStatus); \ + stream >> map; \ + QCOMPARE((int)stream.status(), (int)expectedStatus); \ + if (!inTransaction || stream.commitTransaction()) { \ + QCOMPARE(map.size(), expectedMap.size()); \ + QCOMPARE(map, expectedMap); \ + } else { \ + QVERIFY(map.isEmpty()); \ + } \ + } \ + if (inTransaction) \ + break; \ } void tst_QDataStream::status_QHash_QMap() @@ -2815,38 +2831,60 @@ void tst_QDataStream::status_QHash_QMap() } #define LIST_TEST(byteArray, initialStatus, expectedStatus, expectedList) \ - { \ - QByteArray ba = byteArray; \ - QDataStream stream(&ba, QIODevice::ReadOnly); \ - stream.setStatus(initialStatus); \ - stream >> list; \ - QCOMPARE((int)stream.status(), (int)expectedStatus); \ - QCOMPARE(list.size(), expectedList.size()); \ - QCOMPARE(list, expectedList); \ - } \ - { \ - LinkedList expectedLinkedList; \ - for (int i = 0; i < expectedList.count(); ++i) \ - expectedLinkedList << expectedList.at(i); \ - QByteArray ba = byteArray; \ - QDataStream stream(&ba, QIODevice::ReadOnly); \ - stream.setStatus(initialStatus); \ - stream >> linkedList; \ - QCOMPARE((int)stream.status(), (int)expectedStatus); \ - QCOMPARE(linkedList.size(), expectedLinkedList.size()); \ - QCOMPARE(linkedList, expectedLinkedList); \ - } \ - { \ - Vector expectedVector; \ - for (int i = 0; i < expectedList.count(); ++i) \ - expectedVector << expectedList.at(i); \ - QByteArray ba = byteArray; \ - QDataStream stream(&ba, QIODevice::ReadOnly); \ - stream.setStatus(initialStatus); \ - stream >> vector; \ - QCOMPARE((int)stream.status(), (int)expectedStatus); \ - QCOMPARE(vector.size(), expectedVector.size()); \ - QCOMPARE(vector, expectedVector); \ + for (bool inTransaction = false;; inTransaction = true) { \ + { \ + QByteArray ba = byteArray; \ + QDataStream stream(&ba, QIODevice::ReadOnly); \ + if (inTransaction) \ + stream.startTransaction(); \ + stream.setStatus(initialStatus); \ + stream >> list; \ + QCOMPARE((int)stream.status(), (int)expectedStatus); \ + if (!inTransaction || stream.commitTransaction()) { \ + QCOMPARE(list.size(), expectedList.size()); \ + QCOMPARE(list, expectedList); \ + } else { \ + QVERIFY(list.isEmpty()); \ + } \ + } \ + { \ + LinkedList expectedLinkedList; \ + for (int i = 0; i < expectedList.count(); ++i) \ + expectedLinkedList << expectedList.at(i); \ + QByteArray ba = byteArray; \ + QDataStream stream(&ba, QIODevice::ReadOnly); \ + if (inTransaction) \ + stream.startTransaction(); \ + stream.setStatus(initialStatus); \ + stream >> linkedList; \ + QCOMPARE((int)stream.status(), (int)expectedStatus); \ + if (!inTransaction || stream.commitTransaction()) { \ + QCOMPARE(linkedList.size(), expectedLinkedList.size()); \ + QCOMPARE(linkedList, expectedLinkedList); \ + } else { \ + QVERIFY(linkedList.isEmpty()); \ + } \ + } \ + { \ + Vector expectedVector; \ + for (int i = 0; i < expectedList.count(); ++i) \ + expectedVector << expectedList.at(i); \ + QByteArray ba = byteArray; \ + QDataStream stream(&ba, QIODevice::ReadOnly); \ + if (inTransaction) \ + stream.startTransaction(); \ + stream.setStatus(initialStatus); \ + stream >> vector; \ + QCOMPARE((int)stream.status(), (int)expectedStatus); \ + if (!inTransaction || stream.commitTransaction()) { \ + QCOMPARE(vector.size(), expectedVector.size()); \ + QCOMPARE(vector, expectedVector); \ + } else { \ + QVERIFY(vector.isEmpty()); \ + } \ + } \ + if (inTransaction) \ + break; \ } void tst_QDataStream::status_QLinkedList_QList_QVector()