Bugfix/delete readonly folders

src/libsync/filesystem.cpp

@@ -1,4 +1,4 @@
 /*
  * Copyright (C) by Daniel Molkentin <[email protected]>
  *
  * This program is free software; you can redistribute it and/or modify
@@ -480,6 +480,25 @@
         return false;
     }
 }
+
+FileSystem::FilePermissionsRestore::FilePermissionsRestore(const QString &path, FolderPermissions temporaryPermissions)
+    : _path(path)
+{
+    const auto stdStrPath = _path.toStdWString();
+    _initialPermissions = FileSystem::isFolderReadOnly(stdStrPath) ? OCC::FileSystem::FolderPermissions::ReadOnly : OCC::FileSystem::FolderPermissions::ReadWrite;
+    if (_initialPermissions != temporaryPermissions) {
+        _rollbackNeeded = true;
+        FileSystem::setFolderPermissions(_path, temporaryPermissions);
+    }
+}
+
+FileSystem::FilePermissionsRestore::~FilePermissionsRestore()
+{
+    if (_rollbackNeeded) {
+        FileSystem::setFolderPermissions(_path, _initialPermissions);
+    }
+}
+
 #endif
 
 } // namespace OCC

src/libsync/filesystem.h

@@ -1,4 +1,4 @@
 /*
  * Copyright (C) by Olivier Goffart <[email protected]>
  *
  * This program is free software; you can redistribute it and/or modify
@@ -14,7 +14,7 @@
 
 #pragma once
 
 #include "config.h"
 
 #include "owncloudlib.h"
 #include "common/filesystembase.h"
@@ -41,6 +41,19 @@
  * @brief This file contains file system helper
  */
 namespace FileSystem {
+    class OWNCLOUDSYNC_EXPORT FilePermissionsRestore {
+    public:
+        explicit FilePermissionsRestore(const QString &path,
+                                        FileSystem::FolderPermissions temporaryPermissions);
+
+        ~FilePermissionsRestore();
+
+    private:
+        QString _path;
+        FileSystem::FolderPermissions _initialPermissions;
+        bool _rollbackNeeded = false;
+    };
+
     struct OWNCLOUDSYNC_EXPORT FileLockingInfo {
         enum class Type { Unset = -1, Locked, Unlocked };
         QString path;

src/libsync/propagatorjobs.cpp

@@ -63,6 +63,9 @@ bool PropagateLocalRemove::removeRecursively(const QString &path)
     QStringList errors;
     QList<QPair<QString, bool>> deleted;
 #if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15
+    const auto fileInfo = QFileInfo{absolute};
+    const auto parentFolderPath = fileInfo.dir().absolutePath();
+    const auto parentPermissionsHandler = FileSystem::FilePermissionsRestore{parentFolderPath, FileSystem::FolderPermissions::ReadWrite};
     FileSystem::setFolderPermissions(absolute, FileSystem::FolderPermissions::ReadWrite);
 #endif
     bool success = FileSystem::removeRecursively(
@@ -126,10 +129,18 @@ void PropagateLocalRemove::start()
                 return;
             }
         } else {
-            if (FileSystem::fileExists(filename)
-                && !FileSystem::remove(filename, &removeError)) {
-                done(SyncFileItem::NormalError, removeError, ErrorCategory::GenericError);
-                return;
+            if (FileSystem::fileExists(filename)) {
+#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15
+                const auto fileInfo = QFileInfo{filename};
+                const auto parentFolderPath = fileInfo.dir().absolutePath();
+
+                const auto parentPermissionsHandler = FileSystem::FilePermissionsRestore{parentFolderPath, FileSystem::FolderPermissions::ReadWrite};
+#endif
+
+                if (!FileSystem::remove(filename, &removeError)) {
+                    done(SyncFileItem::NormalError, removeError, ErrorCategory::GenericError);
+                    return;
+                }
             }
         }
     }
@@ -368,6 +379,8 @@ void PropagateLocalRename::start()
         };
 #endif
 
