From 4934138be29868bdf848e2aeb6c6163819ab3a5c Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Wed, 26 Jul 2017 12:17:20 -0700 Subject: [PATCH] Allow QSettings to synchronize non-atomically MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is required so that one can use QSettings in situations that temporary files or renaming may not work. [ChangeLog][QtCore][QSettings] Added setAtomicSyncRequired(), which allows one to use QSettings with config files in unwriteable directories or in Alternate Data Streams on NTFS on Windows. This used to work before Qt 5.4, but remains a non-default behavior due to the potential of data corruption. Task-number: QTBUG-47379 Change-Id: I81480fdb578d4d43b3fcfffd14d4f77112f0402f Reviewed-by: Jesus Fernandez Reviewed-by: Mårten Nordheim Reviewed-by: Oswald Buddenhagen Reviewed-by: Friedemann Kleint Reviewed-by: Kai Koehne Reviewed-by: Leena Miettinen --- src/corelib/io/qsettings.cpp | 78 +++++++++++++++---- src/corelib/io/qsettings.h | 2 + src/corelib/io/qsettings_p.h | 1 + .../corelib/io/qsettings/tst_qsettings.cpp | 76 ++++++++++++++++++ 4 files changed, 143 insertions(+), 14 deletions(-) diff --git a/src/corelib/io/qsettings.cpp b/src/corelib/io/qsettings.cpp index 850e729987d..d0cbc82c560 100644 --- a/src/corelib/io/qsettings.cpp +++ b/src/corelib/io/qsettings.cpp @@ -1405,6 +1405,11 @@ void QConfFileSettingsPrivate::syncConfFile(QConfFile *confFile) return; } + if (!readOnly && !confFile->isWritable()) { + setStatus(QSettings::AccessError); + return; + } + #ifndef QT_BOOTSTRAPPED /* Use a lockfile in order to protect us against other QSettings instances @@ -1414,17 +1419,11 @@ void QConfFileSettingsPrivate::syncConfFile(QConfFile *confFile) Concurrent read and write are not a problem because the writing operation is atomic. */ QLockFile lockFile(confFile->name + QLatin1String(".lock")); -#endif - if (!readOnly) { - if (!confFile->isWritable() -#ifndef QT_BOOTSTRAPPED - || !lockFile.lock() -#endif - ) { - setStatus(QSettings::AccessError); - return; - } + if (!readOnly && !lockFile.lock() && atomicSyncOnly) { + setStatus(QSettings::AccessError); + return; } +#endif /* We hold the lock. Let's reread the file if it has changed @@ -1496,6 +1495,7 @@ void QConfFileSettingsPrivate::syncConfFile(QConfFile *confFile) #if !defined(QT_BOOTSTRAPPED) && QT_CONFIG(temporaryfile) QSaveFile sf(confFile->name); + sf.setDirectWriteFallback(!atomicSyncOnly); #else QFile sf(confFile->name); #endif @@ -2201,10 +2201,16 @@ void QConfFileSettingsPrivate::ensureSectionParsed(QConfFile *confFile, QSettings can safely be used from different processes (which can be different instances of your application running at the same time or different applications altogether) to read and write to - the same system locations. It uses advisory file locking and a - smart merging algorithm to ensure data integrity. Note that sync() - imports changes made by other processes (in addition to writing - the changes from this QSettings). + the same system locations, provided certain conditions are met. For + QSettings::IniFormat, it uses advisory file locking and a smart merging + algorithm to ensure data integrity. The condition for that to work is that + the writeable configuration file must be a regular file and must reside in + a directory that the current user can create new, temporary files in. If + that is not the case, then one must use setAtomicSyncRequired() to turn the + safety off. + + Note that sync() imports changes made by other processes (in addition to + writing the changes from this QSettings). \section1 Platform-Specific Notes @@ -2884,6 +2890,50 @@ QSettings::Status QSettings::status() const return d->status; } +/*! + \since 5.10 + + Returns \c true if QSettings is only allowed to perform atomic saving and + reloading (synchronization) of the settings. Returns \c false if it is + allowed to save the settings contents directly to the configuration file. + + The default is \c true. + + \sa setAtomicSyncRequired(), QSaveFile +*/ +bool QSettings::isAtomicSyncRequired() const +{ + Q_D(const QSettings); + return d->atomicSyncOnly; +} + +/*! + \since 5.10 + + Configures whether QSettings is required to perform atomic saving and + reloading (synchronization) of the settings. If the \a enable argument is + \c true (the default), sync() will only perform synchronization operations + that are atomic. If this is not possible, sync() will fail and status() + will be an error condition. + + Setting this property to \c false will allow QSettings to write directly to + the configuration file and ignore any errors trying to lock it against + other processes trying to write at the same time. Because of the potential + for corruption, this option should be used with care, but is required in + certain conditions, like a QSettings::IniFormat configuration file that + exists in an otherwise non-writeable directory or NTFS Alternate Data + Streams. + + See \l QSaveFile for more information on the feature. + + \sa isAtomicSyncRequired(), QSaveFile +*/ +void QSettings::setAtomicSyncRequired(bool enable) +{ + Q_D(QSettings); + d->atomicSyncOnly = enable; +} + /*! Appends \a prefix to the current group. diff --git a/src/corelib/io/qsettings.h b/src/corelib/io/qsettings.h index edd59026ed1..da5502e5ca1 100644 --- a/src/corelib/io/qsettings.h +++ b/src/corelib/io/qsettings.h @@ -146,6 +146,8 @@ public: void clear(); void sync(); Status status() const; + bool isAtomicSyncRequired() const; + void setAtomicSyncRequired(bool enable); void beginGroup(const QString &prefix); void endGroup(); diff --git a/src/corelib/io/qsettings_p.h b/src/corelib/io/qsettings_p.h index d8e91e48ce5..7923c24770c 100644 --- a/src/corelib/io/qsettings_p.h +++ b/src/corelib/io/qsettings_p.h @@ -249,6 +249,7 @@ protected: QString groupPrefix; bool fallbacks; bool pendingChanges; + bool atomicSyncOnly = true; mutable QSettings::Status status; }; diff --git a/tests/auto/corelib/io/qsettings/tst_qsettings.cpp b/tests/auto/corelib/io/qsettings/tst_qsettings.cpp index 332c2dcc014..012ce5f2f59 100644 --- a/tests/auto/corelib/io/qsettings/tst_qsettings.cpp +++ b/tests/auto/corelib/io/qsettings/tst_qsettings.cpp @@ -118,6 +118,10 @@ private slots: void remove(); void contains(); void sync(); + void syncNonWriteableDir(); +#ifdef Q_OS_WIN + void syncAlternateDataStream(); +#endif void setFallbacksEnabled(); void setFallbacksEnabled_data(); void fromFile_data(); @@ -1750,6 +1754,78 @@ void tst_QSettings::sync() QCOMPARE(settings1.allKeys().count(), 11); } +void tst_QSettings::syncNonWriteableDir() +{ + QTemporaryDir tempDir; + QVERIFY2(tempDir.isValid(), qUtf8Printable(tempDir.errorString())); + + // first, create a file + QString filename = tempDir.path() + "/config.ini"; + { + QFile f(filename); + QVERIFY2(f.open(QIODevice::WriteOnly), qUtf8Printable(f.errorString())); + } + + // second, make the dir unwriteable + QVERIFY(QFile::setPermissions(tempDir.path(), QFile::ReadUser | QFile::ExeUser)); + struct UndoSetPermissions { + QString name; + UndoSetPermissions(const QString &name) : name(name) {} + ~UndoSetPermissions() + { QFile::setPermissions(name, QFile::ReadUser | QFile::WriteUser | QFile::ExeUser); } + }; + UndoSetPermissions undo(tempDir.path()); // otherwise, QTemporaryDir will fail + + { + QSettings settings(filename, QSettings::IniFormat); + QVERIFY(settings.isAtomicSyncRequired()); + settings.setAtomicSyncRequired(false); + settings.setValue("alpha/beta", 1); + settings.sync(); + QCOMPARE(settings.status(), QSettings::NoError); + } + + QVERIFY(QFileInfo(filename).size() != 0); + QSettings settings(filename, QSettings::IniFormat); + QCOMPARE(settings.value("alpha/beta"), QVariant(1)); +} + +#ifdef Q_OS_WIN +void tst_QSettings::syncAlternateDataStream() +{ + QTemporaryDir tempDir; + QVERIFY2(tempDir.isValid(), qUtf8Printable(tempDir.errorString())); + + // first, create a file + QString filename = tempDir.path() + "/file"; + { + QFile f(filename); + QVERIFY2(f.open(QIODevice::WriteOnly), qUtf8Printable(f.errorString())); + } + + // then create an ADS + filename += ":config.ini"; + { + QFile f(filename); + if (!f.open(QIODevice::WriteOnly)) + QSKIP("Could not create ADS file (" + f.errorString().toUtf8() + ") - FAT drive?"); + } + + { + QSettings settings(filename, QSettings::IniFormat); + QVERIFY(settings.isAtomicSyncRequired()); + settings.setAtomicSyncRequired(false); + settings.setValue("alpha/beta", 1); + settings.sync(); + QCOMPARE(settings.status(), QSettings::NoError); + } + + QVERIFY(QFileInfo(filename).size() != 0); + QSettings settings(filename, QSettings::IniFormat); + QCOMPARE(settings.value("alpha/beta"), QVariant(1)); +} +#endif + void tst_QSettings::setFallbacksEnabled_data() { populateWithFormats();