QSaveFile: Fix permissions on creation

QSaveFile is intended to be a replacement for QFile, and should use the
same permissions for newly created files. QTemporaryFile however creates
new files with 0600 mask by default.

Fix this by making the mode_t argument QTemporaryFileEngine uses
configurable, and using 0666 for QSaveFile (like we do in QFile).

[ChangeLog][Important behavior changes] Files created by QSaveFile do
now have the same rights as files created by QFile. This also fixes a
regression in QSettings: In the Qt 5.4 series, new files created by
QSettings were only readable by the current user.

Task-number: QTBUG-44086
Change-Id: Ie1cc20e9f25c6e72e1bc9176490c419c27c5fc82
Reviewed-by: Leena Miettinen <riitta-leena.miettinen@theqtcompany.com>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>

4 files changed, 25 insertions, 8 deletions

diff --git a/src/corelib/io/qsavefile.cpp b/src/corelib/io/qsavefile.cpp
index 33be537433..ffa6ad1c55 100644
--- a/src/corelib/io/qsavefile.cpp
+++ b/src/corelib/io/qsavefile.cpp
@@ -215,7 +215,7 @@ bool QSaveFile::open(OpenMode mode)
             d->finalFileName = existingFile.filePath();
     }
 
-    d->fileEngine = new QTemporaryFileEngine(d->finalFileName);
+    d->fileEngine = new QTemporaryFileEngine(d->finalFileName, 0666);
     // 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();
diff --git a/src/corelib/io/qtemporaryfile.cpp b/src/corelib/io/qtemporaryfile.cpp
index fa851a6d74..556bc6e760 100644
--- a/src/corelib/io/qtemporaryfile.cpp
+++ b/src/corelib/io/qtemporaryfile.cpp
@@ -103,13 +103,14 @@ typedef int NativeFileHandle;
     \a path is used as a template when generating unique paths, \a pos
     identifies the position of the first character that will be replaced in the
     template and \a length the number of characters that may be substituted.
+    \a mode specifies the file mode bits (not used on Windows).
 
     Returns an open handle to the newly created file if successful, an invalid
     handle otherwise. In both cases, the string in \a path will be changed and
     contain the generated path name.
 */
 static bool createFileFromTemplate(NativeFileHandle &file,
-        QFileSystemEntry::NativePath &path, size_t pos, size_t length,
+        QFileSystemEntry::NativePath &path, size_t pos, size_t length, quint32 mode,
         QSystemError &error)
 {
     Q_ASSERT(length != 0);
@@ -143,6 +144,8 @@ static bool createFileFromTemplate(NativeFileHandle &file,
     for (;;) {
         // Atomically create file and obtain handle
 #if defined(Q_OS_WIN)
+        Q_UNUSED(mode);
+
 #  ifndef Q_OS_WINRT
         file = CreateFile((const wchar_t *)path.constData(),
                 GENERIC_READ | GENERIC_WRITE,
@@ -175,7 +178,7 @@ static bool createFileFromTemplate(NativeFileHandle &file,
 #else // POSIX
         file = QT_OPEN(path.constData(),
                 QT_OPEN_CREAT | O_EXCL | QT_OPEN_RDWR | QT_OPEN_LARGEFILE,
-                0600);
+                static_cast<mode_t>(mode));
 
         if (file != -1)
             return true;
@@ -333,7 +336,7 @@ bool QTemporaryFileEngine::open(QIODevice::OpenMode openMode)
     NativeFileHandle &file = d->fd;
 #endif
 
-    if (!createFileFromTemplate(file, filename, phPos, phLength, error)) {
+    if (!createFileFromTemplate(file, filename, phPos, phLength, fileMode, error)) {
         setError(QFile::OpenError, error.toString());
         return false;
     }
@@ -407,9 +410,9 @@ QAbstractFileEngine *QTemporaryFilePrivate::engine() const
 {
     if (!fileEngine) {
         if (fileName.isEmpty())
-            fileEngine = new QTemporaryFileEngine(templateName);
+            fileEngine = new QTemporaryFileEngine(templateName, 0600);
         else
-            fileEngine = new QTemporaryFileEngine(fileName, false);
+            fileEngine = new QTemporaryFileEngine(fileName, 0600, false);
     }
     return fileEngine;
 }
diff --git a/src/corelib/io/qtemporaryfile_p.h b/src/corelib/io/qtemporaryfile_p.h
index 2045edff02..475298f264 100644
--- a/src/corelib/io/qtemporaryfile_p.h
+++ b/src/corelib/io/qtemporaryfile_p.h
@@ -77,8 +77,13 @@ class QTemporaryFileEngine : public QFSFileEngine
 {
     Q_DECLARE_PRIVATE(QFSFileEngine)
 public:
-    QTemporaryFileEngine(const QString &file, bool fileIsTemplate = true)
-        : QFSFileEngine(), filePathIsTemplate(fileIsTemplate),
+
+    QTemporaryFileEngine(const QString &file,
+                         quint32 fileMode,
+                         bool fileIsTemplate = true) :
+        QFSFileEngine(),
+        fileMode(fileMode),
+        filePathIsTemplate(fileIsTemplate),
         filePathWasTemplate(fileIsTemplate)
     {
         Q_D(QFSFileEngine);
@@ -100,6 +105,7 @@ public:
     bool renameOverwrite(const QString &newName);
     bool close();
 
+    quint32 fileMode;
     bool filePathIsTemplate;
     bool filePathWasTemplate;
 };
diff --git a/tests/auto/corelib/io/qsavefile/tst_qsavefile.cpp b/tests/auto/corelib/io/qsavefile/tst_qsavefile.cpp
index 2c86f0f19c..5796636b92 100644
--- a/tests/auto/corelib/io/qsavefile/tst_qsavefile.cpp
+++ b/tests/auto/corelib/io/qsavefile/tst_qsavefile.cpp
@@ -119,6 +119,14 @@ void tst_QSaveFile::transactionalWrite()
     QFile reader(targetFile);
     QVERIFY(reader.open(QIODevice::ReadOnly));
     QCOMPARE(QString::fromLatin1(reader.readAll()), QString::fromLatin1("Hello"));
+
+    // check that permissions are the same as for QFile
+    const QString otherFile = dir.path() + QString::fromLatin1("/otherfile");
+    QFile::remove(otherFile);
+    QFile other(otherFile);
+    other.open(QIODevice::WriteOnly);
+    other.close();
+    QCOMPARE(QFile::permissions(targetFile), QFile::permissions(otherFile));
 }
 
 void tst_QSaveFile::saveTwice()