PropagateDownloadFile/FileSystem: Improve previous commit (by mainly …

…undoing it) Instead of copying the permissions on the symlink, use the default permissions in that case. Otherwise, the newly downloaded files might become executable since symlinks often have with all permissions set. Signed-off-by: Tamino Bauknecht <[email protected]>
nextcloud · Dec 8, 2023 · 132aa68 · 132aa68
1 parent 5e4fe3a
commit 132aa68
Show file tree

Hide file tree

Showing 3 changed files with 5 additions and 39 deletions.
diff --git a/src/libsync/filesystem.cpp b/src/libsync/filesystem.cpp
@@ -14,8 +14,6 @@
 
 #include "filesystem.h"
 
-#include <filesystem>
-
 #include "common/utility.h"
 #include <QFile>
 #include <QFileInfo>
@@ -142,31 +140,6 @@ qint64 FileSystem::getSize(const QString &filename)
     return info.size();
 }
 
-QFile::Permissions FileSystem::getPermissions(const QString &filename)
-{
-    using qtStdFilePermissionsMapping = std::pair<QFile::Permission, std::filesystem::perms>;
-    constexpr std::array<qtStdFilePermissionsMapping, 9> qtStdFilePermissionsMappings = {
-        std::make_pair(QFileDevice::ReadOwner, std::filesystem::perms::owner_read),
-        {QFileDevice::WriteOwner, std::filesystem::perms::owner_write},
-        {QFileDevice::ExeOwner, std::filesystem::perms::owner_exec},
-        {QFileDevice::ReadGroup, std::filesystem::perms::group_read},
-        {QFileDevice::WriteGroup, std::filesystem::perms::group_write},
-        {QFileDevice::ExeGroup, std::filesystem::perms::group_exec},
-        {QFileDevice::ReadOther, std::filesystem::perms::others_read},
-        {QFileDevice::WriteOther, std::filesystem::perms::others_write},
-        {QFileDevice::ExeOther, std::filesystem::perms::others_exec}};
-
-    auto fileStatus = std::filesystem::symlink_status(filename.toStdString());
-    auto permissions = fileStatus.permissions();
-
-    QFile::Permissions resultPermissions;
-    for (auto [qtPermissionFlag, stdPermissionFlag] : qtStdFilePermissionsMappings) {
-        auto isPermissionSet = (permissions & stdPermissionFlag) != std::filesystem::perms::none;
-        resultPermissions.setFlag(qtPermissionFlag, isPermissionSet);
-    }
-    return resultPermissions;
-}
-
 // Code inspired from Qt5's QDir::removeRecursively
 bool FileSystem::removeRecursively(const QString &path, const std::function<void(const QString &path, bool isDir)> &onDeleted, QStringList *errors)
 {

diff --git a/src/libsync/filesystem.h b/src/libsync/filesystem.h
@@ -63,13 +63,6 @@ namespace FileSystem {
      */
     qint64 OWNCLOUDSYNC_EXPORT getSize(const QString &filename);
 
-    /**
-     * @brief Get permissions for a file
-     *
-     * If the file is a symlink, the permissions for the symlink are returned.
-     */
-    QFile::Permissions OWNCLOUDSYNC_EXPORT getPermissions(const QString &filename);
-
     /**
      * @brief Retrieve a file inode with csync
      */

diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp
@@ -1220,12 +1220,12 @@ void PropagateDownloadFile::downloadFinished()
 
     auto previousFileExists = FileSystem::fileExists(filename) && _item->_instruction != CSYNC_INSTRUCTION_CASE_CLASH_CONFLICT;
     if (previousFileExists) {
-        // Preserve the existing file permissions.
-        auto previousPermissions = FileSystem::getPermissions(filename);
-        if (previousPermissions != FileSystem::getPermissions(_tmpFile.fileName())) {
-            _tmpFile.setPermissions(previousPermissions);
+        // Preserve the existing file permissions (except it was a symlink)
+        const auto existingFile = QFileInfo{filename};
+        if (!existingFile.isSymLink() && existingFile.permissions() != _tmpFile.permissions()) {
+            _tmpFile.setPermissions(existingFile.permissions());
         }
-        preserveGroupOwnership(_tmpFile.fileName(), QFileInfo(filename));
+        preserveGroupOwnership(_tmpFile.fileName(), existingFile);
 
         // Make the file a hydrated placeholder if possible
         const auto result = propagator()->syncOptions()._vfs->convertToPlaceholder(_tmpFile.fileName(), *_item, filename);