Refactor setting and checking of file permissions

Create methods to explicitly set default file permissions on Unix
platforms, use these on installer created files and installation
target directory. Add unit test for introduced functions.

Remove method introduced in 46aecc23b2983c807ff2232ae9cb9651b4d2fdc2
as the same effect is achieved more efficiently by ensuring target
directory will be written with explicit permissions on initial
installation. Unlike the removed method, this will also not break if
an installed component contains owner non-writable directories.

Further simplify PackageManagerCore::directoryWritable() introduced
in 89f772f819178ee2502768c3d259d22ecb910fbe. Remove orphan unit
test for removed PackageManagerCore::subdirectoriesWritable().

This does not change permissions of files and directories extracted
for installed components.

Task-number: QTIFW-1412
Change-Id: I59698c78aceef874b1f79482bff5a618b9a1b536
Reviewed-by: Katja Marttila <katja.marttila@qt.io>

src/libs/installer/createdesktopentryoperation.cpp

@@ -137,8 +137,7 @@ bool CreateDesktopEntryOperation::performOperation()
         return false;
     }
 
-    QFile::setPermissions(filename, QFile::ReadOwner | QFile::WriteOwner | QFile::ReadUser | QFile::ReadGroup
-        | QFile::ReadOther | QFile::ExeOwner | QFile::ExeGroup | QFile::ExeOther);
+    setDefaultFilePermissions(filename, DefaultFilePermissions::Executable);
 
     QTextStream stream(&file);
     stream.setCodec("UTF-8");

src/libs/installer/createlocalrepositoryoperation.cpp

@@ -82,8 +82,7 @@ static void fixPermissions(const QString &repoPath)
         if (!it.fileInfo().isFile())
             continue;
 
-        if (!QFile::setPermissions(it.filePath(), QFile::ReadOwner | QFile::WriteOwner
-            | QFile::ReadUser | QFile::WriteUser | QFile::ReadGroup | QFile::ReadOther)) {
+        if (!setDefaultFilePermissions(it.filePath(), DefaultFilePermissions::NonExecutable)) {
                 throw Error(CreateLocalRepositoryOperation::tr("Cannot set permissions for file \"%1\".")
                     .arg(QDir::toNativeSeparators(it.filePath())));
         }

src/libs/installer/fileutils.cpp

@@ -293,6 +293,43 @@ void QInstaller::removeSystemGeneratedFiles(const QString &path)
 #endif
 }
 
