Merge pull request #8095 from nextcloud/bugfix/reset-mtime

fix: try to correct mtime on upsyncs

Author: mgallien

src/common/utility.h

@@ -294,9 +294,6 @@ namespace Utility {
     OCSYNC_EXPORT bool registryWalkValues(HKEY hRootKey, const QString &subKey, const std::function<void(const QString &, bool *)> &callback);
     OCSYNC_EXPORT QRect getTaskbarDimensions();
 
-    // Possibly refactor to share code with UnixTimevalToFileTime in c_time.c
-    OCSYNC_EXPORT void UnixTimeToFiletime(time_t t, FILETIME *filetime);
-    OCSYNC_EXPORT void FiletimeToLargeIntegerFiletime(FILETIME *filetime, LARGE_INTEGER *hundredNSecs);
     OCSYNC_EXPORT void UnixTimeToLargeIntegerFiletime(time_t t, LARGE_INTEGER *hundredNSecs);
 
     OCSYNC_EXPORT QString formatWinError(long error);

src/common/utility_win.cpp

@@ -483,24 +483,9 @@ DWORD Utility::convertSizeToDWORD(size_t &convertVar)
     return static_cast<DWORD>(convertVar);
 }
 
-void Utility::UnixTimeToFiletime(time_t t, FILETIME *filetime)
-{
-    LONGLONG ll = Int32x32To64(t, 10000000) + 116444736000000000;
-    filetime->dwLowDateTime = (DWORD) ll;
-    filetime->dwHighDateTime = ll >>32;
-}
-
-void Utility::FiletimeToLargeIntegerFiletime(FILETIME *filetime, LARGE_INTEGER *hundredNSecs)
-{
-    hundredNSecs->LowPart = filetime->dwLowDateTime;
-    hundredNSecs->HighPart = filetime->dwHighDateTime;
-}
-
 void Utility::UnixTimeToLargeIntegerFiletime(time_t t, LARGE_INTEGER *hundredNSecs)
 {
-    LONGLONG ll = Int32x32To64(t, 10000000) + 116444736000000000;
-    hundredNSecs->LowPart = (DWORD) ll;
-    hundredNSecs->HighPart = ll >>32;
+    hundredNSecs->QuadPart = (t * 10000000LL) + 116444736000000000LL;
 }
 
 bool Utility::canCreateFileInPath(const QString &path)

src/csync/std/c_time.cpp

@@ -25,43 +25,45 @@
 #include <QFile>
 
 #ifdef HAVE_UTIMES
