Merge pull request #8131 from nextcloud/backport/8095/stable-3.16

[stable-3.16] fix: try to correct mtime on upsyncs
This commit is contained in:
Jyrki Gadinger 2025-04-09 09:26:55 +02:00 committed by GitHub
commit a74fa573f0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 149 additions and 62 deletions

View File

@ -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);

View File

@ -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)

View File

@ -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);

View File

@ -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 */

View File

@ -171,10 +171,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;
}
}
}
@ -298,18 +305,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);

View File

@ -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");

View File

@ -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;

View File

@ -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();

View File

@ -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);
}

View File

@ -1333,6 +1333,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";

View File

@ -1751,6 +1751,10 @@ private slots:
return {};
};
const auto lastModified = [&](const QString &path) -> qint64 {
return fakeFolder.currentLocalState().find(path)->lastModified.toSecsSinceEpoch();
};
fakeFolder.localModifier().insert(fooFileRootFolder);
fakeFolder.localModifier().insert(barFileRootFolder);
fakeFolder.localModifier().mkdir(QStringLiteral("subfolder"));
@ -1765,24 +1769,33 @@ private slots:
fakeFolder.scheduleSync();
fakeFolder.execUntilBeforePropagation();
QCOMPARE(checkStatus(), SyncFileStatus::StatusError);
QCOMPARE(checkStatus(), SyncFileStatus::StatusSync);
fakeFolder.execUntilFinished();
// ensure mtime has changed after the sync
QCOMPARE_GT(lastModified(barFileAaaSubFolder), CURRENT_MTIME);
fakeFolder.localModifier().setModTime(barFileAaaSubFolder, QDateTime::fromSecsSinceEpoch(CURRENT_MTIME));
QVERIFY(fakeFolder.syncOnce());
// ensure mtime is now CURRENT_MTIME
QCOMPARE(lastModified(barFileAaaSubFolder), CURRENT_MTIME);
fakeFolder.localModifier().appendByte(barFileAaaSubFolder);
fakeFolder.localModifier().setModTime(barFileAaaSubFolder, QDateTime::fromSecsSinceEpoch(INVALID_MTIME1));
fakeFolder.scheduleSync();
fakeFolder.execUntilBeforePropagation();
QCOMPARE(checkStatus(), SyncFileStatus::StatusError);
QCOMPARE(checkStatus(), SyncFileStatus::StatusSync);
fakeFolder.execUntilFinished();
// ensure mtime has changed after the sync
QCOMPARE_GT(lastModified(barFileAaaSubFolder), CURRENT_MTIME);
fakeFolder.localModifier().setModTime(barFileAaaSubFolder, QDateTime::fromSecsSinceEpoch(CURRENT_MTIME));
QVERIFY(fakeFolder.syncOnce());
@ -1793,7 +1806,11 @@ private slots:
fakeFolder.scheduleSync();
fakeFolder.execUntilBeforePropagation();
QCOMPARE(checkStatus(), SyncFileStatus::StatusError);
QCOMPARE(checkStatus(), SyncFileStatus::StatusSync);
// the server only considers an mtime of 0-86400 (1d) as invalid, so this is fine
// see also: apps/dav/lib/Connector/Sabre/MtimeSanitizer.php
QCOMPARE(lastModified(barFileAaaSubFolder), INVALID_MTIME2);
fakeFolder.execUntilFinished();
}