From 22c19f95d3cf0b223cac4a0fb74a903677e48188 Mon Sep 17 00:00:00 2001 From: Giuseppe D'Angelo Date: Thu, 12 May 2022 16:59:50 +0200 Subject: [PATCH] QtWidgets: restore Qt 5 compatibility for save/restore state Several classes in QWidget use QDataStream internally in order to save and restore state. These QDataStream usages were not versioned, meaning that if Qt changes the serialization for some datatype, then the data saved between different Qt versions becomes incompatible. Note that the save/restore API in question just produce opaque blobs as QByteArrays -- the user has no control over the QDataStream objects and thus versions. Fix by version the usages. In QHeaderView this has caused a regression because QBitArray *did* change version between Qt 5 and 6. In general, using QDataStream without explicit versioning is a mistake, so deploy the same fix elsewhere as well. Fixes: QTBUG-99487 Change-Id: I82bb5c266f4e5dedc0887cbef855dccab1015e29 Reviewed-by: Volker Hilsheimer Reviewed-by: (cherry picked from commit 07d80deeab64db9e10364a162f7d2b7bf9f8bb93) Reviewed-by: Qt CI Bot --- src/widgets/dialogs/qfiledialog.cpp | 2 + src/widgets/itemviews/qheaderview.cpp | 2 + src/widgets/widgets/qmainwindow.cpp | 2 + src/widgets/widgets/qmainwindowlayout.cpp | 2 + src/widgets/widgets/qsplitter.cpp | 2 + .../itemviews/qheaderview/tst_qheaderview.cpp | 37 ++++++++++++++++--- 6 files changed, 42 insertions(+), 5 deletions(-) diff --git a/src/widgets/dialogs/qfiledialog.cpp b/src/widgets/dialogs/qfiledialog.cpp index 38757cd2bb8..abed694cf6e 100644 --- a/src/widgets/dialogs/qfiledialog.cpp +++ b/src/widgets/dialogs/qfiledialog.cpp @@ -452,6 +452,7 @@ QByteArray QFileDialog::saveState() const int version = 4; QByteArray data; QDataStream stream(&data, QIODevice::WriteOnly); + stream.setVersion(QDataStream::Qt_5_0); stream << qint32(QFileDialogMagic); stream << qint32(version); @@ -486,6 +487,7 @@ bool QFileDialog::restoreState(const QByteArray &state) Q_D(QFileDialog); QByteArray sd = state; QDataStream stream(&sd, QIODevice::ReadOnly); + stream.setVersion(QDataStream::Qt_5_0); if (stream.atEnd()) return false; QStringList history; diff --git a/src/widgets/itemviews/qheaderview.cpp b/src/widgets/itemviews/qheaderview.cpp index 8fc0df84e21..934aa43fa7d 100644 --- a/src/widgets/itemviews/qheaderview.cpp +++ b/src/widgets/itemviews/qheaderview.cpp @@ -1778,6 +1778,7 @@ QByteArray QHeaderView::saveState() const Q_D(const QHeaderView); QByteArray data; QDataStream stream(&data, QIODevice::WriteOnly); + stream.setVersion(QDataStream::Qt_5_0); stream << QHeaderViewPrivate::VersionMarker; stream << 0; // current version is 0 d->write(stream); @@ -1799,6 +1800,7 @@ bool QHeaderView::restoreState(const QByteArray &state) return false; QByteArray data = state; QDataStream stream(&data, QIODevice::ReadOnly); + stream.setVersion(QDataStream::Qt_5_0); int marker; int ver; stream >> marker; diff --git a/src/widgets/widgets/qmainwindow.cpp b/src/widgets/widgets/qmainwindow.cpp index cadb5a562f3..8736104246c 100644 --- a/src/widgets/widgets/qmainwindow.cpp +++ b/src/widgets/widgets/qmainwindow.cpp @@ -1256,6 +1256,7 @@ QByteArray QMainWindow::saveState(int version) const { QByteArray data; QDataStream stream(&data, QIODevice::WriteOnly); + stream.setVersion(QDataStream::Qt_5_0); stream << QMainWindowLayout::VersionMarker; stream << version; d_func()->layout->saveState(stream); @@ -1284,6 +1285,7 @@ bool QMainWindow::restoreState(const QByteArray &state, int version) return false; QByteArray sd = state; QDataStream stream(&sd, QIODevice::ReadOnly); + stream.setVersion(QDataStream::Qt_5_0); int marker, v; stream >> marker; stream >> v; diff --git a/src/widgets/widgets/qmainwindowlayout.cpp b/src/widgets/widgets/qmainwindowlayout.cpp index 06a252bb95f..e7bccc6b98f 100644 --- a/src/widgets/widgets/qmainwindowlayout.cpp +++ b/src/widgets/widgets/qmainwindowlayout.cpp @@ -1198,10 +1198,12 @@ bool QMainWindowLayoutState::restoreState(QDataStream &_stream, } QDataStream ds(copy); + ds.setVersion(_stream.version()); if (!checkFormat(ds)) return false; QDataStream stream(copy); + stream.setVersion(_stream.version()); while (!stream.atEnd()) { uchar marker; diff --git a/src/widgets/widgets/qsplitter.cpp b/src/widgets/widgets/qsplitter.cpp index be5cd7bf970..b7a0ddb356c 100644 --- a/src/widgets/widgets/qsplitter.cpp +++ b/src/widgets/widgets/qsplitter.cpp @@ -1690,6 +1690,7 @@ QByteArray QSplitter::saveState() const int version = 1; QByteArray data; QDataStream stream(&data, QIODevice::WriteOnly); + stream.setVersion(QDataStream::Qt_5_0); stream << qint32(SplitterMagic); stream << qint32(version); @@ -1731,6 +1732,7 @@ bool QSplitter::restoreState(const QByteArray &state) int version = 1; QByteArray sd = state; QDataStream stream(&sd, QIODevice::ReadOnly); + stream.setVersion(QDataStream::Qt_5_0); QList list; bool b; qint32 i; diff --git a/tests/auto/widgets/itemviews/qheaderview/tst_qheaderview.cpp b/tests/auto/widgets/itemviews/qheaderview/tst_qheaderview.cpp index 4e857c76260..830af60cea4 100644 --- a/tests/auto/widgets/itemviews/qheaderview/tst_qheaderview.cpp +++ b/tests/auto/widgets/itemviews/qheaderview/tst_qheaderview.cpp @@ -172,6 +172,7 @@ private slots: void moveSectionAndRemove(); void saveRestore(); void restoreQt4State(); + void QTBUG99487_saveRestoreQt5Compat(); void restoreToMoreColumns(); void restoreToMoreColumnsNoMovedColumns(); void restoreBeforeSetModel(); @@ -1742,16 +1743,24 @@ static QByteArray savedState() return h1.saveState(); } -void tst_QHeaderView::saveRestore() +// As generated by savedState() +static const QByteArray qt5SavedSate = QByteArrayLiteral("\x00\x00\x00\xFF\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x01\x00\x00\x00\x02\x01\x00\x00\x00\x04\x00\x00\x00\x02\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00\x04\x00\x00\x00\x02\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00\x04\b\x00\x00\x00\x01\x00\x00\x00\x03\x00\x00\x00""d\x00\x00\x00\xD2\x00\x00\x00\x04\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00""d\x00\x00\x00\x00\x00\x00\x00\x84\x00\x00\x00\x00\x00\x00\x00\x04\x00\x00\x00""d\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\n\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00""d\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x03\xE8\x00\x00\x00\x00\x00\x00\x00\x00\x00"); + +enum class SaveRestoreOption +{ + CheckGeneratedState, + DoNotCheckGeneratedState, +}; + +static void saveRestoreImpl(const QByteArray &state, SaveRestoreOption option) { QStandardItemModel m(4, 4); - const QByteArray s1 = savedState(); QHeaderView h2(Qt::Vertical); QSignalSpy spy(&h2, &QHeaderView::sortIndicatorChanged); h2.setModel(&m); - QVERIFY(h2.restoreState(s1)); + QVERIFY(h2.restoreState(state)); QCOMPARE(spy.count(), 1); QCOMPARE(spy.at(0).at(0).toInt(), 2); @@ -1766,12 +1775,30 @@ void tst_QHeaderView::saveRestore() QVERIFY(h2.isSectionHidden(3)); QCOMPARE(h2.hiddenSectionCount(), 1); - QByteArray s2 = h2.saveState(); - QCOMPARE(s1, s2); + switch (option) { + case SaveRestoreOption::CheckGeneratedState: + { + QByteArray s2 = h2.saveState(); + QCOMPARE(state, s2); + break; + } + case SaveRestoreOption::DoNotCheckGeneratedState: + break; + }; QVERIFY(!h2.restoreState(QByteArrayLiteral("Garbage"))); } +void tst_QHeaderView::saveRestore() +{ + saveRestoreImpl(savedState(), SaveRestoreOption::CheckGeneratedState); +} + +void tst_QHeaderView::QTBUG99487_saveRestoreQt5Compat() +{ + saveRestoreImpl(qt5SavedSate, SaveRestoreOption::DoNotCheckGeneratedState); +} + void tst_QHeaderView::restoreQt4State() { #if QT_VERSION < QT_VERSION_CHECK(6, 0, 0)