From 90befac9012943347f87ab7b6ba94fe9508a19d8 Mon Sep 17 00:00:00 2001
From: Christian Kamm <mail@ckamm.de>
Date: Thu, 5 Oct 2017 11:39:35 +0200
Subject: [PATCH] Rename detection: File size must be equal

Comparison of file sizes for potential conflicts was added in
0eb9401c624f20a128b46f8eb1fa5a984f9ef61e, but did not extend to checking
the file size in case of potential local moves.

This commit adds this check and adds tests for various move+change
scenarios.
---
 src/csync/csync_update.cpp |  4 +-
 test/testsyncengine.cpp    | 78 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 2 deletions(-)

diff --git a/src/csync/csync_update.cpp b/src/csync/csync_update.cpp
index 9412d5185..0271b1c56 100644
--- a/src/csync/csync_update.cpp
+++ b/src/csync/csync_update.cpp
@@ -265,8 +265,8 @@ static int _csync_detect_update(CSYNC *ctx, std::unique_ptr<csync_file_stat_t> f
           fs->instruction = CSYNC_INSTRUCTION_NEW;
 
           bool isRename =
-              base.isValid() && base._inode == fs->inode && base._type == fs->type
-                  && (base._modtime == fs->modtime || fs->type == CSYNC_FTW_TYPE_DIR)
+              base.isValid() && base._type == fs->type
+                  && ((base._modtime == fs->modtime && base._fileSize == fs->size) || fs->type == CSYNC_FTW_TYPE_DIR)
 #ifdef NO_RENAME_EXTENSION
                   && _csync_sameextension(base._path, fs->path)
 #endif
diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp
index 169c2e6fb..9c1b7a2c0 100644
--- a/test/testsyncengine.cpp
+++ b/test/testsyncengine.cpp
@@ -535,6 +535,84 @@ private slots:
         QCOMPARE(nPUT, 6);
         QCOMPARE(n507, 3);
     }
+
+    void testLocalMove()
+    {
+        FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() };
+
+        int nPUT = 0;
+        int nDELETE = 0;
+        fakeFolder.setServerOverride([&](QNetworkAccessManager::Operation op, const QNetworkRequest &) {
+            if (op == QNetworkAccessManager::PutOperation)
+                ++nPUT;
+            if (op == QNetworkAccessManager::DeleteOperation)
+                ++nDELETE;
+            return nullptr;
+        });
+
+        // For directly editing the remote checksum
+        FileInfo &remoteInfo = fakeFolder.remoteModifier();
+
+        // Simple move causing a remote rename
+        fakeFolder.localModifier().rename("A/a1", "A/a1m");
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(fakeFolder.currentLocalState(), remoteInfo);
+        QCOMPARE(nPUT, 0);
+
+        // Move-and-change, causing a upload and delete
+        fakeFolder.localModifier().rename("A/a2", "A/a2m");
+        fakeFolder.localModifier().appendByte("A/a2m");
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(fakeFolder.currentLocalState(), remoteInfo);
+        QCOMPARE(nPUT, 1);
+        QCOMPARE(nDELETE, 1);
+
+        // Move-and-change, mtime+content only
+        fakeFolder.localModifier().rename("B/b1", "B/b1m");
+        fakeFolder.localModifier().setContents("B/b1m", 'C');
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(fakeFolder.currentLocalState(), remoteInfo);
+        QCOMPARE(nPUT, 2);
+        QCOMPARE(nDELETE, 2);
+
+        // Move-and-change, size+content only
+        auto mtime = fakeFolder.remoteModifier().find("B/b2")->lastModified;
+        fakeFolder.localModifier().rename("B/b2", "B/b2m");
+        fakeFolder.localModifier().appendByte("B/b2m");
+        fakeFolder.localModifier().setModTime("B/b2m", mtime);
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(fakeFolder.currentLocalState(), remoteInfo);
+        QCOMPARE(nPUT, 3);
+        QCOMPARE(nDELETE, 3);
+
+        // Move-and-change, content only -- c1 has no checksum, so we fail to detect this!
+        mtime = fakeFolder.remoteModifier().find("C/c1")->lastModified;
+        fakeFolder.localModifier().rename("C/c1", "C/c1m");
+        fakeFolder.localModifier().setContents("C/c1m", 'C');
+        fakeFolder.localModifier().setModTime("C/c1m", mtime);
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(nPUT, 3);
+        QCOMPARE(nDELETE, 3);
+        QVERIFY(!(fakeFolder.currentLocalState() == remoteInfo));
+
+        // cleanup, and upload a file that will have a checksum in the db
+        fakeFolder.localModifier().remove("C/c1m");
+        fakeFolder.localModifier().insert("C/c3");
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(fakeFolder.currentLocalState(), remoteInfo);
+        QCOMPARE(nPUT, 4);
+        QCOMPARE(nDELETE, 4);
+
+        // Move-and-change, content only, this time while having a checksum
+        mtime = fakeFolder.remoteModifier().find("C/c3")->lastModified;
+        fakeFolder.localModifier().rename("C/c3", "C/c3m");
+        fakeFolder.localModifier().setContents("C/c3m", 'C');
+        fakeFolder.localModifier().setModTime("C/c3m", mtime);
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(nPUT, 5);
+        QCOMPARE(nDELETE, 5);
+        QCOMPARE(fakeFolder.currentLocalState(), remoteInfo);
+    }
 };
 
 QTEST_GUILESS_MAIN(TestSyncEngine)
-- 
2.30.2