SyncPal::handleAccessDeniedItem encoding issue

src/libsyncengine/jobs/network/API_v2/downloadjob.cpp

@@ -478,7 +478,6 @@ bool DownloadJob::createLink(const std::string &mimeType, const std::string &dat
 
         IoError ioError = IoError::Success;
         if (!IoHelper::createAlias(data, _localpath, ioError)) {
-            const std::wstring message = Utility::s2ws(IoHelper::ioError2StdString(ioError));
             LOGW_WARN(_logger, L"Failed to create alias: " << Utility::formatIoError(_localpath, ioError).c_str());
 
             return false;

src/libsyncengine/propagation/executor/executorworker.cpp

@@ -1733,7 +1733,7 @@ ExitInfo ExecutorWorker::handleForbiddenAction(SyncOpPtr syncOp, const SyncPath
                         absoluteLocalFilePath, PlatformInconsistencyCheckerUtility::SuffixType::Blacklisted)) {
                 LOGW_SYNCPAL_WARN(_logger, L"PlatformInconsistencyCheckerUtility::renameLocalFile failed for: "
                                                    << Utility::formatSyncPath(absoluteLocalFilePath));
-                _syncPal->handleAccessDeniedItem(relativeLocalPath, ExitCause::FileAccessError);
+                _syncPal->handleAccessDeniedItem(relativeLocalPath);
                 return ExitCode::Ok;
             }
             removeFromDb = false;
@@ -2526,15 +2526,16 @@ ExitInfo ExecutorWorker::handleExecutorError(SyncOpPtr syncOp, ExitInfo opsExitI
 ExitInfo ExecutorWorker::handleOpsFileAccessError(SyncOpPtr syncOp, ExitInfo opsExitInfo) {
     if (syncOp->targetSide() == ReplicaSide::Local && syncOp->type() == OperationType::Create) {
         // The item does not exist yet locally, we will only tmpBlacklist the remote item
-        if (ExitInfo exitInfo = _syncPal->handleAccessDeniedItem(syncOp->affectedNode()->getPath()); !exitInfo) {
+        if (ExitInfo exitInfo =
+                    _syncPal->handleAccessDeniedItem(syncOp->affectedNode()->getPath(), ExitCause::FileAccessError, true);
+            !exitInfo) {
             return exitInfo;
         }
     } else {
         // Both local and remote item will be temporarily blacklisted
         auto localNode = syncOp->targetSide() == ReplicaSide::Remote ? syncOp->affectedNode() : syncOp->correspondingNode();
         if (!localNode) return ExitCode::LogicError;
         SyncPath relativeLocalFilePath = localNode->getPath();
-        NodeId localNodeId = localNode->id().has_value() ? *localNode->id() : NodeId();
         if (ExitInfo exitInfo = _syncPal->handleAccessDeniedItem(relativeLocalFilePath, opsExitInfo.cause()); !exitInfo) {
             return exitInfo;
         }

src/libsyncengine/syncpal/syncpal.cpp

@@ -1448,7 +1448,7 @@ void SyncPal::removeItemFromTmpBlacklist(const SyncPath &relativePath) {
     _tmpBlacklistManager->removeItemFromTmpBlacklist(relativePath);
 }
 
-ExitInfo SyncPal::handleAccessDeniedItem(const SyncPath &relativePath, ExitCause cause) {
+ExitInfo SyncPal::handleAccessDeniedItem(const SyncPath &relativePath, ExitCause cause, const bool normalizedPath) {
     if (relativePath.empty()) {
         LOG_SYNCPAL_WARN(_logger, "Access error on root folder");
         return ExitInfo(ExitCode::SystemError, ExitCause::SyncDirAccesError);
@@ -1458,11 +1458,11 @@ ExitInfo SyncPal::handleAccessDeniedItem(const SyncPath &relativePath, ExitCause
     addError(error);
 
     NodeId localNodeId;
-    if (localNodeId = snapshot(ReplicaSide::Local)->itemId(relativePath); localNodeId.empty()) {
+    if (localNodeId = snapshot(ReplicaSide::Local)->itemId(relativePath, normalizedPath); localNodeId.empty()) {
         // The file does not exit yet on local file system, or we do not have sufficient right on a parent folder.
         LOGW_DEBUG(_logger, L"Item " << Utility::formatSyncPath(relativePath)
                                      << L"is not present local file system, blacklisting the parent item.");
-        return handleAccessDeniedItem(relativePath.parent_path(), cause);
+        return handleAccessDeniedItem(relativePath.parent_path(), cause, normalizedPath);
     }
 
 

src/libsyncengine/syncpal/syncpal.h

@@ -258,7 +258,8 @@ class SYNCENGINE_EXPORT SyncPal : public std::enable_shared_from_this<SyncPal> {
         virtual void removeItemFromTmpBlacklist(const NodeId &nodeId, ReplicaSide side);
         virtual void removeItemFromTmpBlacklist(const SyncPath &relativePath);
 
-        ExitInfo handleAccessDeniedItem(const SyncPath &relativePath, ExitCause cause = ExitCause::FileAccessError);
+        ExitInfo handleAccessDeniedItem(const SyncPath &relativePath, ExitCause cause = ExitCause::FileAccessError,
+                                        const bool normalizedPath = false);
         //! Makes copies of real-time snapshots to be used by synchronization workers.
         void copySnapshots();
 

src/libsyncengine/update_detection/file_system_observer/computefsoperationworker.cpp

@@ -162,9 +162,10 @@ ExitCode ComputeFSOperationWorker::inferChangeFromDbNode(const ReplicaSide side,
             // Check that the file/directory really does not exist on replica
             bool isExcluded = false;
             if (const ExitInfo exitInfo = checkIfOkToDelete(side, dbPath, nodeId, isExcluded); !exitInfo) {
+                // Can happen only on local side
                 if (exitInfo == ExitInfo(ExitCode::SystemError, ExitCause::FileAccessError)) {
                     // Blacklist node
-                    if (ExitInfo exitInfo = _syncPal->handleAccessDeniedItem(dbPath); !exitInfo) {
+                    if (ExitInfo exitInfo = _syncPal->handleAccessDeniedItem(localDbPath); !exitInfo) {
                         return exitInfo;
                     }
 

src/libsyncengine/update_detection/file_system_observer/snapshot/snapshot.cpp

@@ -172,7 +172,7 @@ bool Snapshot::removeItem(const NodeId id) {
     return true;
 }
 
-NodeId Snapshot::itemId(const SyncPath &path) const {
+NodeId Snapshot::itemId(const SyncPath &path, const bool normalizedSearch) const {
     const std::scoped_lock lock(_mutex);
 
     NodeId ret;
@@ -187,7 +187,18 @@ NodeId Snapshot::itemId(const SyncPath &path) const {
 
         bool idFound = false;
         for (const NodeId &childId: itemIt->second.childrenIds()) {
-            if (name(childId) == *pathIt) {
+            SyncName childName;
+            if (normalizedSearch) {
+                if (!Utility::normalizedSyncName(name(childId), childName)) {
+                    LOGW_WARN(Log::instance()->getLogger(),
+                              L"Error in Utility::normalizedSyncName: " << Utility::formatSyncName(name(childId)));
+                    continue;
+                }
+            } else {
+                childName = name(childId);
+            }
+
+            if (childName == *pathIt) {
                 itemIt = _items.find(childId);
                 ret = childId;
                 idFound = true;

src/libsyncengine/update_detection/file_system_observer/snapshot/snapshot.h

@@ -42,7 +42,7 @@ class Snapshot : public SharedObject {
         bool updateItem(const SnapshotItem &newItem);
         bool removeItem(const NodeId id); // Do not pass by reference to avoid dangling references
 
-        NodeId itemId(const SyncPath &path) const;
+        NodeId itemId(const SyncPath &path, const bool normalizedSearch = false) const;
         NodeId parentId(const NodeId &itemId) const;
         bool setParentId(const NodeId &itemId, const NodeId &newParentId);
         bool path(const NodeId &itemId, SyncPath &path) const noexcept;

test/libsyncengine/update_detection/file_system_observer/testsnapshot.cpp

@@ -32,6 +32,39 @@ void TestSnapshot::setUp() {
 
 void TestSnapshot::tearDown() {}
 
+void TestSnapshot::testItemId() {
+    const NodeId rootNodeId = SyncDb::driveRootNode().nodeIdLocal().value();
+
+    const DbNode dummyRootNode(0, std::nullopt, SyncName(), SyncName(), "1", "1", std::nullopt, std::nullopt, std::nullopt,
+                               NodeType::Directory, 0, std::nullopt);
+    Snapshot snapshot(ReplicaSide::Local, dummyRootNode);
+
+    snapshot.updateItem(
+            SnapshotItem("2", rootNodeId, Str("a"), 1640995202, 1640995202, NodeType::Directory, 0, false, true, true));
+    snapshot.updateItem(SnapshotItem("3", "2", Str("aa"), 1640995202, 1640995202, NodeType::Directory, 0, false, true, true));
+    snapshot.updateItem(SnapshotItem("4", "2", Str("ab"), 1640995202, 1640995202, NodeType::Directory, 0, false, true, true));
+    snapshot.updateItem(SnapshotItem("5", "2", Str("ac"), 1640995202, 1640995202, NodeType::Directory, 0, false, true, true));
+    snapshot.updateItem(SnapshotItem("6", "4", Str("aba"), 1640995202, 1640995202, NodeType::Directory, 0, false, true, true));
+    snapshot.updateItem(SnapshotItem("8", "6", Str("abaa"), 1640995202, 1640995202, NodeType::File, 10, false, true, true));
+    CPPUNIT_ASSERT_EQUAL(NodeId("8"), snapshot.itemId(std::filesystem::path("a/ab/aba/abaa")));
+
+    SyncName nfcNormalized;
+    Utility::normalizedSyncName("abà", nfcNormalized);
+
+    SyncName nfdNormalized;
+    Utility::normalizedSyncName("abà", nfdNormalized, Utility::UnicodeNormalization::NFD);
+
+    snapshot.updateItem(SnapshotItem("6", "4", nfcNormalized, 1640995202, 1640995202, NodeType::Directory, 0, false, true, true));
+    CPPUNIT_ASSERT_EQUAL(NodeId("8"), snapshot.itemId(std::filesystem::path("a/ab") / nfcNormalized / "abaa"));
+    CPPUNIT_ASSERT_EQUAL(NodeId(""), snapshot.itemId(std::filesystem::path("a/ab") / nfdNormalized / "abaa"));
+    CPPUNIT_ASSERT_EQUAL(NodeId("8"), snapshot.itemId(std::filesystem::path("a/ab") / nfcNormalized / "abaa", true));
+
+    snapshot.updateItem(SnapshotItem("6", "4", nfdNormalized, 1640995202, 1640995202, NodeType::Directory, 0, false, true, true));
+    CPPUNIT_ASSERT_EQUAL(NodeId("8"), snapshot.itemId(std::filesystem::path("a/ab") / nfdNormalized / "abaa"));
+    CPPUNIT_ASSERT_EQUAL(NodeId(""), snapshot.itemId(std::filesystem::path("a/ab") / nfcNormalized / "abaa"));
+    CPPUNIT_ASSERT_EQUAL(NodeId("8"), snapshot.itemId(std::filesystem::path("a/ab") / nfcNormalized / "abaa", true));
+}
+
 /**
  * Tree:
  *

test/libsyncengine/update_detection/file_system_observer/testsnapshot.h

@@ -27,6 +27,7 @@ namespace KDC {
 
 class TestSnapshot : public CppUnit::TestFixture {
         CPPUNIT_TEST_SUITE(TestSnapshot);
+        CPPUNIT_TEST(testItemId);
         CPPUNIT_TEST(testSnapshot);
         CPPUNIT_TEST(testDuplicatedItem);
         CPPUNIT_TEST(testSnapshotInsertionWithDifferentEncodings);
@@ -37,6 +38,7 @@ class TestSnapshot : public CppUnit::TestFixture {
         void tearDown() override;
 
     private:
+        void testItemId();
         void testSnapshot();
         void testDuplicatedItem();
         void testSnapshotInsertionWithDifferentEncodings();