Fix crash in cldapi.dll

Fixes #3349

Change placeholder functions to take QStrings instead of handles

Signed-off-by: James Horsley <jbhorsley@gmail.com>

diff --git a/src/libsync/vfs/cfapi/cfapiwrapper.cpp b/src/libsync/vfs/cfapi/cfapiwrapper.cpp
index 9f0fb85fd4fb3b4a8b04e4e4cf17a1c070c9efeb..4975ca881b07f96c6681d3b8d1f65d737e5a2568 100644 (file)
--- a/src/libsync/vfs/cfapi/cfapiwrapper.cpp
+++ b/src/libsync/vfs/cfapi/cfapiwrapper.cpp
@@ -649,6 +649,8 @@ OCC::CfApiWrapper::FileHandle OCC::CfApiWrapper::handleForPath(const QString &pa
         const qint64 openResult = CfOpenFileWithOplock(path.toStdWString().data(), CF_OPEN_FILE_FLAG_NONE, &handle);
         if (openResult == S_OK) {
             return {handle, [](HANDLE h) { CfCloseHandle(h); }};
+        } else {
+            qCWarning(lcCfApiWrapper) << "Could not open handle for " << path << " result: " << QString::fromWCharArray(_com_error(openResult).ErrorMessage());
         }
     } else if (pathFileInfo.isFile()) {
         const auto longpath = OCC::FileSystem::longWinPath(path);
@@ -664,14 +666,12 @@ OCC::CfApiWrapper::FileHandle OCC::CfApiWrapper::handleForPath(const QString &pa
     return {};
 }
 
-OCC::CfApiWrapper::PlaceHolderInfo OCC::CfApiWrapper::findPlaceholderInfo(const FileHandle &handle)
+OCC::CfApiWrapper::PlaceHolderInfo OCC::CfApiWrapper::findPlaceholderInfo(const QString &path)
 {
-    Q_ASSERT(handle);
-
     constexpr auto fileIdMaxLength = 128;
     const auto infoSize = sizeof(CF_PLACEHOLDER_BASIC_INFO) + fileIdMaxLength;
     auto info = PlaceHolderInfo(reinterpret_cast<CF_PLACEHOLDER_BASIC_INFO *>(new char[infoSize]), deletePlaceholderInfo);
-    const qint64 result = CfGetPlaceholderInfo(handle.get(), CF_PLACEHOLDER_INFO_BASIC, info.get(), sizeToDWORD(infoSize), nullptr);
+    const qint64 result = CfGetPlaceholderInfo(handleForPath(path).get(), CF_PLACEHOLDER_INFO_BASIC, info.get(), sizeToDWORD(infoSize), nullptr);
 
     if (result == S_OK) {
         return info;
@@ -680,16 +680,16 @@ OCC::CfApiWrapper::PlaceHolderInfo OCC::CfApiWrapper::findPlaceholderInfo(const
     }
 }
 
-OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> OCC::CfApiWrapper::setPinState(const FileHandle &handle, OCC::PinStateEnums::PinState state, SetPinRecurseMode mode)
+OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> OCC::CfApiWrapper::setPinState(const QString &path, OCC::PinStateEnums::PinState state, SetPinRecurseMode mode)
 {
     const auto cfState = pinStateToCfPinState(state);
     const auto flags = pinRecurseModeToCfSetPinFlags(mode);
 
-    const qint64 result = CfSetPinState(handle.get(), cfState, flags, nullptr);
+    const qint64 result = CfSetPinState(handleForPath(path).get(), cfState, flags, nullptr);
     if (result == S_OK) {
         return OCC::Vfs::ConvertToPlaceholderResult::Ok;
     } else {
-        qCWarning(lcCfApiWrapper) << "Couldn't set pin state" << state << "for" << pathForHandle(handle) << "with recurse mode" << mode << ":" << QString::fromWCharArray(_com_error(result).ErrorMessage());
+        qCWarning(lcCfApiWrapper) << "Couldn't set pin state" << state << "for" << path << "with recurse mode" << mode << ":" << QString::fromWCharArray(_com_error(result).ErrorMessage());
         return { "Couldn't set pin state" };
     }
 }
@@ -732,28 +732,25 @@ OCC::Result<void, QString> OCC::CfApiWrapper::createPlaceholderInfo(const QStrin
         return { "Couldn't create placeholder info" };
     }
 
-    const auto parentHandle = handleForPath(QDir::toNativeSeparators(QFileInfo(path).absolutePath()));
-    const auto parentInfo = findPlaceholderInfo(parentHandle);
+    const auto parentInfo = findPlaceholderInfo(QDir::toNativeSeparators(QFileInfo(path).absolutePath()));
     const auto state = parentInfo && parentInfo->PinState == CF_PIN_STATE_UNPINNED ? CF_PIN_STATE_UNPINNED : CF_PIN_STATE_INHERIT;
 
-    const auto handle = handleForPath(path);
-    if (!setPinState(handle, cfPinStateToPinState(state), NoRecurse)) {
+    if (!setPinState(path, cfPinStateToPinState(state), NoRecurse)) {
         return { "Couldn't set the default inherit pin state" };
     }
 
     return {};
 }
 
-OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> OCC::CfApiWrapper::updatePlaceholderInfo(const FileHandle &handle, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath)
+OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> OCC::CfApiWrapper::updatePlaceholderInfo(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath)
 {
-    Q_ASSERT(handle);
 
     if (modtime <= 0) {
-        return {QString{"Could not update metadata due to invalid modification time for %1: %2"}.arg(pathForHandle(handle)).arg(modtime)};
+        return {QString{"Could not update metadata due to invalid modification time for %1: %2"}.arg(path).arg(modtime)};
     }
 
-    const auto info = replacesPath.isEmpty() ? findPlaceholderInfo(handle)
-                                             : findPlaceholderInfo(handleForPath(replacesPath));
+    const auto info = replacesPath.isEmpty() ? findPlaceholderInfo(path)
+                                             : findPlaceholderInfo(replacesPath);
     if (!info) {
         return { "Can't update non existing placeholder info" };
     }
@@ -770,32 +767,30 @@ OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> OCC::CfApiWrapper::up
     OCC::Utility::UnixTimeToLargeIntegerFiletime(modtime, &metadata.BasicInfo.ChangeTime);
     metadata.BasicInfo.FileAttributes = 0;
 
-    const qint64 result = CfUpdatePlaceholder(handle.get(), &metadata,
+    const qint64 result = CfUpdatePlaceholder(handleForPath(path).get(), &metadata,
                                               fileIdentity.data(), sizeToDWORD(fileIdentitySize),
                                               nullptr, 0, CF_UPDATE_FLAG_MARK_IN_SYNC, nullptr, nullptr);
 
     if (result != S_OK) {
-        qCWarning(lcCfApiWrapper) << "Couldn't update placeholder info for" << pathForHandle(handle) << ":" << QString::fromWCharArray(_com_error(result).ErrorMessage()) << replacesPath;
+        qCWarning(lcCfApiWrapper) << "Couldn't update placeholder info for" << path << ":" << QString::fromWCharArray(_com_error(result).ErrorMessage()) << replacesPath;
         return { "Couldn't update placeholder info" };
     }
 
     // Pin state tends to be lost on updates, so restore it every time
-    if (!setPinState(handle, previousPinState, NoRecurse)) {
+    if (!setPinState(path, previousPinState, NoRecurse)) {
         return { "Couldn't restore pin state" };
     }
 
     return OCC::Vfs::ConvertToPlaceholderResult::Ok;
 }
 
-OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> OCC::CfApiWrapper::dehydratePlaceholder(const FileHandle &handle, time_t modtime, qint64 size, const QByteArray &fileId)
+OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> OCC::CfApiWrapper::dehydratePlaceholder(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId)
 {
-    Q_ASSERT(handle);
-
     if (modtime <= 0) {
-        return {QString{"Could not update metadata due to invalid modification time for %1: %2"}.arg(pathForHandle(handle)).arg(modtime)};
+        return {QString{"Could not update metadata due to invalid modification time for %1: %2"}.arg(path).arg(modtime)};
     }
 
-    const auto info = findPlaceholderInfo(handle);
+    const auto info = findPlaceholderInfo(path);
     if (!info) {
         return { "Can't update non existing placeholder info" };
     }
@@ -807,43 +802,40 @@ OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> OCC::CfApiWrapper::de
     dehydrationRange.StartingOffset.QuadPart = 0;
     dehydrationRange.Length.QuadPart = size;
 
-    const qint64 result = CfUpdatePlaceholder(handle.get(), nullptr,
+    const qint64 result = CfUpdatePlaceholder(handleForPath(path).get(), nullptr,
                                               fileIdentity.data(), sizeToDWORD(fileIdentitySize),
                                               &dehydrationRange, 1, CF_UPDATE_FLAG_MARK_IN_SYNC, nullptr, nullptr);
 
     if (result != S_OK) {
-        qCWarning(lcCfApiWrapper) << "Couldn't update placeholder info for" << pathForHandle(handle) << ":" << QString::fromWCharArray(_com_error(result).ErrorMessage());
+        qCWarning(lcCfApiWrapper) << "Couldn't update placeholder info for" << path << ":" << QString::fromWCharArray(_com_error(result).ErrorMessage());
         return { "Couldn't update placeholder info" };
     }
 
     return OCC::Vfs::ConvertToPlaceholderResult::Ok;
 }
 
-OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> OCC::CfApiWrapper::convertToPlaceholder(const FileHandle &handle, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath)
+OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> OCC::CfApiWrapper::convertToPlaceholder(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath)
 {
     Q_UNUSED(modtime);
     Q_UNUSED(size);
 
-    Q_ASSERT(handle);
-
     const auto fileIdentity = QString::fromUtf8(fileId).toStdWString();
     const auto fileIdentitySize = (fileIdentity.length() + 1) * sizeof(wchar_t);
-    const qint64 result = CfConvertToPlaceholder(handle.get(), fileIdentity.data(), sizeToDWORD(fileIdentitySize), CF_CONVERT_FLAG_MARK_IN_SYNC, nullptr, nullptr);
+    const qint64 result = CfConvertToPlaceholder(handleForPath(path).get(), fileIdentity.data(), sizeToDWORD(fileIdentitySize), CF_CONVERT_FLAG_MARK_IN_SYNC, nullptr, nullptr);
     Q_ASSERT(result == S_OK);
     if (result != S_OK) {
-        qCCritical(lcCfApiWrapper) << "Couldn't convert to placeholder" << pathForHandle(handle) << ":" << QString::fromWCharArray(_com_error(result).ErrorMessage());
+        qCCritical(lcCfApiWrapper) << "Couldn't convert to placeholder" << path << ":" << QString::fromWCharArray(_com_error(result).ErrorMessage());
         return { "Couldn't convert to placeholder" };
     }
 
-    const auto originalHandle = handleForPath(replacesPath);
-    const auto originalInfo = originalHandle ? findPlaceholderInfo(originalHandle) : PlaceHolderInfo(nullptr, deletePlaceholderInfo);
+    const auto originalInfo = findPlaceholderInfo(replacesPath);
     if (!originalInfo) {
-        const auto stateResult = setPinState(handle, PinState::Inherited, NoRecurse);
+        const auto stateResult = setPinState(path, PinState::Inherited, NoRecurse);
         Q_ASSERT(stateResult);
         return stateResult;
     } else {
         const auto state = cfPinStateToPinState(originalInfo->PinState);
-        const auto stateResult = setPinState(handle, state, NoRecurse);
+        const auto stateResult = setPinState(path, state, NoRecurse);
         Q_ASSERT(stateResult);
         return stateResult;
     }

diff --git a/src/libsync/vfs/cfapi/cfapiwrapper.h b/src/libsync/vfs/cfapi/cfapiwrapper.h
index 2e9bbee273f879d891cc1b68388477ae2ee1e9b6..10cb51aaa82bb212f384f3bbd96decdb7f384fc2 100644 (file)
--- a/src/libsync/vfs/cfapi/cfapiwrapper.h
+++ b/src/libsync/vfs/cfapi/cfapiwrapper.h
@@ -84,7 +84,7 @@ NEXTCLOUD_CFAPI_EXPORT bool isSparseFile(const QString &path);
 
 NEXTCLOUD_CFAPI_EXPORT FileHandle handleForPath(const QString &path);
 
-PlaceHolderInfo findPlaceholderInfo(const FileHandle &handle);
+PlaceHolderInfo findPlaceholderInfo(const QString &path);
 
 enum SetPinRecurseMode {
     NoRecurse = 0,
@@ -92,11 +92,11 @@ enum SetPinRecurseMode {
     ChildrenOnly
 };
 
-NEXTCLOUD_CFAPI_EXPORT Result<OCC::Vfs::ConvertToPlaceholderResult, QString> setPinState(const FileHandle &handle, PinState state, SetPinRecurseMode mode);
+NEXTCLOUD_CFAPI_EXPORT Result<OCC::Vfs::ConvertToPlaceholderResult, QString> setPinState(const QString &path, PinState state, SetPinRecurseMode mode);
 NEXTCLOUD_CFAPI_EXPORT Result<void, QString> createPlaceholderInfo(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId);
-NEXTCLOUD_CFAPI_EXPORT Result<OCC::Vfs::ConvertToPlaceholderResult, QString> updatePlaceholderInfo(const FileHandle &handle, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath = QString());
-NEXTCLOUD_CFAPI_EXPORT Result<OCC::Vfs::ConvertToPlaceholderResult, QString> convertToPlaceholder(const FileHandle &handle, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath);
-NEXTCLOUD_CFAPI_EXPORT Result<OCC::Vfs::ConvertToPlaceholderResult, QString> dehydratePlaceholder(const FileHandle &handle, time_t modtime, qint64 size, const QByteArray &fileId);
+NEXTCLOUD_CFAPI_EXPORT Result<OCC::Vfs::ConvertToPlaceholderResult, QString> updatePlaceholderInfo(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath = QString());
+NEXTCLOUD_CFAPI_EXPORT Result<OCC::Vfs::ConvertToPlaceholderResult, QString> convertToPlaceholder(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId, const QString &replacesPath);
+NEXTCLOUD_CFAPI_EXPORT Result<OCC::Vfs::ConvertToPlaceholderResult, QString> dehydratePlaceholder(const QString &path, time_t modtime, qint64 size, const QByteArray &fileId);
 
 }
 

diff --git a/src/libsync/vfs/cfapi/vfs_cfapi.cpp b/src/libsync/vfs/cfapi/vfs_cfapi.cpp
index db0d3653c118b2a56c555b772c270b3efec7235f..d201e36c4ac36130f4a5827d375b19004292ad83 100644 (file)
--- a/src/libsync/vfs/cfapi/vfs_cfapi.cpp
+++ b/src/libsync/vfs/cfapi/vfs_cfapi.cpp
@@ -172,9 +172,8 @@ bool VfsCfApi::isHydrating() const
 Result<void, QString> VfsCfApi::updateMetadata(const QString &filePath, time_t modtime, qint64 size, const QByteArray &fileId)
 {
     const auto localPath = QDir::toNativeSeparators(filePath);
-    const auto handle = cfapi::handleForPath(localPath);
-    if (handle) {
-        auto result = cfapi::updatePlaceholderInfo(handle, modtime, size, fileId);
+    if (cfapi::handleForPath(localPath)) {
+        auto result = cfapi::updatePlaceholderInfo(localPath, modtime, size, fileId);
         if (result) {
             return {};
         } else {
@@ -197,9 +196,8 @@ Result<void, QString> VfsCfApi::createPlaceholder(const SyncFileItem &item)
 Result<void, QString> VfsCfApi::dehydratePlaceholder(const SyncFileItem &item)
 {
     const auto localPath = QDir::toNativeSeparators(_setupParams.filesystemPath + item._file);
-    const auto handle = cfapi::handleForPath(localPath);
-    if (handle) {
-        auto result = cfapi::dehydratePlaceholder(handle, item._modtime, item._size, item._fileId);
+    if (cfapi::handleForPath(localPath)) {
+        auto result = cfapi::dehydratePlaceholder(localPath, item._modtime, item._size, item._fileId);
         if (result) {
             return {};
         } else {
@@ -216,14 +214,10 @@ Result<Vfs::ConvertToPlaceholderResult, QString> VfsCfApi::convertToPlaceholder(
     const auto localPath = QDir::toNativeSeparators(filename);
     const auto replacesPath = QDir::toNativeSeparators(replacesFile);
 
-    const auto handle = cfapi::handleForPath(localPath);
-    if (!handle) {
-        return { "Invalid handle for path " + localPath };
-    }
-    if (cfapi::findPlaceholderInfo(handle)) {
-        return cfapi::updatePlaceholderInfo(handle, item._modtime, item._size, item._fileId, replacesPath);
+    if (cfapi::findPlaceholderInfo(localPath)) {
+        return cfapi::updatePlaceholderInfo(localPath, item._modtime, item._size, item._fileId, replacesPath);
     } else {
-        return cfapi::convertToPlaceholder(handle, item._modtime, item._size, item._fileId, replacesPath);
+        return cfapi::convertToPlaceholder(localPath, item._modtime, item._size, item._fileId, replacesPath);
     }
 }
 
@@ -278,15 +272,10 @@ bool VfsCfApi::statTypeVirtualFile(csync_file_stat_t *stat, void *statData)
 bool VfsCfApi::setPinState(const QString &folderPath, PinState state)
 {
     const auto localPath = QDir::toNativeSeparators(params().filesystemPath + folderPath);
-    const auto handle = cfapi::handleForPath(localPath);
-    if (handle) {
-        if (cfapi::setPinState(handle, state, cfapi::Recurse)) {
-            return true;
-        } else {
-            return false;
-        }
+
+    if (cfapi::setPinState(localPath, state, cfapi::Recurse)) {
+        return true;
     } else {
-        qCWarning(lcCfApi) << "Couldn't update pin state for non existing file" << localPath;
         return false;
     }
 }
@@ -294,13 +283,8 @@ bool VfsCfApi::setPinState(const QString &folderPath, PinState state)
 Optional<PinState> VfsCfApi::pinState(const QString &folderPath)
 {
     const auto localPath = QDir::toNativeSeparators(params().filesystemPath + folderPath);
-    const auto handle = cfapi::handleForPath(localPath);
-    if (!handle) {
-        qCWarning(lcCfApi) << "Couldn't find pin state for non existing file" << localPath;
-        return {};
-    }
 
-    const auto info = cfapi::findPlaceholderInfo(handle);
+    const auto info = cfapi::findPlaceholderInfo(localPath);
     if (!info) {
         qCWarning(lcCfApi) << "Couldn't find pin state for regular non-placeholder file" << localPath;
         return {};