+/*!
+    Sets permissions of file or directory specified by \a fileName to \c 644 or \c 755
+    based by the value of \a permissions.
+*/
+bool QInstaller::setDefaultFilePermissions(const QString &fileName, DefaultFilePermissions permissions)
+{
+    QFile file(fileName);
+    return setDefaultFilePermissions(&file, permissions);
+}
+
+/*!
+    Sets permissions of file or directory specified by \a file to \c 644 or \c 755
+    based by the value of \a permissions. This is effective only on Unix platforms
+    as \c setPermissions() does not manipulate ACLs. On Windows this does nothing
+    and always returns \c true.
+*/
+bool QInstaller::setDefaultFilePermissions(QFile *file, DefaultFilePermissions permissions)
+{
+#ifdef Q_OS_UNIX
+    if (!file->exists()) {
+        qWarning() << "Target" << file->fileName() << "does not exists.";
+        return false;
+    }
+    if (file->permissions() == static_cast<QFileDevice::Permission>(permissions))
+        return true;
+
+    if (!file->setPermissions(static_cast<QFileDevice::Permission>(permissions))) {
+        qWarning() << "Cannot set default permissions for target"
+                   << file->fileName() << ":" << file->errorString();
+        return false;
+    }
+    return true;
+#else
+    return true;
+#endif
+}
+
 void QInstaller::copyDirectoryContents(const QString &sourceDir, const QString &targetDir)
 {
     Q_ASSERT(QFileInfo(sourceDir).isDir());

src/libs/installer/fileutils.h

@@ -36,10 +36,17 @@
 
 QT_BEGIN_NAMESPACE
 class QFileInfo;
+class QFile;
 class QUrl;
 QT_END_NAMESPACE
 
 namespace QInstaller {
+
+enum DefaultFilePermissions {
+    NonExecutable = 0x6644,
+    Executable = 0x7755
+};
+
 class INSTALLER_EXPORT TempDirDeleter
 {
 public:
@@ -80,6 +87,9 @@ private:
     void INSTALLER_EXPORT removeDirectoryThreaded(const QString &path, bool ignoreErrors = false);
     void INSTALLER_EXPORT removeSystemGeneratedFiles(const QString &path);
 
+    bool INSTALLER_EXPORT setDefaultFilePermissions(const QString &fileName, DefaultFilePermissions permissions);
+    bool INSTALLER_EXPORT setDefaultFilePermissions(QFile *file, DefaultFilePermissions permissions);
+
     QString INSTALLER_EXPORT generateTemporaryFileName(const QString &templ=QString());
 
     void INSTALLER_EXPORT moveDirectoryContents(const QString &sourceDir, const QString &targetDir);

src/libs/installer/packagemanagercore.cpp

@@ -1585,16 +1585,15 @@ Component *PackageManagerCore::componentByName(const QString &name, const QList<
     return nullptr;
 }
 
+/*!
+    Returns \c true if directory specified by \a path is writable by
+    the current user.
+*/
 bool PackageManagerCore::directoryWritable(const QString &path) const
 {
     return d->directoryWritable(path);
 }
 
-bool PackageManagerCore::subdirectoriesWritable(const QString &path) const
-{
-    return d->subdirectoriesWritable(path);
-}
-
 /*!
     Returns a list of components that are marked for installation. The list can
     be empty.

src/libs/installer/packagemanagercore.h

@@ -122,7 +122,6 @@ public:
     static Component *componentByName(const QString &name, const QList<Component *> &components);
 
     bool directoryWritable(const QString &path) const;
-    bool subdirectoriesWritable(const QString &path) const;
 
     bool fetchLocalPackagesTree();
     LocalPackagesHash localInstalledPackages();

src/libs/installer/packagemanagercore_p.cpp

@@ -347,23 +347,8 @@ QString PackageManagerCorePrivate::targetDir() const
 
 bool PackageManagerCorePrivate::directoryWritable(const QString &path) const
 {
-    QTemporaryFile tempFile(path + QStringLiteral("/tempFile") + QString::number(qrand() % 1000));
-    if (!tempFile.open() || !tempFile.isWritable())
-        return false;
-    else
-        return true;
-}
-
-bool PackageManagerCorePrivate::subdirectoriesWritable(const QString &path) const
-{
-    // Iterate over target directory subdirectories for writing access
-    QDirIterator iterator(path, QDir::AllDirs | QDir::NoDotAndDotDot, QDirIterator::Subdirectories);
-    while (iterator.hasNext()) {
-        QTemporaryFile tempFile(iterator.next() + QLatin1String("/tempFile"));
-        if (!tempFile.open() || !tempFile.isWritable())
-            return false;
-    }
-    return true;
+    QTemporaryFile tempFile(path + QLatin1String("/tempFile.XXXXXX"));
+    return (tempFile.open() && tempFile.isWritable());
 }
 
 QString PackageManagerCorePrivate::configurationFileName() const
@@ -805,6 +790,7 @@ void PackageManagerCorePrivate::writeMaintenanceConfigFiles()
             : tr("Format error");
         throw Error(tr("Cannot write installer configuration to %1: %2").arg(iniPath, reason));
     }
+    setDefaultFilePermissions(iniPath, DefaultFilePermissions::NonExecutable);
 
     QFile file(targetDir() + QLatin1Char('/') + QLatin1String("network.xml"));
     if (file.open(QIODevice::WriteOnly | QIODevice::Truncate)) {
@@ -842,6 +828,7 @@ void PackageManagerCorePrivate::writeMaintenanceConfigFiles()
             writer.writeEndElement();
         writer.writeEndElement();
     }
+    setDefaultFilePermissions(&file, DefaultFilePermissions::NonExecutable);
 }
 
 void PackageManagerCorePrivate::readMaintenanceConfigFiles(const QString &targetDir)
@@ -1073,8 +1060,7 @@ void PackageManagerCorePrivate::writeMaintenanceToolBinary(QFile *const input, q
             throw Error(tr("Cannot write maintenance tool data to %1: %2").arg(dataOut.fileName(),
                 dataOut.errorString()));
         }
-        dataOut.setPermissions(dataOut.permissions() | QFile::WriteUser | QFile::ReadGroup
-            | QFile::ReadOther);
+        setDefaultFilePermissions(&dataOut, DefaultFilePermissions::NonExecutable);
     }
 
     {
@@ -1091,12 +1077,10 @@ void PackageManagerCorePrivate::writeMaintenanceToolBinary(QFile *const input, q
     }
 
     QFile mt(maintenanceToolRenamedName);
-    if (mt.setPermissions(out.permissions() | QFile::WriteUser | QFile::ReadGroup | QFile::ReadOther
-                          | QFile::ExeOther | QFile::ExeGroup | QFile::ExeUser)) {
+    if (setDefaultFilePermissions(&mt, DefaultFilePermissions::Executable))
         qDebug() << "Wrote permissions for maintenance tool.";
-    } else {
+    else
         qDebug() << "Failed to write permissions for maintenance tool.";
-    }
 
     if (out.exists() && !out.remove()) {
         qWarning() << tr("Cannot remove temporary data file \"%1\": %2")
@@ -1386,8 +1370,7 @@ void PackageManagerCorePrivate::writeMaintenanceTool(OperationList performedOper
                 throw Error(tr("Cannot write maintenance tool binary data to %1: %2")
                     .arg(file.fileName(), file.errorString()));
             }
-            file.setPermissions(file.permissions() | QFile::WriteUser | QFile::ReadGroup
-                | QFile::ReadOther);
+            setDefaultFilePermissions(&file, DefaultFilePermissions::NonExecutable);
         } catch (const Error &/*error*/) {
             if (!newBinaryWritten) {
                 newBinaryWritten = true;
@@ -1494,6 +1477,8 @@ bool PackageManagerCorePrivate::runInstaller()
             if (!performOperationThreaded(mkdirOp))
                 throw Error(mkdirOp->errorString());
         }
+        setDefaultFilePermissions(target, DefaultFilePermissions::Executable);
+
         const QString remove = m_core->value(scRemoveTargetDir);
         if (QVariant(remove).toBool())
             addPerformed(takeOwnedOperation(mkdirOp));
@@ -1643,16 +1628,10 @@ bool PackageManagerCorePrivate::runPackageUpdater()
         //to have some progress for the cleanup/write component.xml step
         ProgressCoordinator::instance()->addReservePercentagePoints(1);
 
-#if defined(Q_OS_LINUX) || defined(Q_OS_MACOS)
-        // check if we need admin rights and ask before the action happens
-        // on Linux and macOS also check target directory subdirectories
-        if (!directoryWritable(targetDir()) || !subdirectoriesWritable(targetDir()))
-            adminRightsGained = m_core->gainAdminRights();
-#else
         // check if we need admin rights and ask before the action happens
         if (!directoryWritable(targetDir()))
             adminRightsGained = m_core->gainAdminRights();
-#endif
+
         const QList<Component *> componentsToInstall = m_core->orderedComponentsToInstall();
         qDebug() << "Install size:" << componentsToInstall.size() << "components";
 

src/libs/installer/packagemanagercore_p.h

@@ -93,7 +93,6 @@ public:
     QString registerPath();
 
     bool directoryWritable(const QString &path) const;
-    bool subdirectoriesWritable(const QString &path) const;
 
     QString maintenanceToolName() const;
     QString installerBinaryPath() const;

src/libs/installer/utils.cpp

@@ -28,6 +28,8 @@
 
 #include "utils.h"
 
+#include "fileutils.h"
+
 #include <QCoreApplication>
 #include <QDateTime>
 #include <QDir>
@@ -277,6 +279,7 @@ bool QInstaller::PlainVerboseWriterOutput::write(const QString &fileName, QIODev
     QFile output(fileName);
     if (output.open(openMode)) {
         output.write(data);
+        setDefaultFilePermissions(&output, DefaultFilePermissions::NonExecutable);
         return true;
     }
     return false;

src/libs/kdtools/localpackagehub.cpp

@@ -28,6 +28,7 @@
 ****************************************************************************/
 
 #include "localpackagehub.h"
+#include "fileutils.h"
 #include "globals.h"
 #include "constants.h"
 
@@ -431,6 +432,11 @@ void LocalPackageHub::writeToDisk()
 
         file.write(doc.toByteArray(4));
         file.close();
+
+        // Write permissions for installation information file
+        QInstaller::setDefaultFilePermissions(
+            &file, DefaultFilePermissions::NonExecutable);
+
         d->modified = false;
     }
 }

tests/auto/installer/fileutils/fileutils.pro

@@ -0,0 +1,6 @@
+include(../../qttest.pri)
+
+QT -= gui
+QT += testlib
+
+SOURCES += tst_fileutils.cpp

tests/auto/installer/fileutils/tst_fileutils.cpp

@@ -0,0 +1,107 @@
+/**************************************************************************
+**
+** Copyright (C) 2019 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the Qt Installer Framework.
+**
+** $QT_BEGIN_LICENSE:GPL-EXCEPT$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+**************************************************************************/
+
+#include <qinstallerglobal.h>
+#include <fileutils.h>
+
+#include <QObject>
+#include <QTest>
+#include <QFile>
+#include <QDir>
+
+using namespace QInstaller;
+
+class tst_fileutils : public QObject
+{
+    Q_OBJECT
+
+private slots:
+    void testSetDefaultFilePermissions()
+    {
+#if defined(Q_OS_WIN)
+        QVERIFY(setDefaultFilePermissions(QInstaller::generateTemporaryFileName(),
+            DefaultFilePermissions::NonExecutable));
+
+        QVERIFY(setDefaultFilePermissions(QInstaller::generateTemporaryFileName(),
+            DefaultFilePermissions::Executable));
+#elif defined(Q_OS_UNIX)
+        // Need to include the "user" flags here as they will be returned
+        // by QFile::permissions(). Same as owner permissions of the file.
+        QFlags<QFileDevice::Permission> permissions(QFileDevice::ReadOwner
+            | QFileDevice::WriteOwner | QFileDevice::ReadUser | QFileDevice::WriteUser
+            | QFileDevice::ReadGroup | QFileDevice::ReadOther);
+
+        QFlags<QFileDevice::Permission> exePermissions(permissions | QFileDevice::ExeOwner
+            | QFileDevice::ExeUser | QFileDevice::ExeGroup | QFileDevice::ExeOther);
+
+        QString fileName = QInstaller::generateTemporaryFileName();
+        QFile testFile(fileName);
+
+        const QString message = "Target \"%1\" does not exists.";
+
+        // Test non-existing file
+        QTest::ignoreMessage(QtWarningMsg, qPrintable(message.arg(fileName)));
+        QVERIFY(!setDefaultFilePermissions(fileName, DefaultFilePermissions::NonExecutable));
+
+        QTest::ignoreMessage(QtWarningMsg, qPrintable(message.arg(testFile.fileName())));
+        QVERIFY(!setDefaultFilePermissions(&testFile, DefaultFilePermissions::NonExecutable));
+
+        QVERIFY(testFile.open(QIODevice::ReadWrite));
+        QVERIFY(testFile.exists());
+        testFile.close();
+
+        // Test with file name
+        QVERIFY(setDefaultFilePermissions(fileName, DefaultFilePermissions::NonExecutable));
+        QCOMPARE(QFile().permissions(fileName), permissions);
+
+        QVERIFY(setDefaultFilePermissions(fileName, DefaultFilePermissions::Executable));
+        QCOMPARE(QFile().permissions(fileName), exePermissions);
+
+        // Test with QFile object
+        QVERIFY(setDefaultFilePermissions(&testFile, DefaultFilePermissions::NonExecutable));
+        QCOMPARE(QFile().permissions(fileName), permissions);
+
+        QVERIFY(setDefaultFilePermissions(&testFile, DefaultFilePermissions::Executable));
+        QCOMPARE(QFile().permissions(fileName), exePermissions);
+
+        // Test with directory path
+        QString testDir = QDir().tempPath() + QLatin1String("/testDir");
+        QVERIFY(QDir().mkdir(testDir));
+
+        QVERIFY(setDefaultFilePermissions(testDir, DefaultFilePermissions::Executable));
+        QCOMPARE(QFile().permissions(testDir), exePermissions);
+
+        QVERIFY(QDir().rmdir(testDir));
+        QVERIFY(testFile.remove());
+#endif
+    }
+};
+
+QTEST_MAIN(tst_fileutils)
+
+#include "tst_fileutils.moc"

tests/auto/installer/installer.pro

@@ -10,6 +10,7 @@ SUBDIRS += \
     messageboxhandler \
     extractarchiveoperationtest \
     lib7zfacade \
+    fileutils \
     unicodeexecutable \
     scriptengine \
     consumeoutputoperationtest \

tests/auto/installer/packagemanagercore/tst_packagemanagercore.cpp

@@ -284,35 +284,6 @@ private slots:
 #endif
         QVERIFY(QDir().rmdir(testDirectory));
     }
-
-    void testSubdirectoriesWritable()
-    {
-        PackageManagerCore core;
-
-        const QString testDirectory = QInstaller::generateTemporaryFileName();
-        QVERIFY(QDir().mkpath(testDirectory));
-        QVERIFY(QDir(testDirectory).exists());
-
-        const QString testSubdirectory = testDirectory + "/" + QString::number(qrand() % 1000);
-
-        QVERIFY(QDir().mkpath(testSubdirectory));
-        QVERIFY(QDir(testSubdirectory).exists());
-
-        // should be writable
-        QVERIFY(core.subdirectoriesWritable(testDirectory));
-
-#if defined(Q_OS_LINUX) || defined(Q_OS_MACOS)
-        QFile dirDevice(testSubdirectory);
-        dirDevice.setPermissions(QFileDevice::ReadOwner | QFileDevice::ExeOwner);
-
-        // should not be writable
-        QVERIFY(!core.subdirectoriesWritable(testDirectory));
-
-        dirDevice.setPermissions(QFileDevice::ReadOwner | QFileDevice::WriteOwner | QFileDevice::ExeOwner);
-#endif
-        QVERIFY(QDir().rmdir(testSubdirectory));
-        QVERIFY(QDir().rmdir(testDirectory));
-    }
 };