Skip to content

[stable-34.0] Bugfix/mgallien/improve new folder in windows vfs handling #11061

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

Merged
mgallien merged 3 commits into from
Oct 14, 2026
Merged

[stable-34.0] Bugfix/mgallien/improve new folder in windows vfs handling #11061

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ccb7a86
chore: fix format of Windows VFS files
mgallien Oct 13, 2026
7d96ba5
fix(vfs/windows): properly detect partially synced folders
mgallien Oct 13, 2026
b123cb4
test(vfs/windows): check population flags of synced folders
mgallien Oct 13, 2026
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
366 changes: 216 additions & 150 deletions src/libsync/vfs/cfapi/cfapiwrapper.cpp
View file
Open in desktop

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion src/libsync/vfs/cfapi/shellext/dllmain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
#include "cfapishellintegrationclassfactory.h"
#include "customstateprovider.h"
#include "thumbnailprovider.h"
#include <comdef.h>

#include <ntstatus.h>

long dllReferenceCount = 0;
long dllObjectsCount = 0;
Expand Down
143 changes: 83 additions & 60 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 @@ -9,22 +9,23 @@
#include <QFile>

#include "cfapiwrapper.h"
#include "hydrationjob.h"
#include "syncfileitem.h"
#include "filesystem.h"
#include "common/filesystembase.h"
#include "common/syncjournaldb.h"
#include "config.h"
#include "filesystem.h"
#include "hydrationjob.h"
#include "syncfileitem.h"

#include <ntstatus.h>
#include <cfapi.h>
#include <comdef.h>
#include <ntstatus.h>

#include <QCoreApplication>

Q_LOGGING_CATEGORY(lcCfApi, "nextcloud.sync.vfs.cfapi", QtInfoMsg)