-int c_utimes(const QString &uri, const struct timeval *times) {
-    int ret = utimes(QFile::encodeName(uri).constData(), times);
-    return ret;
+int c_utimes(const QString &uri, const time_t time)
+{
+    struct timeval times[2];
+    times[0].tv_sec = times[1].tv_sec = time;
+    times[0].tv_usec = times[1].tv_usec = 0;
+    return utimes(QFile::encodeName(uri).constData(), times);
 }
+
 #else // HAVE_UTIMES
 
 #ifdef _WIN32
-// implementation for utimes taken from KDE mingw headers
+// based on the implementation for utimes from KDE mingw headers
 
 #include <errno.h>
 #include <wtypes.h>
-#define CSYNC_SECONDS_SINCE_1601 11644473600LL
-#define CSYNC_USEC_IN_SEC            1000000LL
-//after Microsoft KB167296
-static void UnixTimevalToFileTime(struct timeval t, LPFILETIME pft)
+
+constexpr long long CSYNC_SECONDS_SINCE_1601 = 11644473600LL;
+constexpr long long CSYNC_USEC_IN_SEC = 1000000LL;
+
+// after Microsoft KB167296, except it uses a `time_t` instead of a `struct timeval`.
+//
+// `struct timeval` is defined in the winsock.h header of all places, and its fields are two `long`s,
+// which even on x64 Windows is 4 bytes wide (i.e. int32).  `time_t` on the other hand is 8 bytes
+// wide (int64) on x64 Windows as well.
+static void UnixTimeToFiletime(const time_t time, LPFILETIME pft)
 {
-    LONGLONG ll = 0;
-    ll = Int32x32To64(t.tv_sec, CSYNC_USEC_IN_SEC*10) + t.tv_usec*10 + CSYNC_SECONDS_SINCE_1601*CSYNC_USEC_IN_SEC*10;
+    LONGLONG ll = time * CSYNC_USEC_IN_SEC * 10 + CSYNC_SECONDS_SINCE_1601 * CSYNC_USEC_IN_SEC * 10;
     pft->dwLowDateTime = (DWORD)ll;
     pft->dwHighDateTime = ll >> 32;
 }
 
-int c_utimes(const QString &uri, const struct timeval *times) {
-    FILETIME LastAccessTime;
-    FILETIME LastModificationTime;
+int c_utimes(const QString &uri, const time_t time)
+{
+    FILETIME filetime;
     HANDLE hFile = nullptr;
 
     auto wuri = uri.toStdWString();
 
-    if(times) {
-        UnixTimevalToFileTime(times[0], &LastAccessTime);
-        UnixTimevalToFileTime(times[1], &LastModificationTime);
-    }
-    else {
-        GetSystemTimeAsFileTime(&LastAccessTime);
-        GetSystemTimeAsFileTime(&LastModificationTime);
-    }
+    UnixTimeToFiletime(time, &filetime);
 
     hFile=CreateFileW(wuri.data(), FILE_WRITE_ATTRIBUTES, FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE,
                       nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL+FILE_FLAG_BACKUP_SEMANTICS, nullptr);
@@ -87,7 +89,7 @@ int c_utimes(const QString &uri, const struct timeval *times) {
         return -1;
     }
 
-    if(!SetFileTime(hFile, nullptr, &LastAccessTime, &LastModificationTime)) {
+    if (!SetFileTime(hFile, nullptr, &filetime, &filetime)) {
         //can this happen?
         errno=ENOENT;
         CloseHandle(hFile);

src/csync/std/c_time.h

@@ -31,7 +31,7 @@
 #include <sys/time.h>
 #endif
 
-OCSYNC_EXPORT int c_utimes(const QString &uri, const struct timeval *times);
+OCSYNC_EXPORT int c_utimes(const QString &uri, time_t time);
 
 
 #endif /* _C_TIME_H */

src/libsync/bulkpropagatorjob.cpp

@@ -195,10 +195,17 @@ void BulkPropagatorJob::doStartUpload(SyncFileItemPtr item,
 
         item->_modtime = FileSystem::getModTime(newFilePathAbsolute);
         if (item->_modtime <= 0) {
-            _pendingChecksumFiles.remove(item->_file);
-            slotOnErrorStartFolderUnlock(item, SyncFileItem::NormalError, tr("File %1 has invalid modified time. Do not upload to the server.").arg(QDir::toNativeSeparators(item->_file)), ErrorCategory::GenericError);
-            checkPropagationIsDone();
-            return;
+            const auto now = QDateTime::currentSecsSinceEpoch();
+            qCInfo(lcPropagateUpload) << "File" << item->_file << "has invalid modification time of" << item->_modtime << "-- trying to update it to" << now;
+            if (FileSystem::setModTime(newFilePathAbsolute, now)) {
+                item->_modtime = now;
+            } else {
+                qCWarning(lcPropagateUpload) << "Could not update modification time for" << item->_file;
+                _pendingChecksumFiles.remove(item->_file);
+                slotOnErrorStartFolderUnlock(item, SyncFileItem::NormalError, tr("File %1 has invalid modified time. Do not upload to the server.").arg(QDir::toNativeSeparators(item->_file)), ErrorCategory::GenericError);
+                checkPropagationIsDone();
+                return;
+            }
         }
     }
 
@@ -325,18 +332,26 @@ void BulkPropagatorJob::slotStartUpload(SyncFileItemPtr item,
         return;
     }
 
-    const auto prevModtime = item->_modtime; // the _item value was set in PropagateUploadFile::start()
+    const auto prevModtime = item->_modtime; // the item value was set in PropagateUploadFile::start()
     // but a potential checksum calculation could have taken some time during which the file could
     // have been changed again, so better check again here.
 
     item->_modtime = FileSystem::getModTime(originalFilePath);
+    qCDebug(lcPropagateUpload) << "fullFilePath" << fullFilePath << "originalFilePath" << originalFilePath << "prevModtime" << prevModtime << "item->_modtime" << item->_modtime;
     if (item->_modtime <= 0) {
-        _pendingChecksumFiles.remove(item->_file);
-        slotOnErrorStartFolderUnlock(item, SyncFileItem::NormalError, tr("File %1 has invalid modification time. Do not upload to the server.").arg(QDir::toNativeSeparators(item->_file)), ErrorCategory::GenericError);
-        checkPropagationIsDone();
-        return;
+        const auto now = QDateTime::currentSecsSinceEpoch();
+        qCInfo(lcPropagateUpload) << "File" << item->_file << "has invalid modification time of" << item->_modtime << "-- trying to update it to" << now;
+        if (FileSystem::setModTime(originalFilePath, now)) {
+            item->_modtime = now;
+        } else {
+            qCWarning(lcPropagateUpload) << "Could not update modification time for" << item->_file;
+            _pendingChecksumFiles.remove(item->_file);
+            slotOnErrorStartFolderUnlock(item, SyncFileItem::NormalError, tr("File %1 has invalid modification time. Do not upload to the server.").arg(QDir::toNativeSeparators(item->_file)), ErrorCategory::GenericError);
+            checkPropagationIsDone();
+            return;
+        }
     }
-    if (prevModtime != item->_modtime) {
+    if (prevModtime > 0 && prevModtime != item->_modtime) {
         propagator()->_anotherSyncNeeded = true;
         _pendingChecksumFiles.remove(item->_file);
 

src/libsync/discovery.cpp

@@ -1121,6 +1121,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
         }
 
         if ((item->_direction == SyncFileItem::Down || item->_instruction == CSYNC_INSTRUCTION_CONFLICT || item->_instruction == CSYNC_INSTRUCTION_NEW || item->_instruction == CSYNC_INSTRUCTION_SYNC) &&
+                item->_direction != SyncFileItem::Up &&
                 (item->_modtime <= 0 || item->_modtime >= 0xFFFFFFFF)) {
             item->_instruction = CSYNC_INSTRUCTION_ERROR;
             item->_errorString = tr("Cannot sync due to invalid modification time");

src/libsync/filesystem.cpp

@@ -196,10 +196,7 @@ time_t FileSystem::getModTime(const QString &filename)
 
 bool FileSystem::setModTime(const QString &filename, time_t modTime)
 {
-    struct timeval times[2];
-    times[0].tv_sec = times[1].tv_sec = modTime;
-    times[0].tv_usec = times[1].tv_usec = 0;
-    int rc = c_utimes(filename, times);
+    int rc = c_utimes(filename, modTime);
     if (rc != 0) {
         qCWarning(lcFileSystem) << "Error setting mtime for" << filename
                                 << "failed: rc" << rc << ", errno:" << errno;

src/libsync/propagateupload.cpp

@@ -324,8 +324,15 @@ void PropagateUploadFileCommon::slotComputeContentChecksum()
     // probably temporary one.
     _item->_modtime = FileSystem::getModTime(filePath);
     if (_item->_modtime <= 0) {
-        slotOnErrorStartFolderUnlock(SyncFileItem::NormalError, tr("File %1 has invalid modification time. Do not upload to the server.").arg(QDir::toNativeSeparators(_item->_file)));
-        return;
+        const auto now = QDateTime::currentSecsSinceEpoch();
+        qCInfo(lcPropagateUpload) << "File" << _item->_file << "has invalid modification time of" << _item->_modtime << "-- trying to update it to" << now;
+        if (FileSystem::setModTime(filePath, now)) {
+            _item->_modtime = now;
+        } else {
+            qCWarning(lcPropagateUpload) << "Could not update modification time for" << _item->_file;
+            slotOnErrorStartFolderUnlock(SyncFileItem::NormalError, tr("File %1 has invalid modification time. Do not upload to the server.").arg(QDir::toNativeSeparators(_item->_file)));
+            return;
+        }
     }
 
     const QByteArray checksumType = propagator()->account()->capabilities().preferredUploadChecksumType();

test/syncenginetestutils.cpp

@@ -535,6 +535,7 @@ FakePutMultiFileReply::FakePutMultiFileReply(FileInfo &remoteRootFileInfo, QNetw
 
 QVector<FileInfo *> FakePutMultiFileReply::performMultiPart(FileInfo &remoteRootFileInfo, const QNetworkRequest &request, const QByteArray &putPayload, const QString &contentType)
 {
+    Q_UNUSED(request)
     QVector<FileInfo *> result;
 
     auto stringPutPayload = QString::fromUtf8(putPayload);
@@ -564,7 +565,7 @@ QVector<FileInfo *> FakePutMultiFileReply::performMultiPart(FileInfo &remoteRoot
             // Assume that the file is filled with the same character
             fileInfo = remoteRootFileInfo.create(fileName, onePartBody.size(), onePartBody.at(0).toLatin1());
         }
-        fileInfo->lastModified = OCC::Utility::qDateTimeFromTime_t(request.rawHeader("x-oc-mtime").toLongLong());
+        fileInfo->lastModified = OCC::Utility::qDateTimeFromTime_t(modtime);
         remoteRootFileInfo.find(fileName, /*invalidateEtags=*/true);
         result.push_back(fileInfo);
     }

test/testsyncengine.cpp

@@ -1411,6 +1411,71 @@ private slots:
         QCOMPARE(fakeFolder.currentRemoteState(), expectedState);
     }
 
+    void testLocalInvalidMtimeCorrection()
+    {
+        const auto INVALID_MTIME = QDateTime::fromSecsSinceEpoch(0);
+        const auto RECENT_MTIME = QDateTime::fromSecsSinceEpoch(1743004783); // 2025-03-26T16:59:43+0100
+
+        FakeFolder fakeFolder{FileInfo{}};
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+
+        fakeFolder.localModifier().insert(QStringLiteral("invalid"));
+        fakeFolder.localModifier().setModTime("invalid", INVALID_MTIME);
+        fakeFolder.localModifier().insert(QStringLiteral("recent"));
+        fakeFolder.localModifier().setModTime("recent", RECENT_MTIME);
+
+        QVERIFY(fakeFolder.syncOnce());
+
+        // "invalid" file had a mtime of 0, so it's been updated to the current time during testing
+        const auto currentMtime = fakeFolder.currentLocalState().find("invalid")->lastModified;
+        QCOMPARE_GT(currentMtime, RECENT_MTIME);
+        QCOMPARE_GT(fakeFolder.currentRemoteState().find("invalid")->lastModified, RECENT_MTIME);
+
+        // "recent" file had a mtime of RECENT_MTIME, so it shouldn't have been changed
+        QCOMPARE(fakeFolder.currentLocalState().find("recent")->lastModified, RECENT_MTIME);
+        QCOMPARE(fakeFolder.currentRemoteState().find("recent")->lastModified, RECENT_MTIME);
+
+        QVERIFY(fakeFolder.syncOnce());
+
+        // verify that the mtime of "invalid" hasn't changed since the last sync that fixed it
+        QCOMPARE(fakeFolder.currentLocalState().find("invalid")->lastModified, currentMtime);
+
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+    }
+
+    void testLocalInvalidMtimeCorrectionBulkUpload()
+    {
+        const auto INVALID_MTIME = QDateTime::fromSecsSinceEpoch(0);
+        const auto RECENT_MTIME = QDateTime::fromSecsSinceEpoch(1743004783); // 2025-03-26T16:59:43+0100
+
+        FakeFolder fakeFolder{FileInfo{}};
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+        fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"bulkupload", "1.0"} } } });
+
+        fakeFolder.localModifier().insert(QStringLiteral("invalid"));
+        fakeFolder.localModifier().setModTime("invalid", INVALID_MTIME);
+        fakeFolder.localModifier().insert(QStringLiteral("recent"));
+        fakeFolder.localModifier().setModTime("recent", RECENT_MTIME);
+
+        QVERIFY(fakeFolder.syncOnce()); // this will use the BulkPropagatorJob
+
+        // "invalid" file had a mtime of 0, so it's been updated to the current time during testing
+        const auto currentMtime = fakeFolder.currentLocalState().find("invalid")->lastModified;
+        QCOMPARE_GT(currentMtime, RECENT_MTIME);
+        QCOMPARE_GT(fakeFolder.currentRemoteState().find("invalid")->lastModified, RECENT_MTIME);
+
+        // "recent" file had a mtime of RECENT_MTIME, so it shouldn't have been changed
+        QCOMPARE(fakeFolder.currentLocalState().find("recent")->lastModified, RECENT_MTIME);
+        QCOMPARE(fakeFolder.currentRemoteState().find("recent")->lastModified, RECENT_MTIME);
+
+        QVERIFY(fakeFolder.syncOnce()); // this will not propagate anything
+
+        // verify that the mtime of "invalid" hasn't changed since the last sync that fixed it
+        QCOMPARE(fakeFolder.currentLocalState().find("invalid")->lastModified, currentMtime);
+
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+    }
+
     void testServerUpdatingMTimeShouldNotCreateConflicts()
     {
         constexpr auto testFile = "test.txt";