Skip to content

Commit 89487f4

Browse files
nilsdingnilsding
committed
fix: try to correct mtime on upsyncs
Files with a modification time of less than 0 do usually not make sense (and afaik the server doesn't accept them either). --> attempt to update the modification time to _Time.now_ while propagating side note: I ran into this because KArchive/Ark(?) didn't consider the extra time attributes on entries for a certain zip file, so it instead used the standard time value of each zip entry which was set to <= 1980 for files and < 1970 for directories... Signed-off-by: Jyrki Gadinger <nilsding@nilsding.org>
1 parent 4594733 commit 89487f4

6 files changed

Lines changed: 123 additions & 16 deletions

File tree

‎src/libsync/bulkpropagatorjob.cpp‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -195,10 +195,17 @@ void BulkPropagatorJob::doStartUpload(SyncFileItemPtr item,
195195

196196
item->_modtime = FileSystem::getModTime(newFilePathAbsolute);
197197
if (item->_modtime <= 0) {
198-
_pendingChecksumFiles.remove(item->_file);
199-
slotOnErrorStartFolderUnlock(item, SyncFileItem::NormalError, tr("File %1 has invalid modified time. Do not upload to the server.").arg(QDir::toNativeSeparators(item->_file)), ErrorCategory::GenericError);
200-
checkPropagationIsDone();
201-
return;
198+
const auto now = QDateTime::currentSecsSinceEpoch();
199+
qCInfo(lcPropagateUpload) << "File" << item->_file << "has invalid modification time of" << item->_modtime << "-- trying to update it to" << now;
200+
if (FileSystem::setModTime(newFilePathAbsolute, now)) {
201+
item->_modtime = now;
202+
} else {
203+
qCWarning(lcPropagateUpload) << "Could not update modification time for" << item->_file;
204+
_pendingChecksumFiles.remove(item->_file);
205+
slotOnErrorStartFolderUnlock(item, SyncFileItem::NormalError, tr("File %1 has invalid modified time. Do not upload to the server.").arg(QDir::toNativeSeparators(item->_file)), ErrorCategory::GenericError);
206+
checkPropagationIsDone();
207+
return;
208+
}
202209
}
203210
}
204211

@@ -325,18 +332,26 @@ void BulkPropagatorJob::slotStartUpload(SyncFileItemPtr item,
325332
return;
326333
}
327334

328-
const auto prevModtime = item->_modtime; // the _item value was set in PropagateUploadFile::start()
335+
const auto prevModtime = item->_modtime; // the item value was set in PropagateUploadFile::start()
329336
// but a potential checksum calculation could have taken some time during which the file could
330337
// have been changed again, so better check again here.
331338

332339
item->_modtime = FileSystem::getModTime(originalFilePath);
340+
qCDebug(lcPropagateUpload) << "fullFilePath" << fullFilePath << "originalFilePath" << originalFilePath << "prevModtime" << prevModtime << "item->_modtime" << item->_modtime;
333341
if (item->_modtime <= 0) {
334-
_pendingChecksumFiles.remove(item->_file);
335-
slotOnErrorStartFolderUnlock(item, SyncFileItem::NormalError, tr("File %1 has invalid modification time. Do not upload to the server.").arg(QDir::toNativeSeparators(item->_file)), ErrorCategory::GenericError);
336-
checkPropagationIsDone();
337-
return;
342+
const auto now = QDateTime::currentSecsSinceEpoch();
343+
qCInfo(lcPropagateUpload) << "File" << item->_file << "has invalid modification time of" << item->_modtime << "-- trying to update it to" << now;
344+
if (FileSystem::setModTime(originalFilePath, now)) {
345+
item->_modtime = now;
346+
} else {
347+
qCWarning(lcPropagateUpload) << "Could not update modification time for" << item->_file;
348+
_pendingChecksumFiles.remove(item->_file);
349+
slotOnErrorStartFolderUnlock(item, SyncFileItem::NormalError, tr("File %1 has invalid modification time. Do not upload to the server.").arg(QDir::toNativeSeparators(item->_file)), ErrorCategory::GenericError);
350+
checkPropagationIsDone();
351+
return;
352+
}
338353
}
339-
if (prevModtime != item->_modtime) {
354+
if (prevModtime > 0 && prevModtime != item->_modtime) {
340355
propagator()->_anotherSyncNeeded = true;
341356
_pendingChecksumFiles.remove(item->_file);
342357

‎src/libsync/discovery.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,6 +1121,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
11211121
}
11221122

11231123
if ((item->_direction == SyncFileItem::Down || item->_instruction == CSYNC_INSTRUCTION_CONFLICT || item->_instruction == CSYNC_INSTRUCTION_NEW || item->_instruction == CSYNC_INSTRUCTION_SYNC) &&
1124+
item->_direction != SyncFileItem::Up &&
11241125
(item->_modtime <= 0 || item->_modtime >= 0xFFFFFFFF)) {
11251126
item->_instruction = CSYNC_INSTRUCTION_ERROR;
11261127
item->_errorString = tr("Cannot sync due to invalid modification time");

‎src/libsync/propagateupload.cpp‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -324,8 +324,15 @@ void PropagateUploadFileCommon::slotComputeContentChecksum()
324324
// probably temporary one.
325325
_item->_modtime = FileSystem::getModTime(filePath);
326326
if (_item->_modtime <= 0) {
327-
slotOnErrorStartFolderUnlock(SyncFileItem::NormalError, tr("File %1 has invalid modification time. Do not upload to the server.").arg(QDir::toNativeSeparators(_item->_file)));
328-
return;
327+
const auto now = QDateTime::currentSecsSinceEpoch();
328+
qCInfo(lcPropagateUpload) << "File" << _item->_file << "has invalid modification time of" << _item->_modtime << "-- trying to update it to" << now;
329+
if (FileSystem::setModTime(filePath, now)) {
330+
_item->_modtime = now;
331+
} else {
332+
qCWarning(lcPropagateUpload) << "Could not update modification time for" << _item->_file;
333+
slotOnErrorStartFolderUnlock(SyncFileItem::NormalError, tr("File %1 has invalid modification time. Do not upload to the server.").arg(QDir::toNativeSeparators(_item->_file)));
334+
return;
335+
}
329336
}
330337

331338
const QByteArray checksumType = propagator()->account()->capabilities().preferredUploadChecksumType();

‎test/syncenginetestutils.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,7 @@ FakePutMultiFileReply::FakePutMultiFileReply(FileInfo &remoteRootFileInfo, QNetw
539539

540540
QVector<FileInfo *> FakePutMultiFileReply::performMultiPart(FileInfo &remoteRootFileInfo, const QNetworkRequest &request, const QByteArray &putPayload, const QString &contentType)
541541
{
542+
Q_UNUSED(request)
542543
QVector<FileInfo *> result;
543544

544545
auto stringPutPayload = QString::fromUtf8(putPayload);
@@ -568,7 +569,7 @@ QVector<FileInfo *> FakePutMultiFileReply::performMultiPart(FileInfo &remoteRoot
568569
// Assume that the file is filled with the same character
569570
fileInfo = remoteRootFileInfo.create(fileName, onePartBody.size(), onePartBody.at(0).toLatin1());
570571
}
571-
fileInfo->lastModified = OCC::Utility::qDateTimeFromTime_t(request.rawHeader("x-oc-mtime").toLongLong());
572+
fileInfo->lastModified = OCC::Utility::qDateTimeFromTime_t(modtime);
572573
remoteRootFileInfo.find(fileName, /*invalidateEtags=*/true);
573574
result.push_back(fileInfo);
574575
}
@@ -589,6 +590,7 @@ void FakePutMultiFileReply::respond()
589590
QJsonObject fileInfoReply;
590591
fileInfoReply.insert("error", QStringLiteral("false"));
591592
fileInfoReply.insert("etag", QLatin1String{fileInfo->etag});
593+
fileInfoReply.insert("x-oc-mtime", QLatin1String("accepted"));
592594
emit uploadProgress(fileInfo->size, totalSize);
593595
allFileInfoReply.insert(QChar('/') + fileInfo->path(), fileInfoReply);
594596
}

‎test/testsyncengine.cpp‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1411,6 +1411,71 @@ private slots:
14111411
QCOMPARE(fakeFolder.currentRemoteState(), expectedState);
14121412
}
14131413

1414+
void testLocalInvalidMtimeCorrection()
1415+
{
1416+
const auto INVALID_MTIME = QDateTime::fromSecsSinceEpoch(0);
1417+
const auto RECENT_MTIME = QDateTime::fromSecsSinceEpoch(1743004783); // 2025-03-26T16:59:43+0100
1418+
1419+
FakeFolder fakeFolder{FileInfo{}};
1420+
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
1421+
1422+
fakeFolder.localModifier().insert(QStringLiteral("invalid"));
1423+
fakeFolder.localModifier().setModTime("invalid", INVALID_MTIME);
1424+
fakeFolder.localModifier().insert(QStringLiteral("recent"));
1425+
fakeFolder.localModifier().setModTime("recent", RECENT_MTIME);
1426+
1427+
QVERIFY(fakeFolder.syncOnce());
1428+
1429+
// "invalid" file had a mtime of 0, so it's been updated to the current time during testing
1430+
const auto currentMtime = fakeFolder.currentLocalState().find("invalid")->lastModified;
1431+
QCOMPARE_GT(currentMtime, RECENT_MTIME);
1432+
QCOMPARE_GT(fakeFolder.currentRemoteState().find("invalid")->lastModified, RECENT_MTIME);
1433+
1434+
// "recent" file had a mtime of RECENT_MTIME, so it shouldn't have been changed
1435+
QCOMPARE(fakeFolder.currentLocalState().find("recent")->lastModified, RECENT_MTIME);
1436+
QCOMPARE(fakeFolder.currentRemoteState().find("recent")->lastModified, RECENT_MTIME);
1437+
1438+
QVERIFY(fakeFolder.syncOnce());
1439+
1440+
// verify that the mtime of "invalid" hasn't changed since the last sync that fixed it
1441+
QCOMPARE(fakeFolder.currentLocalState().find("invalid")->lastModified, currentMtime);
1442+
1443+
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
1444+
}
1445+
1446+
void testLocalInvalidMtimeCorrectionBulkUpload()
1447+
{
1448+
const auto INVALID_MTIME = QDateTime::fromSecsSinceEpoch(0);
1449+
const auto RECENT_MTIME = QDateTime::fromSecsSinceEpoch(1743004783); // 2025-03-26T16:59:43+0100
1450+
1451+
FakeFolder fakeFolder{FileInfo{}};
1452+
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
1453+
fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"bulkupload", "1.0"} } } });
1454+
1455+
fakeFolder.localModifier().insert(QStringLiteral("invalid"));
1456+
fakeFolder.localModifier().setModTime("invalid", INVALID_MTIME);
1457+
fakeFolder.localModifier().insert(QStringLiteral("recent"));
1458+
fakeFolder.localModifier().setModTime("recent", RECENT_MTIME);
1459+
1460+
QVERIFY(fakeFolder.syncOnce()); // this will use the BulkPropagatorJob
1461+
1462+
// "invalid" file had a mtime of 0, so it's been updated to the current time during testing
1463+
const auto currentMtime = fakeFolder.currentLocalState().find("invalid")->lastModified;
1464+
QCOMPARE_GT(currentMtime, RECENT_MTIME);
1465+
QCOMPARE_GT(fakeFolder.currentRemoteState().find("invalid")->lastModified, RECENT_MTIME);
1466+
1467+
// "recent" file had a mtime of RECENT_MTIME, so it shouldn't have been changed
1468+
QCOMPARE(fakeFolder.currentLocalState().find("recent")->lastModified, RECENT_MTIME);
1469+
QCOMPARE(fakeFolder.currentRemoteState().find("recent")->lastModified, RECENT_MTIME);
1470+
1471+
QVERIFY(fakeFolder.syncOnce()); // this will not propagate anything
1472+
1473+
// verify that the mtime of "invalid" hasn't changed since the last sync that fixed it
1474+
QCOMPARE(fakeFolder.currentLocalState().find("invalid")->lastModified, currentMtime);
1475+
1476+
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
1477+
}
1478+
14141479
void testServerUpdatingMTimeShouldNotCreateConflicts()
14151480
{
14161481
constexpr auto testFile = "test.txt";

‎test/testsyncvirtualfiles.cpp‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1751,6 +1751,10 @@ private slots:
17511751
return {};
17521752
};
17531753