namespace cfapi {
namespace cfapi
{
using namespace OCC::CfApiWrapper;

constexpr auto appIdRegKey = R"(Software\Classes\AppID\)";
Expand All @@ -35,11 +36,11 @@ bool registerShellExtension()
{
const QList<QPair<QString, QString>> listExtensions = {
{CFAPI_SHELLEXT_THUMBNAIL_HANDLER_DISPLAY_NAME, CFAPI_SHELLEXT_THUMBNAIL_HANDLER_CLASS_ID_REG},
{CFAPI_SHELLEXT_CUSTOM_STATE_HANDLER_DISPLAY_NAME, CFAPI_SHELLEXT_CUSTOM_STATE_HANDLER_CLASS_ID_REG}
};
{CFAPI_SHELLEXT_CUSTOM_STATE_HANDLER_DISPLAY_NAME, CFAPI_SHELLEXT_CUSTOM_STATE_HANDLER_CLASS_ID_REG}};
// assume CFAPI_SHELL_EXTENSIONS_LIB_NAME is always in the same folder as the main executable
// assume CFAPI_SHELL_EXTENSIONS_LIB_NAME is always in the same folder as the main executable
const auto shellExtensionDllPath = QDir::toNativeSeparators(QString(QCoreApplication::applicationDirPath() + QStringLiteral("/") + CFAPI_SHELL_EXTENSIONS_LIB_NAME + QStringLiteral(".dll")));
const auto shellExtensionDllPath = QDir::toNativeSeparators(
QString(QCoreApplication::applicationDirPath() + QStringLiteral("/") + CFAPI_SHELL_EXTENSIONS_LIB_NAME + QStringLiteral(".dll")));
if (!OCC::FileSystem::fileExists(shellExtensionDllPath)) {
Q_ASSERT(false);
qCWarning(lcCfApi) << "Register CfAPI shell extensions failed. Dll does not exist in " << QCoreApplication::applicationDirPath();
Expand Down Expand Up @@ -82,10 +83,7 @@ void unregisterShellExtensions()
OCC::Utility::registryDeleteKeyTree(rootKey, appIdPath);
}

const QStringList listExtensions = {
CFAPI_SHELLEXT_CUSTOM_STATE_HANDLER_CLASS_ID_REG,
CFAPI_SHELLEXT_THUMBNAIL_HANDLER_CLASS_ID_REG
};
const QStringList listExtensions = {CFAPI_SHELLEXT_CUSTOM_STATE_HANDLER_CLASS_ID_REG, CFAPI_SHELLEXT_THUMBNAIL_HANDLER_CLASS_ID_REG};

for (const auto &extension : listExtensions) {
const QString clsidPath = QString() % clsIdRegKey % extension;
Expand All @@ -97,7 +95,8 @@ void unregisterShellExtensions()

}

namespace OCC {
namespace OCC
{

class VfsCfApiPrivate
{
Expand Down Expand Up @@ -129,7 +128,13 @@ void VfsCfApi::startImpl(const VfsSetupParams &params)
cfapi::registerShellExtension();
const auto localPath = QDir::toNativeSeparators(params.filesystemPath);

const auto registerResult = cfapi::registerSyncRoot(localPath, params.providerName, params.providerVersion, params.alias, params.navigationPaneClsid, params.displayName, params.account->displayName());
const auto registerResult = cfapi::registerSyncRoot(localPath,
params.providerName,
params.providerVersion,
params.alias,
params.navigationPaneClsid,
params.displayName,
params.account->displayName());
if (!registerResult) {
qCCritical(lcCfApi) << "Initialization failed, couldn't register sync root:" << registerResult.error();
return;
Expand Down Expand Up @@ -177,7 +182,8 @@ bool VfsCfApi::isHydrating() const
return !d->hydrationJobs.isEmpty();
}

OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString> VfsCfApi::updateMetadata(const SyncFileItem &syncItem, const QString &filePath, const QString &replacesFile)
OCC::Result<OCC::Vfs::ConvertToPlaceholderResult, QString>
VfsCfApi::updateMetadata(const SyncFileItem &syncItem, const QString &filePath, const QString &replacesFile)
{
const auto localPath = QDir::toNativeSeparators(filePath);
const auto replacesPath = QDir::toNativeSeparators(replacesFile);
Expand Down Expand Up @@ -219,7 +225,12 @@ Result<void, QString> VfsCfApi::createPlaceholders(const QList<SyncFileItemPtr>
auto allPlaceholdersInfo = QList<cfapi::PlaceholdersInfo>{};
for (const auto &oneItem : items) {
auto fileInfo = QFileInfo(_setupParams.filesystemPath + oneItem->_file);
allPlaceholdersInfo.emplace_back(std::move(fileInfo), fileInfo.fileName(), QDir::toNativeSeparators(fileInfo.fileName()).toStdWString(), oneItem->_fileId, oneItem->_modtime, oneItem->_size);
allPlaceholdersInfo.emplace_back(std::move(fileInfo),
fileInfo.fileName(),
QDir::toNativeSeparators(fileInfo.fileName()).toStdWString(),
oneItem->_fileId,
oneItem->_modtime,
oneItem->_size);
}

const auto result = cfapi::createPlaceholdersInfo(localPath, {allPlaceholdersInfo});
Expand All @@ -243,7 +254,8 @@ Result<void, QString> VfsCfApi::dehydratePlaceholder(const SyncFileItem &item)
}
}

Result<Vfs::ConvertToPlaceholderResult, QString> VfsCfApi::convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile, UpdateMetadataTypes updateType)
Result<Vfs::ConvertToPlaceholderResult, QString>
VfsCfApi::convertToPlaceholder(const QString &filename, const SyncFileItem &item, const QString &replacesFile, UpdateMetadataTypes updateType)
{
const auto localPath = FileSystem::longWinPath(QDir::toNativeSeparators(filename));
const auto replacesPath = FileSystem::longWinPath(QDir::toNativeSeparators(replacesFile));
Expand Down Expand Up @@ -290,8 +302,25 @@ bool VfsCfApi::statTypeVirtualFile(csync_file_stat_t *stat, void *statData)
// it is a file... let's prevent it
if (isDirectory) {
if (hasCloudTag) {
auto placeholderState = CfGetPlaceholderStateFromAttributeTag(ffd->dwFileAttributes, ffd->dwReserved0);

#if defined NEXTCLOUD_DEV
qCDebug(lcCfApi()) << stat->path << (CF_PLACEHOLDER_STATE_PLACEHOLDER & placeholderState ? "CF_PLACEHOLDER_STATE_PLACEHOLDER" : "NONE")
<< (CF_PLACEHOLDER_STATE_SYNC_ROOT & placeholderState ? "CF_PLACEHOLDER_STATE_SYNC_ROOT" : "NONE")
<< (CF_PLACEHOLDER_STATE_ESSENTIAL_PROP_PRESENT & placeholderState ? "CF_PLACEHOLDER_STATE_ESSENTIAL_PROP_PRESENT" : "NONE")
<< (CF_PLACEHOLDER_STATE_IN_SYNC & placeholderState ? "CF_PLACEHOLDER_STATE_IN_SYNC" : "NONE")
<< (CF_PLACEHOLDER_STATE_PARTIAL & placeholderState ? "CF_PLACEHOLDER_STATE_PARTIAL" : "NONE")
<< (CF_PLACEHOLDER_STATE_PARTIALLY_ON_DISK & placeholderState ? "CF_PLACEHOLDER_STATE_PARTIALLY_ON_DISK" : "NONE")
<< (CF_PLACEHOLDER_STATE_INVALID & placeholderState ? "CF_PLACEHOLDER_STATE_INVALID" : "NONE");
#endif

ffd->dwFileAttributes &= ~FILE_ATTRIBUTE_REPARSE_POINT;
stat->type = CSyncEnums::ItemTypeVirtualDirectory;
if (CF_PLACEHOLDER_STATE_PARTIALLY_ON_DISK & placeholderState) {
stat->type = CSyncEnums::ItemTypeVirtualDirectory;
} else {
stat->type = CSyncEnums::ItemTypeDirectory;
}

return true;
}
return false;
Expand Down Expand Up @@ -349,8 +378,7 @@ Vfs::AvailabilityResult VfsCfApi::availability(const QString &folderPath, const
{
const auto basePinState = pinState(folderPath);
if (basePinState && recursiveCheck == Vfs::AvailabilityRecursivity::NotRecursiveAvailability) {
switch (*basePinState)
{
switch (*basePinState) {
case OCC::PinState::AlwaysLocal:
return VfsItemAvailability::AlwaysLocal;
break;
Expand All @@ -372,17 +400,20 @@ Vfs::AvailabilityResult VfsCfApi::availability(const QString &folderPath, const
const auto hydrationStatus = hydrationAndPinStates.hydrationStatus;

if (hydrationStatus.hasDehydrated) {
if (hydrationStatus.hasHydrated)
if (hydrationStatus.hasHydrated) {
return VfsItemAvailability::Mixed;
if (pin && *pin == PinState::OnlineOnly)
}
if (pin && *pin == PinState::OnlineOnly) {
return VfsItemAvailability::OnlineOnly;
else
} else {
return VfsItemAvailability::AllDehydrated;
}
} else if (hydrationStatus.hasHydrated) {
if (pin && *pin == PinState::AlwaysLocal)
if (pin && *pin == PinState::AlwaysLocal) {
return VfsItemAvailability::AlwaysLocal;
else
} else {
return VfsItemAvailability::AllHydrated;
}
} else {
if (pin && *pin == PinState::OnlineOnly) {
return VfsItemAvailability::OnlineOnly;
Expand Down Expand Up @@ -530,14 +561,12 @@ int VfsCfApi::finalizeHydrationJob(const QString &requestId)
return HydrationJob::Status::Error;
}

int VfsCfApi::finalizeNewPlaceholders(const QList<PlaceholderCreateInfo> &newEntries,
const QString &pathString)
int VfsCfApi::finalizeNewPlaceholders(const QList<PlaceholderCreateInfo> &newEntries, const QString &pathString)
{
const auto &journal = params().journal;

for (const auto &entryInfo : newEntries) {
if (entryInfo.creationStatus == VirtualItemCreationStatus::Error ||
entryInfo.creationStatus == VirtualItemCreationStatus::Unknown) {
if (entryInfo.creationStatus == VirtualItemCreationStatus::Error || entryInfo.creationStatus == VirtualItemCreationStatus::Unknown) {
qCWarning(lcCfApi()) << "do not create DB record for virtual item not created" << entryInfo.fullPath;
continue;
}
Expand All @@ -557,7 +586,8 @@ int VfsCfApi::finalizeNewPlaceholders(const QList<PlaceholderCreateInfo> &newEnt
(entryInfo.parsedProperties.isDirectory ? ItemTypeVirtualDirectory
: (FileSystem::isExcludeFile(entryInfo.fullPath) ? ItemTypeVirtualFileDownload : ItemTypeVirtualFile));
folderRecord._etag = entryInfo.parsedProperties.etag;
folderRecord._e2eEncryptionStatus = static_cast<SyncJournalFileRecord::EncryptionStatus>(entryInfo.parsedProperties.isE2eEncrypted() ? SyncFileItem::EncryptionStatus::EncryptedMigratedV2_0 : SyncFileItem::EncryptionStatus::NotEncrypted);
folderRecord._e2eEncryptionStatus = static_cast<SyncJournalFileRecord::EncryptionStatus>(
entryInfo.parsedProperties.isE2eEncrypted() ? SyncFileItem::EncryptionStatus::EncryptedMigratedV2_0 : SyncFileItem::EncryptionStatus::NotEncrypted);
folderRecord._lockstate._locked = (entryInfo.parsedProperties.locked == SyncFileItemEnums::LockStatus::LockedItem);
folderRecord._lockstate._lockOwnerDisplayName = entryInfo.parsedProperties.lockOwnerDisplayName;
folderRecord._lockstate._lockOwnerId = entryInfo.parsedProperties.lockOwnerId;
Expand Down Expand Up @@ -617,43 +647,36 @@ VfsCfApi::HydratationAndPinStates VfsCfApi::computeRecursiveHydrationAndPinState
}
const auto effectivePin = pinState(folderPath);
const auto pinResult = (!effectivePin && !basePinState) ? Optional<PinState>()
: (!effectivePin || !basePinState) ? PinState::Inherited
: (*effectivePin == *basePinState) ? *effectivePin
: PinState::Inherited;
: (!effectivePin || !basePinState) ? PinState::Inherited
: (*effectivePin == *basePinState) ? *effectivePin
: PinState::Inherited;

if (FileSystem::isDir(fullPath)) {
const auto dirState = HydratationAndPinStates {
pinResult,
{}
};
const auto dirState = HydratationAndPinStates{pinResult, {}};
const auto dir = QDir(info.absoluteFilePath());
Q_ASSERT(dir.exists());
const auto children = dir.entryList();
return std::accumulate(std::cbegin(children), std::cend(children), dirState, [=, this](const HydratationAndPinStates &currentState, const QString &name) {
if (name == QStringLiteral("..") || name == QStringLiteral(".")) {
return currentState;
}

// if the folderPath.isEmpty() we don't want to end up having path "/example.file" because this will lead to double slash later, when appending to "SyncFolder/"
const auto path = folderPath.isEmpty() ? name : folderPath + '/' + name;
const auto states = computeRecursiveHydrationAndPinStates(path, currentState.pinState);
return HydratationAndPinStates {
states.pinState,
{
states.hydrationStatus.hasHydrated || currentState.hydrationStatus.hasHydrated,
states.hydrationStatus.hasDehydrated || currentState.hydrationStatus.hasDehydrated,
}
};
});
return std::accumulate(std::cbegin(children),
std::cend(children),
dirState,
[=, this](const HydratationAndPinStates &currentState, const QString &name) {
if (name == QStringLiteral("..") || name == QStringLiteral(".")) {
return currentState;
}

// if the folderPath.isEmpty() we don't want to end up having path "/example.file" because this will lead to double slash
// later, when appending to "SyncFolder/"
const auto path = folderPath.isEmpty() ? name : folderPath + '/' + name;
const auto states = computeRecursiveHydrationAndPinStates(path, currentState.pinState);
return HydratationAndPinStates{states.pinState,
{
states.hydrationStatus.hasHydrated || currentState.hydrationStatus.hasHydrated,
states.hydrationStatus.hasDehydrated || currentState.hydrationStatus.hasDehydrated,
}};
});
} else { // file case
const auto isDehydrated = isDehydratedPlaceholder(info.absoluteFilePath());
return {
pinResult,
{
!isDehydrated,
isDehydrated
}
};
return {pinResult, {!isDehydrated, isDehydrated}};
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/libsync/vfs/cfapi/vfs_cfapi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@
*/
#pragma once

#include <QObject>

Check failure on line 7 in src/libsync/vfs/cfapi/vfs_cfapi.h

View workflow job for this annotation

GitHub Actions / build

src/libsync/vfs/cfapi/vfs_cfapi.h:7:10 [clang-diagnostic-error] 

'QObject' file not found
#include <QScopedPointer>

#include "common/vfs.h"
#include "common/plugin.h"

#include <ntstatus.h>

namespace OCC {
class HydrationJob;
class VfsCfApiPrivate;
Expand Down
1 change: 1 addition & 0 deletions test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ if (WIN32)
)

nextcloud_add_test(SyncCfApi)
target_link_libraries(SyncCfApiTest PRIVATE cldapi)
nextcloud_add_test(CfApiShellExtensionsIPC)
target_sources(CfApiShellExtensionsIPCTest PRIVATE "${CMAKE_SOURCE_DIR}/src/libsync/vfs/cfapi/shellext/thumbnailprovideripc.cpp" "${CMAKE_SOURCE_DIR}/src/libsync/vfs/cfapi/shellext/customstateprovideripc.cpp" "${CMAKE_SOURCE_DIR}/src/libsync/vfs/cfapi/shellext/ipccommon.cpp")
elseif(LINUX) # elseif(LINUX OR APPLE)
Expand Down
73 changes: 73 additions & 0 deletions test/testsynccfapi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,17 @@ SyncJournalFileRecord dbRecord(FakeFolder &folder, const QString &path)
return record;
}

CF_PLACEHOLDER_STATE placeholderState(const QString &path)
{
WIN32_FIND_DATAW findData = {};
const auto handle = FindFirstFileW(reinterpret_cast<const wchar_t *>(QDir::toNativeSeparators(path).utf16()), &findData);
if (handle == INVALID_HANDLE_VALUE) {
return CF_PLACEHOLDER_STATE_INVALID;
}
FindClose(handle);
return CfGetPlaceholderStateFromAttributeTag(findData.dwFileAttributes, findData.dwReserved0);
}

void triggerDownload(FakeFolder &folder, const QByteArray &path)
{
auto &journal = folder.syncJournal();
Expand Down Expand Up @@ -1432,6 +1443,68 @@ private Q_SLOTS:
QVERIFY(itemInstruction(completeSpy, "A", CSYNC_INSTRUCTION_NONE));
}

void testFolderPopulationFlags_data()
{
QTest::addColumn<bool>("isRemoteFolder");
QTest::addColumn<bool>("visitFolder");
QTest::addColumn<bool>("expectFullySynced");

QTest::newRow("local new folder") << false << false << true;
QTest::newRow("locally visited folder") << true << true << true;
QTest::newRow("server new folder") << true << false << false;
}

void testFolderPopulationFlags()
{
QFETCH(bool, isRemoteFolder);
QFETCH(bool, visitFolder);
QFETCH(bool, expectFullySynced);

FakeFolder fakeFolder{FileInfo{}};
setupVfs(fakeFolder);

if (isRemoteFolder) {
fakeFolder.remoteModifier().mkdir("A");
fakeFolder.remoteModifier().insert("A/a1");
} else {
fakeFolder.localModifier().mkdir("A");
}
QVERIFY(fakeFolder.syncOnce());

if (visitFolder) {
OCC::showInFileManager(fakeFolder.localPath() + "A");
QTest::qWait(5000);
QVERIFY(fakeFolder.syncOnce());

OCC::showInFileManager(fakeFolder.localPath() + "A");
QTest::qWait(5000);
QVERIFY(fakeFolder.syncOnce());
}

if (isRemoteFolder) {
QCOMPARE(QFileInfo::exists(fakeFolder.localPath() + "A/a1"), expectFullySynced);
}

const auto expectedType = expectFullySynced ? ItemTypeDirectory : ItemTypeVirtualDirectory;
const auto verifyFolderState = [&] {
const auto state = placeholderState(fakeFolder.localPath() + "A");
QVERIFY(state & CF_PLACEHOLDER_STATE_PLACEHOLDER);
QVERIFY(state & CF_PLACEHOLDER_STATE_IN_SYNC);
QCOMPARE(static_cast<bool>(state & CF_PLACEHOLDER_STATE_PARTIALLY_ON_DISK), !expectFullySynced);
QCOMPARE(dbRecord(fakeFolder, "A")._type, expectedType);
};

verifyFolderState();

// local discovery must find the same folder type again and leave the folder untouched
fakeFolder.syncEngine().setLocalDiscoveryOptions(LocalDiscoveryStyle::FilesystemOnly);
ItemCompletedSpy completeSpy(fakeFolder);
QVERIFY(fakeFolder.syncOnce());
QCOMPARE(completeSpy.findItem("A")->_instruction, CSYNC_INSTRUCTION_NONE);

verifyFolderState();
}

void testRemoteTypeChangeExistingLocalMustGetRemoved()
{
FakeFolder fakeFolder{FileInfo{}};
Expand Down
Loading