From: Olivier Goffart <ogoffart@woboq.com>
Date: Thu, 10 Oct 2019 11:24:24 +0000 (+0200)
Subject: When moving is allowed but deleting is not, do not restore moved items
X-Git-Tag: archive/raspbian/3.16.7-1_deb13u1+rpi1~1^2~12^2~21^2~468^2~169
X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=83d743b66b7b9107db5fcdbe28eec36f942b3f9d;p=nextcloud-desktop.git

When moving is allowed but deleting is not, do not restore moved items

Issue #7293
---

diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp
index d2d562d5a..b018c1a8c 100644
--- a/src/libsync/discovery.cpp
+++ b/src/libsync/discovery.cpp
@@ -1108,6 +1108,7 @@ void ProcessDirectoryJob::processFileFinalize(
 
     if (item->isDirectory() && item->_instruction == CSYNC_INSTRUCTION_SYNC)
         item->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA;
+    bool removed = item->_instruction == CSYNC_INSTRUCTION_REMOVE;
     if (checkPermissions(item)) {
         if (item->_isRestoration && item->isDirectory())
             recurse = true;
@@ -1116,7 +1117,7 @@ void ProcessDirectoryJob::processFileFinalize(
     }
     if (recurse) {
         auto job = new ProcessDirectoryJob(path, item, recurseQueryLocal, recurseQueryServer, this);
-        if (item->_instruction == CSYNC_INSTRUCTION_REMOVE) {
+        if (removed) {
             job->setParent(_discoveryData);
             _discoveryData->_queuedDeletedDirectories[path._original] = job;
         } else {
@@ -1124,7 +1125,7 @@ void ProcessDirectoryJob::processFileFinalize(
             _queuedJobs.push_back(job);
         }
     } else {
-        if (item->_instruction == CSYNC_INSTRUCTION_REMOVE
+        if (removed
             // For the purpose of rename deletion, restored deleted placeholder is as if it was deleted
             || (item->_type == ItemTypeVirtualFile && item->_instruction == CSYNC_INSTRUCTION_NEW)) {
             _discoveryData->_deletedItem[path._original] = item;
diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp
index ca0322c5d..3cd1f3e34 100644
--- a/src/libsync/discoveryphase.cpp
+++ b/src/libsync/discoveryphase.cpp
@@ -161,7 +161,9 @@ QPair<bool, QByteArray> DiscoveryPhase::findAndCancelDeletedJob(const QString &o
     if (it != _deletedItem.end()) {
         ENFORCE((*it)->_instruction == CSYNC_INSTRUCTION_REMOVE
             // re-creation of virtual files count as a delete
-             || ((*it)->_type == ItemTypeVirtualFile && (*it)->_instruction == CSYNC_INSTRUCTION_NEW));
+             || ((*it)->_type == ItemTypeVirtualFile && (*it)->_instruction == CSYNC_INSTRUCTION_NEW)
+             || ((*it)->_isRestoration && (*it)->_instruction == CSYNC_INSTRUCTION_NEW)
+               );
         (*it)->_instruction = CSYNC_INSTRUCTION_NONE;
         result = true;
         oldEtag = (*it)->_etag;
diff --git a/test/testpermissions.cpp b/test/testpermissions.cpp
index 10494d3b2..8fdcc62de 100644
--- a/test/testpermissions.cpp
+++ b/test/testpermissions.cpp
@@ -465,6 +465,50 @@ private slots:
         QVERIFY(cls.find("zallowed/sub2"));
         QVERIFY(cls.find("zallowed/sub2/file"));
     }
+
+    // Test for issue #7293
+    void testAllowedMoveForbiddenDelete() {
+         FakeFolder fakeFolder{FileInfo{}};
+
+        // Some of this test depends on the order of discovery. With threading
+        // that order becomes effectively random, but we want to make sure to test
+        // all cases and thus disable threading.
+        auto syncOpts = fakeFolder.syncEngine().syncOptions();
+        syncOpts._parallelNetworkJobs = 1;
+        fakeFolder.syncEngine().setSyncOptions(syncOpts);
+
+        auto &lm = fakeFolder.localModifier();
+        auto &rm = fakeFolder.remoteModifier();
+        rm.mkdir("changeonly");
+        rm.mkdir("changeonly/sub1");
+        rm.insert("changeonly/sub1/file1");
+        rm.insert("changeonly/sub1/filetorname1a");
+        rm.insert("changeonly/sub1/filetorname1z");
+        rm.mkdir("changeonly/sub2");
+        rm.insert("changeonly/sub2/file2");
+        rm.insert("changeonly/sub2/filetorname2a");
+        rm.insert("changeonly/sub2/filetorname2z");
+
+        setAllPerm(rm.find("changeonly"), RemotePermissions::fromServerString("NSV"));
+
+        QVERIFY(fakeFolder.syncOnce());
+
+        lm.rename("changeonly/sub1/filetorname1a", "changeonly/sub1/aaa1_renamed");
+        lm.rename("changeonly/sub1/filetorname1z", "changeonly/sub1/zzz1_renamed");
+
+        lm.rename("changeonly/sub2/filetorname2a", "changeonly/sub2/aaa2_renamed");
+        lm.rename("changeonly/sub2/filetorname2z", "changeonly/sub2/zzz2_renamed");
+
+        lm.rename("changeonly/sub1", "changeonly/aaa");
+        lm.rename("changeonly/sub2", "changeonly/zzz");
+
+
+        auto expectedState = fakeFolder.currentLocalState();
+
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(fakeFolder.currentLocalState(), expectedState);
+        QCOMPARE(fakeFolder.currentRemoteState(), expectedState);
+    }
 };
 
 QTEST_GUILESS_MAIN(TestPermissions)