Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions src/common/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
17 changes: 1 addition & 16 deletions src/common/utility_win.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
46 changes: 24 additions & 22 deletions src/csync/std/c_time.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/csync/std/c_time.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
35 changes: 25 additions & 10 deletions src/libsync/bulkpropagatorjob.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}

Expand Down Expand Up @@ -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);

Expand Down
1 change: 1 addition & 0 deletions src/libsync/discovery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
5 changes: 1 addition & 4 deletions src/libsync/filesystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
11 changes: 9 additions & 2 deletions src/libsync/propagateupload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
3 changes: 2 additions & 1 deletion test/syncenginetestutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that would differ from server behavior
modification time of the server side file should be what is in the header x-oc-mtime

@nilsding nilsding Apr 1, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really -- request.rawHeader only contains the headers of the HTTP request itself, not the multipart-part that's being uploaded (and modtime is set to the value from the multipart headers)

while cross-checking this I just saw that the header used for the modtime variable uses the x-file-mtime multipart header, which has a TODO set on the server side to be replaced with x-oc-mtime... https://github.com/nextcloud/server/blob/c029616ec05f363b28467bbdccd5ce806fd43212/apps/dav/lib/BulkUpload/BulkUploadPlugin.php#L61-L68

remoteRootFileInfo.find(fileName, /*invalidateEtags=*/true);
result.push_back(fileInfo);
}
Expand Down
65 changes: 65 additions & 0 deletions test/testsyncengine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
23 changes: 20 additions & 3 deletions test/testsyncvirtualfiles.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand All @@ -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());
Expand All @@ -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);

@nilsding nilsding Apr 3, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

of course Windows doesn't like that change

lastModified(barFileAaaSubFolder) is -1, which is weird considering I'm running this on a 64-bit system (and Windows has been using a signed 64-bit time value internally since basically forever -- however it's 100ns intervals since Jan 1st 1601). time_t seems to be 64-bit on 64-bit Windows as well; my guess is that we need to update https://github.com/nextcloud/desktop/blob/master/src/csync/std/c_time.cpp#L45 to convert from a 64-bit time_t to a FILETIME type ...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh my, it's even worse than that

struct timeval, used within FileSystem::setModTime, is defined in the winsock.h header of all places and uses two longs for tv_sec and tv_usec -- and longs are 32-bit wide on Windows, even on x64 systems...

on x64 Linux a long is 64-bit wide, same as time_t and all other fields in a struct timeval


fakeFolder.execUntilFinished();
}
Expand Down