1754+
const auto lastModified = [&](const QString &path) -> qint64 {
1755+
return fakeFolder.currentLocalState().find(path)->lastModified.toSecsSinceEpoch();
1756+
};
1757+
17541758
fakeFolder.localModifier().insert(fooFileRootFolder);
17551759
fakeFolder.localModifier().insert(barFileRootFolder);
17561760
fakeFolder.localModifier().mkdir(QStringLiteral("subfolder"));
@@ -1765,24 +1769,33 @@ private slots:
17651769
fakeFolder.scheduleSync();
17661770
fakeFolder.execUntilBeforePropagation();
17671771

1768-
QCOMPARE(checkStatus(), SyncFileStatus::StatusError);
1772+
QCOMPARE(checkStatus(), SyncFileStatus::StatusSync);
17691773

17701774
fakeFolder.execUntilFinished();
17711775

1776+
// ensure mtime has changed after the sync
1777+
QCOMPARE_GT(lastModified(barFileAaaSubFolder), CURRENT_MTIME);
1778+
17721779
fakeFolder.localModifier().setModTime(barFileAaaSubFolder, QDateTime::fromSecsSinceEpoch(CURRENT_MTIME));
17731780

17741781
QVERIFY(fakeFolder.syncOnce());
17751782

1783+
// ensure mtime is now CURRENT_MTIME
1784+
QCOMPARE(lastModified(barFileAaaSubFolder), CURRENT_MTIME);
1785+
17761786
fakeFolder.localModifier().appendByte(barFileAaaSubFolder);
17771787
fakeFolder.localModifier().setModTime(barFileAaaSubFolder, QDateTime::fromSecsSinceEpoch(INVALID_MTIME1));
17781788

