From b3a505dc924fb26fcf68bb2016b6f5ea206a946b Mon Sep 17 00:00:00 2001 From: David Faure Date: Sun, 24 Mar 2013 11:10:21 +0100 Subject: QSaveFile: allow saving to a writable file in a non-writable directory The only way to make this possible is to disable the atomic-rename-from-temp-file behavior. This is not done by default, but only if the application allows this to happen. https://bugs.kde.org/show_bug.cgi?id=312415 Change-Id: I71ce54ae1f7f50ab5e8379f04c0ede74ebe3136d Reviewed-by: Thiago Macieira --- src/corelib/io/qsavefile.cpp | 101 ++++++++++++++++++---- src/corelib/io/qsavefile.h | 3 + src/corelib/io/qsavefile_p.h | 3 + tests/auto/corelib/io/qsavefile/tst_qsavefile.cpp | 85 ++++++++++++++++-- 4 files changed, 166 insertions(+), 26 deletions(-) diff --git a/src/corelib/io/qsavefile.cpp b/src/corelib/io/qsavefile.cpp index fee6a4c4d8..f8b5ebcabd 100644 --- a/src/corelib/io/qsavefile.cpp +++ b/src/corelib/io/qsavefile.cpp @@ -48,11 +48,16 @@ #include "qtemporaryfile.h" #include "private/qiodevice_p.h" #include "private/qtemporaryfile_p.h" +#ifdef Q_OS_UNIX +#include +#endif QT_BEGIN_NAMESPACE QSaveFilePrivate::QSaveFilePrivate() - : writeError(QFileDevice::NoError) + : writeError(QFileDevice::NoError), + useTemporaryFile(true), + directWriteFallback(false) { } @@ -201,6 +206,18 @@ bool QSaveFile::open(OpenMode mode) // Same as in QFile: QIODevice provides the buffering, so there's no need to request it from the file engine. if (!d->fileEngine->open(mode | QIODevice::Unbuffered)) { QFileDevice::FileError err = d->fileEngine->error(); +#ifdef Q_OS_UNIX + if (d->directWriteFallback && err == QFileDevice::OpenError && errno == EACCES) { + delete d->fileEngine; + d->fileEngine = QAbstractFileEngine::create(d->fileName); + if (d->fileEngine->open(mode | QIODevice::Unbuffered)) { + d->useTemporaryFile = false; + QFileDevice::open(mode); + return true; + } + err = d->fileEngine->error(); + } +#endif if (err == QFileDevice::UnspecifiedError) err = QFileDevice::OpenError; d->setError(err, d->fileEngine->errorString()); @@ -209,6 +226,7 @@ bool QSaveFile::open(OpenMode mode) return false; } + d->useTemporaryFile = true; QFileDevice::open(mode); if (existingFile.exists()) setPermissions(existingFile.permissions()); @@ -253,22 +271,24 @@ bool QSaveFile::commit() // Sync to disk if possible. Ignore errors (e.g. not supported). d->fileEngine->syncToDisk(); - if (d->writeError != QFileDevice::NoError) { - d->fileEngine->remove(); - d->writeError = QFileDevice::NoError; - delete d->fileEngine; - d->fileEngine = 0; - return false; - } - // atomically replace old file with new file - // Can't use QFile::rename for that, must use the file engine directly - Q_ASSERT(d->fileEngine); - if (!d->fileEngine->renameOverwrite(d->fileName)) { - d->setError(d->fileEngine->error(), d->fileEngine->errorString()); - d->fileEngine->remove(); - delete d->fileEngine; - d->fileEngine = 0; - return false; + if (d->useTemporaryFile) { + if (d->writeError != QFileDevice::NoError) { + d->fileEngine->remove(); + d->writeError = QFileDevice::NoError; + delete d->fileEngine; + d->fileEngine = 0; + return false; + } + // atomically replace old file with new file + // Can't use QFile::rename for that, must use the file engine directly + Q_ASSERT(d->fileEngine); + if (!d->fileEngine->renameOverwrite(d->fileName)) { + d->setError(d->fileEngine->error(), d->fileEngine->errorString()); + d->fileEngine->remove(); + delete d->fileEngine; + d->fileEngine = 0; + return false; + } } delete d->fileEngine; d->fileEngine = 0; @@ -286,6 +306,11 @@ bool QSaveFile::commit() Further write operations are possible after calling this method, but none of it will have any effect, the written file will be discarded. + This method has no effect when direct write fallback is used. This is the case + when saving over an existing file in a readonly directory: no temporary file can + be created, so the existing file is overwritten no matter what, and cancelWriting() + cannot do anything about that, the contents of the existing file will be lost. + \sa commit() */ void QSaveFile::cancelWriting() @@ -313,4 +338,46 @@ qint64 QSaveFile::writeData(const char *data, qint64 len) return ret; } +/*! + Allows writing over the existing file if necessary. + + QSaveFile creates a temporary file in the same directory as the final + file and atomically renames it. However this is not possible if the + directory permissions do not allow creating new files. + In order to preserve atomicity guarantees, open() fails when it + cannot create the temporary file. + + In order to allow users to edit files with write permissions in a + directory with restricted permissions, call setDirectWriteFallback() with + \a enabled set to true, and the following calls to open() will fallback to + opening the existing file directly and writing into it, without the use of + a temporary file. + This does not have atomicity guarantees, i.e. an application crash or + for instance a power failure could lead to a partially-written file on disk. + It also means cancelWriting() has no effect, in such a case. + + Typically, to save documents edited by the user, call setDirectWriteFallback(true), + and to save application internal files (configuration files, data files, ...), keep + the default setting which ensures atomicity. + + \sa directWriteFallback() +*/ +void QSaveFile::setDirectWriteFallback(bool enabled) +{ + Q_D(QSaveFile); + d->directWriteFallback = enabled; +} + +/*! + Returns true if the fallback solution for saving files in read-only + directories is enabled. + + \sa setDirectWriteFallback() +*/ +bool QSaveFile::directWriteFallback() const +{ + Q_D(const QSaveFile); + return d->directWriteFallback; +} + QT_END_NAMESPACE diff --git a/src/corelib/io/qsavefile.h b/src/corelib/io/qsavefile.h index 32af4a708e..6d81f58d42 100644 --- a/src/corelib/io/qsavefile.h +++ b/src/corelib/io/qsavefile.h @@ -75,6 +75,9 @@ public: void cancelWriting(); + void setDirectWriteFallback(bool enabled); + bool directWriteFallback() const; + protected: qint64 writeData(const char *data, qint64 len) Q_DECL_OVERRIDE; diff --git a/src/corelib/io/qsavefile_p.h b/src/corelib/io/qsavefile_p.h index 27e687835b..53a8b5eb34 100644 --- a/src/corelib/io/qsavefile_p.h +++ b/src/corelib/io/qsavefile_p.h @@ -68,6 +68,9 @@ protected: QString fileName; QFileDevice::FileError writeError; + + bool useTemporaryFile; + bool directWriteFallback; }; QT_END_NAMESPACE diff --git a/tests/auto/corelib/io/qsavefile/tst_qsavefile.cpp b/tests/auto/corelib/io/qsavefile/tst_qsavefile.cpp index 5ef4b11e8a..d9292b8460 100644 --- a/tests/auto/corelib/io/qsavefile/tst_qsavefile.cpp +++ b/tests/auto/corelib/io/qsavefile/tst_qsavefile.cpp @@ -66,6 +66,7 @@ private slots: void textStreamManualFlush(); void textStreamAutoFlush(); void saveTwice(); + void transactionalWriteNoPermissionsOnDir_data(); void transactionalWriteNoPermissionsOnDir(); void transactionalWriteNoPermissionsOnFile(); void transactionalWriteCanceled(); @@ -153,20 +154,86 @@ void tst_QSaveFile::textStreamAutoFlush() QFile::remove(targetFile); } +void tst_QSaveFile::transactionalWriteNoPermissionsOnDir_data() +{ + QTest::addColumn("directWriteFallback"); + + QTest::newRow("default") << false; + QTest::newRow("directWriteFallback") << true; +} + void tst_QSaveFile::transactionalWriteNoPermissionsOnDir() { #ifdef Q_OS_UNIX - if (::geteuid() == 0) - QSKIP("not valid running this test as root"); + QFETCH(bool, directWriteFallback); + // Restore permissions so that the QTemporaryDir cleanup can happen + class PermissionRestorer + { + QString m_path; + public: + PermissionRestorer(const QString& path) + : m_path(path) + {} - // You can write into /dev/zero, but you can't create a /dev/zero.XXXXXX temp file. - QSaveFile file("/dev/zero"); - if (!QDir("/dev").exists()) - QSKIP("/dev doesn't exist on this system"); + ~PermissionRestorer() + { + restore(); + } + void restore() + { + QFile file(m_path); + file.setPermissions(QFile::Permissions(QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOwner)); + } + }; - QVERIFY(!file.open(QIODevice::WriteOnly)); - QCOMPARE((int)file.error(), (int)QFile::OpenError); - QVERIFY(!file.commit()); + + QTemporaryDir dir; + QVERIFY(QFile(dir.path()).setPermissions(QFile::ReadOwner | QFile::ExeOwner)); + PermissionRestorer permissionRestorer(dir.path()); + + const QString targetFile = dir.path() + QString::fromLatin1("/outfile"); + QSaveFile firstTry(targetFile); + QVERIFY(!firstTry.open(QIODevice::WriteOnly)); + QCOMPARE((int)firstTry.error(), (int)QFile::OpenError); + QVERIFY(!firstTry.commit()); + + // Now make an existing writable file + permissionRestorer.restore(); + QFile f(targetFile); + QVERIFY(f.open(QIODevice::WriteOnly)); + QCOMPARE(f.write("Hello"), Q_INT64_C(5)); + f.close(); + + // Make the directory non-writable again + QVERIFY(QFile(dir.path()).setPermissions(QFile::ReadOwner | QFile::ExeOwner)); + + // And write to it again using QSaveFile; only works if directWriteFallback is enabled + QSaveFile file(targetFile); + file.setDirectWriteFallback(directWriteFallback); + QCOMPARE(file.directWriteFallback(), directWriteFallback); + if (directWriteFallback) { + QVERIFY(file.open(QIODevice::WriteOnly)); + QCOMPARE((int)file.error(), (int)QFile::NoError); + QCOMPARE(file.write("World"), Q_INT64_C(5)); + QVERIFY(file.commit()); + + QFile reader(targetFile); + QVERIFY(reader.open(QIODevice::ReadOnly)); + QCOMPARE(QString::fromLatin1(reader.readAll()), QString::fromLatin1("World")); + reader.close(); + + QVERIFY(file.open(QIODevice::WriteOnly)); + QCOMPARE((int)file.error(), (int)QFile::NoError); + QCOMPARE(file.write("W"), Q_INT64_C(1)); + file.cancelWriting(); // no effect, as per the documentation + QVERIFY(file.commit()); + + QVERIFY(reader.open(QIODevice::ReadOnly)); + QCOMPARE(QString::fromLatin1(reader.readAll()), QString::fromLatin1("W")); + } else { + QVERIFY(!file.open(QIODevice::WriteOnly)); + QCOMPARE((int)file.error(), (int)QFile::OpenError); + } #endif } -- cgit v1.2.3