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 <thiago.macieira@intel.com>

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 <errno.h>
+#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

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;
 

src/corelib/io/qsavefile_p.h

@@ -68,6 +68,9 @@ protected:
     QString fileName;
 
     QFileDevice::FileError writeError;
+
+    bool useTemporaryFile;
+    bool directWriteFallback;
 };
 
 QT_END_NAMESPACE

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<bool>("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
 }