ensure discovery phase will not try to upload files with invalid mtime

Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>

diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp
index cb6ce27555a244cf5379617fd468c7bb00532e73..a7e41d8bc9e23d41e05af83353b1a2089f536f71 100644 (file)
--- a/src/libsync/discovery.cpp
+++ b/src/libsync/discovery.cpp
@@ -539,19 +539,11 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(
             } else {
                 item->_instruction = CSYNC_INSTRUCTION_SYNC;
             }
-        } else if (dbEntry._modtime <= 0 && serverEntry.modtime > 0) {
+        } else if (dbEntry._modtime != serverEntry.modtime && localEntry.size == serverEntry.size && dbEntry._fileSize == serverEntry.size && dbEntry._etag == serverEntry.etag) {
             item->_direction = SyncFileItem::Down;
             item->_modtime = serverEntry.modtime;
             item->_size = sizeOnServer;
-            if (serverEntry.isDirectory) {
-                ENFORCE(dbEntry.isDirectory());
-                item->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA;
-            } else if (!localEntry.isValid() && _queryLocal != ParentNotChanged) {
-                // Deleted locally, changed on server
-                item->_instruction = CSYNC_INSTRUCTION_NEW;
-            } else {
-                item->_instruction = CSYNC_INSTRUCTION_SYNC;
-            }
+            item->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA;
         } else if (dbEntry._remotePerm != serverEntry.remotePerm || dbEntry._fileId != serverEntry.fileId || metaDataSizeNeedsUpdateForE2EeFilePlaceholder) {
             if (metaDataSizeNeedsUpdateForE2EeFilePlaceholder) {
                 // we are updating placeholder sizes after migrating from older versions with VFS + E2EE implicit hydration not supported
@@ -848,7 +840,8 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
         if (_queryLocal != NormalQuery && _queryServer != NormalQuery)
             recurse = false;
 
-        if ((item->_direction == SyncFileItem::Down || item->_instruction == CSYNC_INSTRUCTION_CONFLICT) && (item->_modtime <= 0 || item->_modtime >= 0xFFFFFFFF)) {
+        if ((item->_direction == SyncFileItem::Down || item->_instruction == CSYNC_INSTRUCTION_CONFLICT || item->_instruction == CSYNC_INSTRUCTION_NEW || item->_instruction == CSYNC_INSTRUCTION_SYNC) &&
+                (item->_modtime <= 0 || item->_modtime >= 0xFFFFFFFF)) {
             item->_instruction = CSYNC_INSTRUCTION_ERROR;
             item->_errorString = tr("Cannot sync due to invalid modification time");
             item->_status = SyncFileItem::Status::NormalError;
@@ -1007,7 +1000,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
             item->_modtime = localEntry.modtime;
             item->_type = localEntry.isDirectory ? ItemTypeDirectory : ItemTypeFile;
             _childModified = true;
-        } else if (dbEntry._modtime > 0 && localEntry.modtime <= 0) {
+        } else if (dbEntry._modtime > 0 && (localEntry.modtime <= 0 || localEntry.modtime >= 0xFFFFFFFF) && dbEntry._fileSize == localEntry.size) {
             item->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA;
             item->_direction = SyncFileItem::Down;
             item->_size = localEntry.size > 0 ? localEntry.size : dbEntry._fileSize;

diff --git a/test/testsyncvirtualfiles.cpp b/test/testsyncvirtualfiles.cpp
index e45fc7d583aeed5495c51f322383efd596381401..bff6488e45caae9e680d882d08bc98b86fdd34ef 100644 (file)
--- a/test/testsyncvirtualfiles.cpp
+++ b/test/testsyncvirtualfiles.cpp
@@ -1582,6 +1582,84 @@ private slots:
 
         QVERIFY(fakeFolder.syncOnce());
     }
+
+    void testInvalidMtimeLocalDiscovery()
+    {
+        constexpr auto INVALID_MTIME1 = 0;
+        constexpr auto INVALID_MTIME2 = 0xFFFFFFFF;
+        constexpr auto CURRENT_MTIME = 1646057277;
+
+        FakeFolder fakeFolder{FileInfo{}};
+        setupVfs(fakeFolder);
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+        QSignalSpy statusSpy(&fakeFolder.syncEngine().syncFileStatusTracker(), &SyncFileStatusTracker::fileStatusChanged);
+
+        const QString fooFileRootFolder("foo");
+        const QString barFileRootFolder("bar");
+        const QString fooFileSubFolder("subfolder/foo");
+        const QString barFileSubFolder("subfolder/bar");
+        const QString fooFileAaaSubFolder("aaa/subfolder/foo");
+        const QString barFileAaaSubFolder("aaa/subfolder/bar");
+
+        auto checkStatus = [&]() -> SyncFileStatus::SyncFileStatusTag {
+            auto file = QFileInfo{fakeFolder.syncEngine().localPath(), barFileAaaSubFolder};
+            auto locPath = fakeFolder.syncEngine().localPath();
+            auto itemFound = false;
+            // Start from the end to get the latest status
+            for (int i = statusSpy.size() - 1; i >= 0 && !itemFound; --i) {
+                if (QFileInfo(statusSpy.at(i)[0].toString()) == file) {
+                    itemFound = true;
+                    return statusSpy.at(i)[1].value<SyncFileStatus>().tag();
+                }
+            }
+
+            return {};
+        };
+
+        fakeFolder.localModifier().insert(fooFileRootFolder);
+        fakeFolder.localModifier().insert(barFileRootFolder);
+        fakeFolder.localModifier().mkdir(QStringLiteral("subfolder"));
+        fakeFolder.localModifier().insert(fooFileSubFolder);
+        fakeFolder.localModifier().insert(barFileSubFolder);
+        fakeFolder.localModifier().mkdir(QStringLiteral("aaa"));
+        fakeFolder.localModifier().mkdir(QStringLiteral("aaa/subfolder"));
+        fakeFolder.localModifier().insert(fooFileAaaSubFolder);
+        fakeFolder.localModifier().insert(barFileAaaSubFolder);
+        fakeFolder.localModifier().setModTime(barFileAaaSubFolder, QDateTime::fromSecsSinceEpoch(INVALID_MTIME1));
+
+        fakeFolder.scheduleSync();
+        fakeFolder.execUntilBeforePropagation();
+
+        QCOMPARE(checkStatus(), SyncFileStatus::StatusError);
+
+        fakeFolder.execUntilFinished();
+
+        fakeFolder.localModifier().setModTime(barFileAaaSubFolder, QDateTime::fromSecsSinceEpoch(CURRENT_MTIME));
+
+        QVERIFY(fakeFolder.syncOnce());
+
+        fakeFolder.localModifier().appendByte(barFileAaaSubFolder);
+        fakeFolder.localModifier().setModTime(barFileAaaSubFolder, QDateTime::fromSecsSinceEpoch(INVALID_MTIME1));
+
+        fakeFolder.scheduleSync();
+        fakeFolder.execUntilBeforePropagation();
+
+        QCOMPARE(checkStatus(), SyncFileStatus::StatusError);
+
+        fakeFolder.execUntilFinished();
+
+        fakeFolder.localModifier().setModTime(barFileAaaSubFolder, QDateTime::fromSecsSinceEpoch(CURRENT_MTIME));
+
+        QVERIFY(fakeFolder.syncOnce());
+
+        fakeFolder.localModifier().appendByte(barFileAaaSubFolder);
+        fakeFolder.localModifier().setModTime(barFileAaaSubFolder, QDateTime::fromSecsSinceEpoch(INVALID_MTIME2));
+
+        fakeFolder.scheduleSync();
+        fakeFolder.execUntilBeforePropagation();
+
+        QCOMPARE(checkStatus(), SyncFileStatus::StatusError);
+    }
 };
 
 QTEST_GUILESS_MAIN(TestSyncVirtualFiles)