Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash in cldapi.dll #3623

Closed
wants to merge 4 commits into from
Closed

Fix crash in cldapi.dll #3623

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
67dd557
Fix crash in cldapi.dll
mmdanggg2 Jun 18, 2021
752444d
Fix compile error.
allexzander Aug 4, 2021
20155c8
Try not to close handles.
allexzander Aug 6, 2021
2527fcc
Revert "Try not to close handles."
allexzander Aug 6, 2021
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
49 changes: 21 additions & 28 deletions src/libsync/vfs/cfapi/cfapiwrapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,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 {
const auto longpath = OCC::FileSystem::longWinPath(path);
Expand All @@ -606,14 +608,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;
Expand All @@ -622,16 +622,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" };
}
}
Expand Down Expand Up @@ -670,24 +670,20 @@ 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);

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" };
}
Expand All @@ -703,48 +699,45 @@ OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> OCC::CfApiWrapper::up
OCC::Utility::UnixTimeToLargeIntegerFiletime(modtime, &metadata.BasicInfo.LastAccessTime);
OCC::Utility::UnixTimeToLargeIntegerFiletime(modtime, &metadata.BasicInfo.ChangeTime);

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());
qCWarning(lcCfApiWrapper) << "Couldn't update placeholder info for" << path << ":" << QString::fromWCharArray(_com_error(result).ErrorMessage());
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::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;
}
Expand Down
8 changes: 4 additions & 4 deletions src/libsync/vfs/cfapi/cfapiwrapper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,18 +82,18 @@ 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,
Recurse,
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> 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);

}

Expand Down
30 changes: 9 additions & 21 deletions src/libsync/vfs/cfapi/vfs_cfapi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,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 {
Expand Down Expand Up @@ -159,11 +158,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 (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);
}
}

Expand Down Expand Up @@ -218,29 +216,19 @@ 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;
}
}

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 {};
Expand Down
Loading