17791789
fakeFolder.scheduleSync();
17801790
fakeFolder.execUntilBeforePropagation();
17811791

1782-
QCOMPARE(checkStatus(), SyncFileStatus::StatusError);
1792+
QCOMPARE(checkStatus(), SyncFileStatus::StatusSync);
17831793

17841794
fakeFolder.execUntilFinished();
17851795

1796+
// ensure mtime has changed after the sync
1797+
QCOMPARE_GT(lastModified(barFileAaaSubFolder), CURRENT_MTIME);
1798+
17861799
fakeFolder.localModifier().setModTime(barFileAaaSubFolder, QDateTime::fromSecsSinceEpoch(CURRENT_MTIME));
17871800

17881801
QVERIFY(fakeFolder.syncOnce());
@@ -1793,7 +1806,11 @@ private slots:
17931806
fakeFolder.scheduleSync();
17941807
fakeFolder.execUntilBeforePropagation();
17951808

1796-
QCOMPARE(checkStatus(), SyncFileStatus::StatusError);
1809+
QCOMPARE(checkStatus(), SyncFileStatus::StatusSync);
1810+
1811+
// the server only considers an mtime of 0-86400 (1d) as invalid, so this is fine
1812+
// see also: apps/dav/lib/Connector/Sabre/MtimeSanitizer.php
1813+
QCOMPARE(lastModified(barFileAaaSubFolder), INVALID_MTIME2);
17971814

17981815
fakeFolder.execUntilFinished();
17991816
}

0 commit comments

Comments
 (0)