From: Jocelyn Turcotte <jturcotte@woboq.com>
Date: Thu, 31 Aug 2017 11:32:00 +0000 (+0200)
Subject: StatusTracker: Fix different case paths not matching (#5981)
X-Git-Tag: archive/raspbian/3.16.7-1_deb13u1+rpi1~1^2~701^2~139
X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=4aca1b96979e8f41414ee146874eddb17be1dc3d;p=nextcloud-desktop.git

StatusTracker: Fix different case paths not matching (#5981)

Use a custom std::map comparator functor to do all comparisons
on contained QStrings using Qt::CaseInsensitive on macOS and Windows.

Issue #5257
---

diff --git a/src/libsync/syncfilestatustracker.cpp b/src/libsync/syncfilestatustracker.cpp
index 9d4fa6e37..32e3ccb00 100644
--- a/src/libsync/syncfilestatustracker.cpp
+++ b/src/libsync/syncfilestatustracker.cpp
@@ -25,20 +25,49 @@ namespace OCC {
 
 Q_LOGGING_CATEGORY(lcStatusTracker, "sync.statustracker", QtInfoMsg)
 
-static SyncFileStatus::SyncFileStatusTag lookupProblem(const QString &pathToMatch, const std::map<QString, SyncFileStatus::SyncFileStatusTag> &problemMap)
+static int pathCompare( const QString& lhs, const QString& rhs )
+{
+    // Should match Utility::fsCasePreserving, we want don't want to pay for the runtime check on every comparison.
+    return lhs.compare(rhs,
+#if defined(Q_OS_WIN) || defined(Q_OS_MAC)
+        Qt::CaseInsensitive
+#else
+        Qt::CaseSensitive
+#endif
+        );
+}
+
+static bool pathStartsWith( const QString& lhs, const QString& rhs )
+{
+    return lhs.startsWith(rhs,
+#if defined(Q_OS_WIN) || defined(Q_OS_MAC)
+        Qt::CaseInsensitive
+#else
+        Qt::CaseSensitive
+#endif
+        );
+}
+
+bool SyncFileStatusTracker::PathComparator::operator()( const QString& lhs, const QString& rhs ) const
+{
+    // This will make sure that the std::map is ordered and queried case-insensitively on macOS and Windows.
+    return pathCompare(lhs, rhs) < 0;
+}
+
+SyncFileStatus::SyncFileStatusTag SyncFileStatusTracker::lookupProblem(const QString &pathToMatch, const SyncFileStatusTracker::ProblemsMap &problemMap)
 {
     auto lower = problemMap.lower_bound(pathToMatch);
     for (auto it = lower; it != problemMap.cend(); ++it) {
         const QString &problemPath = it->first;
         SyncFileStatus::SyncFileStatusTag severity = it->second;
 
-        if (problemPath == pathToMatch) {
+        if (pathCompare(problemPath, pathToMatch) == 0) {
             return severity;
         } else if (severity == SyncFileStatus::StatusError
-            && problemPath.startsWith(pathToMatch)
+            && pathStartsWith(problemPath, pathToMatch)
             && (pathToMatch.isEmpty() || problemPath.at(pathToMatch.size()) == '/')) {
             return SyncFileStatus::StatusWarning;
-        } else if (!problemPath.startsWith(pathToMatch)) {
+        } else if (!pathStartsWith(problemPath, pathToMatch)) {
             // Starting at lower_bound we get the first path that is not smaller,
             // since: "a/" < "a/aa" < "a/aa/aaa" < "a/ab/aba"
             // If problemMap keys are ["a/aa/aaa", "a/ab/aba"] and pathToMatch == "a/aa",
@@ -182,7 +211,7 @@ void SyncFileStatusTracker::slotAboutToPropagate(SyncFileItemVector &items)
 {
     ASSERT(_syncCount.isEmpty());
 
-    std::map<QString, SyncFileStatus::SyncFileStatusTag> oldProblems;
+    ProblemsMap oldProblems;
     std::swap(_syncProblems, oldProblems);
 
     foreach (const SyncFileItemPtr &item, items) {
diff --git a/src/libsync/syncfilestatustracker.h b/src/libsync/syncfilestatustracker.h
index 12fe0b337..48ecfd5c2 100644
--- a/src/libsync/syncfilestatustracker.h
+++ b/src/libsync/syncfilestatustracker.h
@@ -51,6 +51,12 @@ private slots:
     void slotSyncEngineRunningChanged();
 
 private:
+    struct PathComparator {
+        bool operator()( const QString& lhs, const QString& rhs ) const;
+    };
+    typedef std::map<QString, SyncFileStatus::SyncFileStatusTag, PathComparator> ProblemsMap;
+    SyncFileStatus::SyncFileStatusTag lookupProblem(const QString &pathToMatch, const ProblemsMap &problemMap);
+
     enum SharedFlag { UnknownShared,
         NotShared,
         Shared };
@@ -65,7 +71,7 @@ private:
 
     SyncEngine *_syncEngine;
 
-    std::map<QString, SyncFileStatus::SyncFileStatusTag> _syncProblems;
+    ProblemsMap _syncProblems;
     QSet<QString> _dirtyPaths;
     // Counts the number direct children currently being synced (has unfinished propagation jobs).
     // We'll show a file/directory as SYNC as long as its sync count is > 0.
diff --git a/test/testsyncfilestatustracker.cpp b/test/testsyncfilestatustracker.cpp
index 741ae1dc6..ed828857d 100644
--- a/test/testsyncfilestatustracker.cpp
+++ b/test/testsyncfilestatustracker.cpp
@@ -260,6 +260,24 @@ private slots:
         QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
     }
 
+    void warningStatusForExcludedFile_CasePreserving() {
+        FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()};
+        fakeFolder.syncEngine().excludedFiles().addExcludeExpr("B");
+        fakeFolder.serverErrorPaths().append("A/a1");
+        fakeFolder.localModifier().appendByte("A/a1");
+
+        fakeFolder.syncOnce();
+        QCOMPARE(fakeFolder.syncEngine().syncFileStatusTracker().fileStatus(""), SyncFileStatus(SyncFileStatus::StatusWarning));
+        QCOMPARE(fakeFolder.syncEngine().syncFileStatusTracker().fileStatus("A"), SyncFileStatus(SyncFileStatus::StatusWarning));
+        QCOMPARE(fakeFolder.syncEngine().syncFileStatusTracker().fileStatus("A/a1"), SyncFileStatus(SyncFileStatus::StatusError));
+        QCOMPARE(fakeFolder.syncEngine().syncFileStatusTracker().fileStatus("B"), SyncFileStatus(SyncFileStatus::StatusWarning));
+
+        // Should still get the status for different casing on macOS and Windows.
+        QCOMPARE(fakeFolder.syncEngine().syncFileStatusTracker().fileStatus("a"), SyncFileStatus(Utility::fsCasePreserving() ? SyncFileStatus::StatusWarning : SyncFileStatus::StatusNone));
+        QCOMPARE(fakeFolder.syncEngine().syncFileStatusTracker().fileStatus("A/A1"), SyncFileStatus(Utility::fsCasePreserving() ? SyncFileStatus::StatusError : SyncFileStatus::StatusNone));
+        QCOMPARE(fakeFolder.syncEngine().syncFileStatusTracker().fileStatus("b"), SyncFileStatus(Utility::fsCasePreserving() ? SyncFileStatus::StatusWarning : SyncFileStatus::StatusNone));
+    }
+
     void parentsGetWarningStatusForError() {
         FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()};
         fakeFolder.serverErrorPaths().append("A/a1");