From efe686992e32aedf670c5b286725451f639f7f15 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Wed, 30 Oct 2024 16:37:42 +0100 Subject: [PATCH 01/10] do not move to trash online only files/folders: going to fail ensure that even if move to trash is enabled, we do not erroneously try to move online only files or folders to trash bin Signed-off-by: Matthieu Gallien --- src/libsync/propagatorjobs.cpp | 3 ++- src/libsync/vfs/cfapi/vfs_cfapi.cpp | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index bcdf5a4697..8b397f033e 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -117,7 +117,8 @@ void PropagateLocalRemove::start() } QString removeError; - if (_moveToTrash) { + const auto availability = propagator()->syncOptions()._vfs->availability(_item->_file, Vfs::AvailabilityRecursivity::RecursiveAvailability); + if (_moveToTrash && (!availability || (*availability != VfsItemAvailability::AllDehydrated && *availability != VfsItemAvailability::OnlineOnly && *availability != VfsItemAvailability::Mixed))) { if ((QDir(filename).exists() || FileSystem::fileExists(filename)) && !FileSystem::moveToTrash(filename, &removeError)) { done(SyncFileItem::NormalError, removeError, ErrorCategory::GenericError); diff --git a/src/libsync/vfs/cfapi/vfs_cfapi.cpp b/src/libsync/vfs/cfapi/vfs_cfapi.cpp index 414caed1bf..7ae45a6d28 100644 --- a/src/libsync/vfs/cfapi/vfs_cfapi.cpp +++ b/src/libsync/vfs/cfapi/vfs_cfapi.cpp @@ -511,7 +511,7 @@ int VfsCfApi::finalizeHydrationJob(const QString &requestId) VfsCfApi::HydratationAndPinStates VfsCfApi::computeRecursiveHydrationAndPinStates(const QString &folderPath, const Optional &basePinState) { Q_ASSERT(!folderPath.endsWith('/')); - const auto fullPath = params().filesystemPath + folderPath; + const auto fullPath = QString{params().filesystemPath + folderPath}; QFileInfo info(params().filesystemPath + folderPath); if (!FileSystem::fileExists(fullPath)) { From 1f21a934db9c8a36323e621d7702b1e1ca682051 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Wed, 30 Oct 2024 18:48:53 +0100 Subject: [PATCH 02/10] fix computation of availability for empty folders Signed-off-by: Matthieu Gallien --- src/libsync/vfs/cfapi/vfs_cfapi.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libsync/vfs/cfapi/vfs_cfapi.cpp b/src/libsync/vfs/cfapi/vfs_cfapi.cpp index 7ae45a6d28..64779407f2 100644 --- a/src/libsync/vfs/cfapi/vfs_cfapi.cpp +++ b/src/libsync/vfs/cfapi/vfs_cfapi.cpp @@ -372,6 +372,14 @@ Vfs::AvailabilityResult VfsCfApi::availability(const QString &folderPath, const return VfsItemAvailability::AlwaysLocal; else return VfsItemAvailability::AllHydrated; + } else { + if (pin && *pin == PinState::OnlineOnly) { + return VfsItemAvailability::OnlineOnly; + } else if (pin && *pin == PinState::AlwaysLocal) { + return VfsItemAvailability::AlwaysLocal; + } else { + return VfsItemAvailability::AllDehydrated; + } } return AvailabilityError::NoSuchItem; } From 3c021166fadddc2eaea4d8584a780b1972f6d392 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Tue, 5 Nov 2024 16:29:53 +0100 Subject: [PATCH 03/10] use proper item type when converting existing windows shortcuts we need to make shortcut files be proper virtual placeholder files let the sync engine use the proper item type for that Signed-off-by: Matthieu Gallien --- src/libsync/discovery.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 7300a4c5f9..3fe2fb2d82 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -1193,11 +1193,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( item->_direction = SyncFileItem::Down; item->_instruction = CSYNC_INSTRUCTION_SYNC; const auto pinState = _discoveryData->_syncOptions._vfs->pinState(path._local); - if (FileSystem::isLnkFile(path._local) && !_discoveryData->_syncOptions._vfs->pinState(path._local).isValid()) { - item->_type = ItemTypeVirtualFileDownload; - } else { - item->_type = ItemTypeVirtualFileDehydration; - } + item->_type = ItemTypeVirtualFileDehydration; } else if (!serverModified && (dbEntry._inode != localEntry.inode || (localEntry.isMetadataMissing && item->_type == ItemTypeFile && !FileSystem::isLnkFile(item->_file)) From 8ac293bfd44fa93081b2db88dad63abf6dae7e24 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Tue, 5 Nov 2024 17:32:53 +0100 Subject: [PATCH 04/10] fix migration of existing windows shortcut files to placeholder we cannot do a single call to convert the plain old file to be a dehydrated virtual file placeholder split the conversion into a separate initial API call fix transition from releases < 3.14 to current releases that have first class support for windows shortcuts (so called link files) Signed-off-by: Matthieu Gallien --- src/libsync/propagatedownload.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index 9fcc3988fb..b6474ea54d 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -498,6 +498,13 @@ void PropagateDownloadFile::startAfterIsEncryptedIsChecked() } qCDebug(lcPropagateDownload) << "dehydrating file" << _item->_file; + if (FileSystem::isLnkFile(fsPath)) { + const auto convertResult = vfs->convertToPlaceholder(fsPath, *_item); + if (!convertResult) { + qCCritical(lcPropagateDownload()) << "error when converting a shortcut file to placeholder" << convertResult.error(); + } + } + auto r = vfs->dehydratePlaceholder(*_item); if (!r) { done(SyncFileItem::NormalError, r.error(), ErrorCategory::GenericError); From caf1d492b19f6364776b9113e665eb38a2b45d2c Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Tue, 5 Nov 2024 17:38:04 +0100 Subject: [PATCH 05/10] provide proper log when we block a self hydration request should make the existing log more useful when tracking the self requested implicit hydration bugs Signed-off-by: Matthieu Gallien --- src/libsync/vfs/cfapi/cfapiwrapper.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libsync/vfs/cfapi/cfapiwrapper.cpp b/src/libsync/vfs/cfapi/cfapiwrapper.cpp index a483d2c066..abe72f1916 100644 --- a/src/libsync/vfs/cfapi/cfapiwrapper.cpp +++ b/src/libsync/vfs/cfapi/cfapiwrapper.cpp @@ -157,17 +157,17 @@ void CALLBACK cfApiFetchDataCallback(const CF_CALLBACK_INFO *callbackInfo, const callbackInfo->FileSize.QuadPart); }; - if (QCoreApplication::applicationPid() == callbackInfo->ProcessInfo->ProcessId) { - qCCritical(lcCfApiWrapper) << "implicit hydration triggered by the client itself. Will lead to a deadlock. Cancel"; - sendTransferError(); - return; - } - auto vfs = reinterpret_cast(callbackInfo->CallbackContext); Q_ASSERT(vfs->metaObject()->className() == QByteArrayLiteral("OCC::VfsCfApi")); const auto path = QString(QString::fromWCharArray(callbackInfo->VolumeDosName) + QString::fromWCharArray(callbackInfo->NormalizedPath)); const auto requestId = QString::number(callbackInfo->TransferKey.QuadPart, 16); + if (QCoreApplication::applicationPid() == callbackInfo->ProcessInfo->ProcessId) { + qCCritical(lcCfApiWrapper) << "implicit hydration triggered by the client itself. Will lead to a deadlock. Cancel" << path << requestId; + sendTransferError(); + return; + } + qCDebug(lcCfApiWrapper) << "Request hydration for" << path << requestId; const auto invokeResult = QMetaObject::invokeMethod(vfs, [=] { vfs->requestHydration(requestId, path); }, Qt::QueuedConnection); From b6b0a9d4a33e167fa73bf412cb2e6ba511fb496d Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Tue, 5 Nov 2024 17:39:28 +0100 Subject: [PATCH 06/10] prevent implicit hydration when setting file permissions on shortcuts we were using specialized API to set the file permissions on the windows shortcut and later falling back to the generic code path due to the mising return Signed-off-by: Matthieu Gallien --- src/common/filesystembase.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/common/filesystembase.cpp b/src/common/filesystembase.cpp index 765b081bff..8371cc3360 100644 --- a/src/common/filesystembase.cpp +++ b/src/common/filesystembase.cpp @@ -131,6 +131,7 @@ void FileSystem::setFileReadOnly(const QString &filename, bool readonly) { qCWarning(lcFileSystem()) << filename << (readonly ? "readonly" : "read write") << e.what(); } + return; } #endif QFile file(filename); From 9c23be343420e3fe85f84cf98da955786f381d44 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Tue, 5 Nov 2024 17:43:10 +0100 Subject: [PATCH 07/10] never use QFileInfo on possibly pure virtual placeholder file will prevent some type of activity being added to the list from triggering implicit hydration during synchronization most probably breaks existing open actions on those activities Signed-off-by: Matthieu Gallien --- src/gui/tray/activitylistmodel.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gui/tray/activitylistmodel.cpp b/src/gui/tray/activitylistmodel.cpp index f2fc26ba1e..47cc441f6c 100644 --- a/src/gui/tray/activitylistmodel.cpp +++ b/src/gui/tray/activitylistmodel.cpp @@ -275,9 +275,9 @@ QVariant ActivityListModel::data(const QModelIndex &index, int role) const case DisplayPathRole: return getDisplayPath(); case PathRole: - return QFileInfo(getFilePath()).path(); + return getFilePath(); case OpenablePathRole: - return a._isMultiObjectActivity ? QFileInfo(getFilePath()).canonicalPath() : QFileInfo(getFilePath()).canonicalFilePath(); + return getFilePath(); case DisplayLocationRole: return displayLocation(); case ActionsLinksRole: { From d705cad6b9c7817daa553824005cda54b9045e71 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Tue, 5 Nov 2024 17:45:20 +0100 Subject: [PATCH 08/10] improve logs for CfAPI handle requests in erroneous cases Signed-off-by: Matthieu Gallien --- src/libsync/vfs/cfapi/cfapiwrapper.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libsync/vfs/cfapi/cfapiwrapper.cpp b/src/libsync/vfs/cfapi/cfapiwrapper.cpp index abe72f1916..ed214b88f9 100644 --- a/src/libsync/vfs/cfapi/cfapiwrapper.cpp +++ b/src/libsync/vfs/cfapi/cfapiwrapper.cpp @@ -781,10 +781,12 @@ bool OCC::CfApiWrapper::isSparseFile(const QString &path) OCC::CfApiWrapper::FileHandle OCC::CfApiWrapper::handleForPath(const QString &path) { if (path.isEmpty()) { + qCWarning(lcCfApiWrapper) << "empty path"; return {}; } if (!FileSystem::fileExists(path)) { + qCWarning(lcCfApiWrapper) << "file does not exist"; return {}; } @@ -807,6 +809,7 @@ OCC::CfApiWrapper::FileHandle OCC::CfApiWrapper::handleForPath(const QString &pa } } + qCWarning(lcCfApiWrapper) << "no handle was created"; return {}; } From 2d0529c423d61d07406e6979a2a6a83373abf8fb Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Thu, 7 Nov 2024 16:47:45 +0100 Subject: [PATCH 09/10] remote permissions of a view-only folder: no new file or folder change the way we evaluate a folder permissions to be read-only when no new file or new folders can be created if those permissions are lacking, client will make the local folder read-only if those permissions are granted, client will make local folder read-write Signed-off-by: Matthieu Gallien --- src/libsync/propagatorjobs.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index 8b397f033e..b57a4ede46 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -229,8 +229,6 @@ void PropagateLocalMkdir::startLocalMkdir() #if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 if (!_item->_remotePerm.isNull() && !_item->_remotePerm.hasPermission(RemotePermissions::CanAddFile) && - !_item->_remotePerm.hasPermission(RemotePermissions::CanRename) && - !_item->_remotePerm.hasPermission(RemotePermissions::CanMove) && !_item->_remotePerm.hasPermission(RemotePermissions::CanAddSubDirectories)) { try { FileSystem::setFolderPermissions(newDirStr, FileSystem::FolderPermissions::ReadOnly); From 85e71d47bfa64c3293afc2b88eff7e72dbfce3c3 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Tue, 5 Nov 2024 23:09:00 +0100 Subject: [PATCH 10/10] ensure lnk shortcut files are converted to virtual files Signed-off-by: Matthieu Gallien --- src/libsync/discovery.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 3fe2fb2d82..f4a98b5182 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -1731,6 +1731,15 @@ void ProcessDirectoryJob::processFileFinalize( item->_instruction = CSyncEnums::CSYNC_INSTRUCTION_UPDATE_VFS_METADATA; } + if (_discoveryData->_syncOptions._vfs && + (item->_type == CSyncEnums::ItemTypeFile || item->_type == CSyncEnums::ItemTypeDirectory) && + item->_instruction == CSyncEnums::CSYNC_INSTRUCTION_NONE && + FileSystem::isLnkFile((_discoveryData->_localDir + path._local))) { + item->_instruction = CSyncEnums::CSYNC_INSTRUCTION_SYNC; + item->_direction = SyncFileItem::Down; + item->_type = CSyncEnums::ItemTypeVirtualFileDehydration; + } + if (path._original != path._target && (item->_instruction == CSYNC_INSTRUCTION_UPDATE_VFS_METADATA || item->_instruction == CSYNC_INSTRUCTION_UPDATE_METADATA || item->_instruction == CSYNC_INSTRUCTION_NONE)) { ASSERT(_dirItem && _dirItem->_instruction == CSYNC_INSTRUCTION_RENAME); // This is because otherwise subitems are not updated! (ideally renaming a directory could