+        const auto folderPermissionsHandler = FileSystem::FilePermissionsRestore{existingFile, FileSystem::FolderPermissions::ReadWrite};
+
         emit propagator()->touchedFile(existingFile);
         emit propagator()->touchedFile(targetFile);
         if (QString renameError; !FileSystem::rename(existingFile, targetFile, &renameError)) {

test/testpermissions.cpp

@@ -1,4 +1,4 @@
 /*
  *    This software is in the public domain, furnished "as is", without technical
  *    support, and with no warranty, express or implied, as to its usefulness for
  *    any purpose.
@@ -769,6 +769,140 @@
         QVERIFY(testFolderStatus.permissions() & std::filesystem::perms::owner_read);
         QVERIFY(!static_cast<bool>(testFolderStatus.permissions() & std::filesystem::perms::owner_write));
     }
+
+    void testDeleteChildItemsInReadOnlyFolder()
+    {
+        FakeFolder fakeFolder{FileInfo{}};
+
+        auto &remote = fakeFolder.remoteModifier();
+
+        remote.mkdir("readOnlyFolder");
+        remote.mkdir("readOnlyFolder/test");
+        remote.insert("readOnlyFolder/readOnlyFile.txt");
+
+        remote.find("readOnlyFolder")->permissions = RemotePermissions::fromServerString("M");
+        remote.find("readOnlyFolder/test")->permissions = RemotePermissions::fromServerString("m");
+        remote.find("readOnlyFolder/readOnlyFile.txt")->permissions = RemotePermissions::fromServerString("m");
+
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+
+        remote.remove("readOnlyFolder/test");
+        remote.remove("readOnlyFolder/readOnlyFile.txt");
+
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+
+        const auto ensureReadOnlyItem = [&fakeFolder] (const auto path) -> bool {
+            const auto itemStatus = std::filesystem::status(static_cast<QString>(fakeFolder.localPath() + path).toStdWString());
+            return static_cast<bool>(itemStatus.permissions() & std::filesystem::perms::owner_read);
+        };
+        QVERIFY(ensureReadOnlyItem("/readOnlyFolder"));
+    }
+
+    void testRenameChildItemsInReadOnlyFolder()
+    {
+        FakeFolder fakeFolder{FileInfo{}};
+
+        auto &remote = fakeFolder.remoteModifier();
+
+        remote.mkdir("readOnlyFolder");
+        remote.mkdir("readOnlyFolder/test");
+        remote.insert("readOnlyFolder/readOnlyFile.txt");
+
+        remote.find("readOnlyFolder")->permissions = RemotePermissions::fromServerString("M");
+        remote.find("readOnlyFolder/test")->permissions = RemotePermissions::fromServerString("m");
+        remote.find("readOnlyFolder/readOnlyFile.txt")->permissions = RemotePermissions::fromServerString("m");
+
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+
+        remote.rename("readOnlyFolder/test", "readOnlyFolder/testRename");
+        remote.rename("readOnlyFolder/readOnlyFile.txt", "readOnlyFolder/readOnlyFileRename.txt");
+
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+
+        const auto ensureReadOnlyItem = [&fakeFolder] (const auto path) -> bool {
+            const auto itemStatus = std::filesystem::status(static_cast<QString>(fakeFolder.localPath() + path).toStdWString());
+            return static_cast<bool>(itemStatus.permissions() & std::filesystem::perms::owner_read);
+        };
+        QVERIFY(ensureReadOnlyItem("/readOnlyFolder"));
+        QVERIFY(ensureReadOnlyItem("/readOnlyFolder/testRename"));
+        QVERIFY(ensureReadOnlyItem("/readOnlyFolder/readOnlyFileRename.txt"));
+    }
+
+    void testMoveChildItemsInReadOnlyFolder()
+    {
+        FakeFolder fakeFolder{FileInfo{}};
+
+        auto &remote = fakeFolder.remoteModifier();
+
+        remote.mkdir("readOnlyFolder");
+        remote.mkdir("readOnlyFolder/child");
+        remote.mkdir("readOnlyFolder/test");
+        remote.insert("readOnlyFolder/readOnlyFile.txt");
+
+        remote.find("readOnlyFolder")->permissions = RemotePermissions::fromServerString("M");
+        remote.find("readOnlyFolder/child")->permissions = RemotePermissions::fromServerString("m");
+        remote.find("readOnlyFolder/test")->permissions = RemotePermissions::fromServerString("m");
+        remote.find("readOnlyFolder/readOnlyFile.txt")->permissions = RemotePermissions::fromServerString("m");
+
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+
+        remote.rename("readOnlyFolder/test", "readOnlyFolder/child/test");
+        remote.rename("readOnlyFolder/readOnlyFile.txt", "readOnlyFolder/child/readOnlyFile.txt");
+
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+
+        const auto ensureReadOnlyItem = [&fakeFolder] (const auto path) -> bool {
+            const auto itemStatus = std::filesystem::status(static_cast<QString>(fakeFolder.localPath() + path).toStdWString());
+            return static_cast<bool>(itemStatus.permissions() & std::filesystem::perms::owner_read);
+        };
+        QVERIFY(ensureReadOnlyItem("/readOnlyFolder"));
+        QVERIFY(ensureReadOnlyItem("/readOnlyFolder/child"));
+        QVERIFY(ensureReadOnlyItem("/readOnlyFolder/child/test"));
+        QVERIFY(ensureReadOnlyItem("/readOnlyFolder/child/readOnlyFile.txt"));
+    }
+
+    void testModifyChildItemsInReadOnlyFolder()
+    {
+        FakeFolder fakeFolder{FileInfo{}};
+
+        auto &remote = fakeFolder.remoteModifier();
+
+        remote.mkdir("readOnlyFolder");
+        remote.mkdir("readOnlyFolder/test");
+        remote.insert("readOnlyFolder/readOnlyFile.txt");
+
+        remote.find("readOnlyFolder")->permissions = RemotePermissions::fromServerString("M");
+        remote.find("readOnlyFolder/test")->permissions = RemotePermissions::fromServerString("m");
+        remote.find("readOnlyFolder/readOnlyFile.txt")->permissions = RemotePermissions::fromServerString("m");
+
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+
+        remote.insert("readOnlyFolder/test/newFile.txt");
+        remote.find("readOnlyFolder/test/newFile.txt")->permissions = RemotePermissions::fromServerString("m");
+        remote.mkdir("readOnlyFolder/test/newFolder");
+        remote.find("readOnlyFolder/test/newFolder")->permissions = RemotePermissions::fromServerString("m");
+        remote.appendByte("readOnlyFolder/readOnlyFile.txt");
+
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+
+        const auto ensureReadOnlyItem = [&fakeFolder] (const auto path) -> bool {
+            const auto itemStatus = std::filesystem::status(static_cast<QString>(fakeFolder.localPath() + path).toStdWString());
+            return static_cast<bool>(itemStatus.permissions() & std::filesystem::perms::owner_read);
+        };
+        QVERIFY(ensureReadOnlyItem("/readOnlyFolder"));
+        QVERIFY(ensureReadOnlyItem("/readOnlyFolder/test"));
+        QVERIFY(ensureReadOnlyItem("/readOnlyFolder/readOnlyFile.txt"));
+        QVERIFY(ensureReadOnlyItem("/readOnlyFolder/test/newFile.txt"));
+        QVERIFY(ensureReadOnlyItem("/readOnlyFolder/newFolder"));
+    }
 #endif
 };