fix: bypass Qt's QFile::encodeName() in csync

src/common/filesystembase.cpp

@@ -35,6 +35,10 @@
 #include <io.h>
 #endif
 
+#ifdef Q_OS_MAC
+#include <CoreServices/CoreServices.h>
+#endif
+
 namespace {
 // Regarding
 // https://en.wikipedia.org/wiki/Comparison_of_file_systems#Limits
@@ -49,6 +53,16 @@ namespace OCC {
 
 Q_LOGGING_CATEGORY(lcFileSystem, "sync.filesystem", QtInfoMsg)
 
+QByteArray FileSystem::encodeFileName(const QString &fileName)
+{
+    return fileName.toLocal8Bit();
+}
+
+QString FileSystem::decodeFileName(const char *localFileName)
+{
+    return QString::fromLocal8Bit(localFileName);
+}
+
 QString FileSystem::longWinPath(const QString &inpath)
 {
 #ifndef Q_OS_WIN
@@ -198,6 +212,17 @@ bool FileSystem::uncheckedRenameReplace(const QString &originFileName,
 {
     Q_ASSERT(errorString);
 #ifndef Q_OS_WIN
+
+#ifdef Q_OS_MAC
+    // Don't use QFile::rename, it will normalize the destination filename to NFC
+    auto src = QFile::encodeName(originFileName);
+    auto dest = encodeFileName(destinationFileName);
+    if (::renameatx_np(AT_FDCWD, src.constData(), AT_FDCWD, dest.constData(), 0) != 0) {
+        *errorString = QString::fromLocal8Bit(strerror(errno));
+        qCWarning(lcFileSystem) << "Renaming temp file to final failed: " << *errorString;
+        return false;
+    }
+#else
     bool success;
     QFile orig(originFileName);
     // We want a rename that also overwrites.  QFile::rename does not overwrite.
@@ -218,6 +243,7 @@ bool FileSystem::uncheckedRenameReplace(const QString &originFileName,
         qCWarning(lcFileSystem) << "Renaming temp file to final failed: " << *errorString;
         return false;
     }
+#endif
 
 #else //Q_OS_WIN
     // You can not overwrite a read-only file on windows.
@@ -351,6 +377,26 @@ bool FileSystem::fileExists(const QString &filename, const QFileInfo &fileInfo)
     return re;
 }
 
+bool FileSystem::mkpath(const QString &parent, const QString &newDir)
+{
+#ifdef Q_OS_WIN
+    return QDir(parent).mkpath(newDir);
+#else // POSIX
+    auto parts = newDir.split(u'/');
+    QString parentIt = parent;
+    while (!parts.isEmpty()) {
+        auto part = parts.takeFirst();
+        parentIt.append(u'/' + part);
+        if (::mkdir(encodeFileName(QDir::toNativeSeparators(parentIt)).constData(), 0777) != 0) {
+            if (errno != EEXIST) {
+                return false;
+            }
+        }
+    }
+    return true;
+#endif
+}
+
 QString FileSystem::fileSystemForPath(const QString &path)
 {
     QString p = path;

src/common/filesystembase.h

@@ -47,6 +47,9 @@ OCSYNC_EXPORT Q_DECLARE_LOGGING_CATEGORY(lcFileSystem)
 namespace FileSystem {
     OCSYNC_EXPORT Q_NAMESPACE;
 
+    QByteArray OCSYNC_EXPORT encodeFileName(const QString &fileName);
+    QString OCSYNC_EXPORT decodeFileName(const char *localFileName);
+
     /**
      * List of characters not allowd in filenames on Windows
      */
@@ -105,6 +108,8 @@ namespace FileSystem {
      */
     bool OCSYNC_EXPORT fileExists(const QString &filename, const QFileInfo & = QFileInfo());
 
+    bool OCSYNC_EXPORT mkpath(const QString &parent, const QString &newDir);
+
     /**
      * @brief Rename the file \a originFileName to \a destinationFileName.
      *

src/csync/std/c_time.cpp

@@ -23,11 +23,9 @@
 
 #include "common/filesystembase.h"
 
-#include <QFile>
-
 #ifdef HAVE_UTIMES
 int c_utimes(const QString &uri, const struct timeval *times) {
-    int ret = utimes(QFile::encodeName(uri).constData(), times);
+    int ret = utimes(uri.toLocal8Bit().constData(), times);
     return ret;
 }
 #else // HAVE_UTIMES

src/csync/vio/csync_vio_local_unix.cpp

@@ -28,11 +28,11 @@
 
 #include "csync.h"
 
-#include "vio/csync_vio_local.h"
+#include "common/filesystembase.h"
 #include "common/vfs.h"
+#include "vio/csync_vio_local.h"
 
 #include <QtCore/QLoggingCategory>
-#include <QtCore/QFile>
 
 Q_LOGGING_CATEGORY(lcCSyncVIOLocal, "sync.csync.vio_local", QtInfoMsg)
 
@@ -48,7 +48,7 @@ struct csync_vio_handle_t {
 csync_vio_handle_t *csync_vio_local_opendir(const QString &name) {
     std::unique_ptr<csync_vio_handle_t> handle(new csync_vio_handle_t{});
 
-    auto dirname = QFile::encodeName(name);
+    auto dirname = OCC::FileSystem::encodeFileName(name);
 
     handle->dh = opendir(dirname.constData());
     if (!handle->dh) {
@@ -77,7 +77,7 @@ std::unique_ptr<csync_file_stat_t> csync_vio_local_readdir(csync_vio_handle_t *h
     } while (qstrcmp(dirent->d_name, ".") == 0 || qstrcmp(dirent->d_name, "..") == 0);
 
   file_stat.reset(new csync_file_stat_t);
-  file_stat->path = QFile::decodeName(dirent->d_name);
+  file_stat->path = OCC::FileSystem::decodeFileName(dirent->d_name);
 
   /* Check for availability of d_type, see manpage. */
 #if defined(_DIRENT_HAVE_D_TYPE) || defined(__APPLE__)
@@ -120,8 +120,8 @@ int csync_vio_local_stat(const QString &uri, csync_file_stat_t *buf)
 {
     struct stat sb;
 
-    if (lstat(QFile::encodeName(uri).constData(), &sb) < 0) {
-        return -1;
+    if (lstat(OCC::FileSystem::encodeFileName(uri).constData(), &sb) < 0) {
+      return -1;
     }
 
     switch (sb.st_mode & S_IFMT) {

src/gui/socketapi/socketapi.cpp

@@ -235,9 +235,7 @@ void SocketApi::slotReadSocket()
     static auto invalidListener = QSharedPointer<SocketListener>::create(nullptr);
     const auto listener = _listeners.value(socket, invalidListener);
     while (socket->canReadLine()) {
-        // Make sure to normalize the input from the socket to
-        // make sure that the path will match, especially on OS X.
-        QString line = QString::fromUtf8(socket->readLine()).normalized(QString::NormalizationForm_C);
+        QString line = QString::fromUtf8(socket->readLine());
         // Note: do NOT use QString::trimmed() here! That will also remove any trailing spaces (which _are_ part of the filename)!
         line.chop(1); // remove the '\n'
 

src/libsync/propagateremotedelete.h

@@ -33,7 +33,7 @@ class DeleteJob : public AbstractNetworkJob
 };
 
 /**
- * @brief The PropagateRemoteDelete class
+ * @brief Propagate a local delete to the server
  * @ingroup libsync
  */
 class PropagateRemoteDelete : public PropagateItemJob

src/libsync/propagateremotemkdir.h

@@ -19,7 +19,7 @@
 namespace OCC {
 
 /**
- * @brief The PropagateRemoteMkdir class
+ * @brief Propagate a local mkdir to the server
  * @ingroup libsync
  */
 class PropagateRemoteMkdir : public PropagateItemJob

src/libsync/propagateremotemove.h

@@ -37,7 +37,7 @@ class MoveJob : public AbstractNetworkJob
 };
 
 /**
- * @brief The PropagateRemoteMove class
+ * @brief Propagate a local move (or rename) to the server
  * @ingroup libsync
  */
 class PropagateRemoteMove : public PropagateItemJob

src/libsync/propagatorjobs.cpp

@@ -178,8 +178,8 @@ void PropagateLocalMkdir::start()
         done(SyncFileItem::NormalError, tr("Can not create local folder %1 because of a local file name clash with %2").arg(newDirStr, QDir::toNativeSeparators(clash.get())));
         return;
     }
-    QDir localDir(propagator()->localPath());
-    if (!localDir.mkpath(_item->localName())) {
+
+    if (!FileSystem::mkpath(propagator()->localPath(), _item->localName())) {
         done(SyncFileItem::NormalError, tr("could not create folder %1").arg(newDirStr));
         return;
     }

src/libsync/propagatorjobs.h

@@ -28,7 +28,7 @@ static const char checkSumHeaderC[] = "OC-Checksum";
 static const char contentMd5HeaderC[] = "Content-MD5";
 
 /**
- * @brief Declaration of the other propagation jobs
+ * @brief Remove (or move to the trash) a file or folder that was removed remotely
  * @ingroup libsync
  */
 class PropagateLocalRemove : public PropagateItemJob
@@ -47,7 +47,7 @@ class PropagateLocalRemove : public PropagateItemJob
 };
 
 /**
- * @brief The PropagateLocalMkdir class
+ * @brief Make a local directory after discovering it on the server
  * @ingroup libsync
  */
 class PropagateLocalMkdir : public PropagateItemJob
@@ -74,7 +74,7 @@ class PropagateLocalMkdir : public PropagateItemJob
 };
 
 /**
- * @brief The PropagateLocalRename class
+ * @brief Rename a local file/directory after discovering a rename on the server
  * @ingroup libsync
  */
 class PropagateLocalRename : public PropagateItemJob

test/testlocaldiscovery.cpp

@@ -238,6 +238,54 @@ private Q_SLOTS:
         QVERIFY(!fakeFolder.currentRemoteState().find(QStringLiteral("C/.foo")));
         QVERIFY(!fakeFolder.currentRemoteState().find(QStringLiteral("C/bar")));
     }
+
+    void testDirNameEncoding()
+    {
+        QFETCH_GLOBAL(Vfs::Mode, vfsMode);
+        QFETCH_GLOBAL(bool, filesAreDehydrated);
+
+        const unsigned char a_umlaut_composed_bytes[] = {0xc3, 0xa4, 0x00};
+        const QString a_umlaut_composed = QString::fromUtf8(reinterpret_cast<const char *>(a_umlaut_composed_bytes));
+        const QString a_umlaut_decomposed = a_umlaut_composed.normalized(QString::NormalizationForm_D);
+
+        FakeFolder fakeFolder({FileInfo{}}, vfsMode, filesAreDehydrated);
+        fakeFolder.remoteModifier().mkdir(QStringLiteral("P"));
+        fakeFolder.remoteModifier().mkdir(QStringLiteral("P/A"));
+        fakeFolder.remoteModifier().insert(QStringLiteral("P/A/") + a_umlaut_decomposed);
+        fakeFolder.remoteModifier().mkdir(QStringLiteral("P/B") + a_umlaut_decomposed);
+        fakeFolder.remoteModifier().insert(QStringLiteral("P/B") + a_umlaut_decomposed + QStringLiteral("/b"));
+
+        LocalDiscoveryTracker tracker;
+        connect(&fakeFolder.syncEngine(), &SyncEngine::itemCompleted, &tracker, &LocalDiscoveryTracker::slotItemCompleted);
+        connect(&fakeFolder.syncEngine(), &SyncEngine::finished, &tracker, &LocalDiscoveryTracker::slotSyncFinished);
+
+        QVERIFY(fakeFolder.applyLocalModificationsAndSync());
+
+        {
+            auto localState = fakeFolder.currentLocalState();
+            FileInfo *localFile = localState.find(QStringLiteral("P/A/") + a_umlaut_decomposed);
+            QVERIFY(localFile != nullptr); // check if the file exists
+        }
+        {
+            auto localState = fakeFolder.currentLocalState();
+            FileInfo *localFile = localState.find(QStringLiteral("P/B") + a_umlaut_decomposed + QStringLiteral("/b"));
+            QVERIFY(localFile != nullptr); // check if the file exists
+        }
+
+        qDebug() << "*** MARK"; // Log marker to check if a PUT/DELETE shows up in the second sync
+
+        fakeFolder.syncEngine().setLocalDiscoveryOptions(LocalDiscoveryStyle::DatabaseAndFilesystem, {QStringLiteral("P")});
+        tracker.startSyncFullDiscovery();
+        QVERIFY(fakeFolder.applyLocalModificationsAndSync());
+
+        auto remoteState = fakeFolder.currentRemoteState();
+        QVERIFY(remoteState.find(QStringLiteral("P/A/") + a_umlaut_decomposed) != nullptr); // check if the file still exists in the original normalization
+        QVERIFY(remoteState.find(QStringLiteral("P/A/") + a_umlaut_composed) == nullptr); // there should NOT be a file with another normalization
+        QVERIFY(remoteState.find(QStringLiteral("P/B") + a_umlaut_decomposed + QStringLiteral("/b"))
+            != nullptr); // check if the directory still exists in the original normalization
+        QVERIFY(remoteState.find(QStringLiteral("P/B") + a_umlaut_composed + QStringLiteral("/b"))
+            == nullptr); // there should NOT be a directory with another normalization
+    }
 };
 
 QTEST_GUILESS_MAIN(TestLocalDiscovery)

test/testutils/syncenginetestutils.cpp

@@ -15,6 +15,7 @@
 #include "libsync/syncresult.h"
 
 #include <thread>
+#include <vio/csync_vio_local.h>
 
 using namespace std::chrono_literals;
 using namespace std::chrono;
@@ -1099,34 +1100,41 @@ void FakeFolder::toDisk(QDir &dir, const FileInfo &templateFi)
 
 void FakeFolder::fromDisk(QDir &dir, FileInfo &templateFi)
 {
-    const auto infoList = dir.entryInfoList(QDir::AllEntries | QDir::NoDotAndDotDot);
-    for (const auto &diskChild : infoList) {
-        if (diskChild.isHidden() || diskChild.fileName().startsWith(QStringLiteral(".sync_"))) {
-            // Skip system files, sqlite db files, sync log, etc.
+    auto dh = csync_vio_local_opendir(dir.absolutePath());
+    if (!dh) {
+        return;
+    }
+    while (true) {
+        auto dirent = csync_vio_local_readdir(dh, nullptr);
+        if (!dirent)
+            break;
+        if (dirent->type == ItemTypeSkip)
+            continue;
+        if (dirent->is_hidden || dirent->path.startsWith(QStringLiteral(".sync_")))
             continue;
-        }
 
-        if (diskChild.isDir()) {
+        QString absolutePathItem = dir.absolutePath() + QDir::separator() + dirent->path;
+        if (dirent->type == ItemTypeDirectory) {
             QDir subDir = dir;
-            subDir.cd(diskChild.fileName());
-            FileInfo &subFi = templateFi.children[diskChild.fileName()] = FileInfo { diskChild.fileName() };
-            subFi.setLastModified(diskChild.lastModified());
+            subDir.cd(dirent->path);
+            FileInfo &subFi = templateFi.children[dirent->path] = FileInfo{dirent->path};
+            subFi.setLastModified(QDateTime::fromSecsSinceEpoch(dirent->modtime, QTimeZone::utc()));
             fromDisk(subDir, subFi);
         } else {
-            FileInfo fi(diskChild.fileName());
+            FileInfo fi(dirent->path);
             fi.isDir = false;
-            fi.fileSize = diskChild.size();
-            fi.isDehydratedPlaceholder = isDehydratedPlaceholder(diskChild.absoluteFilePath());
-            fi.setLastModified(diskChild.lastModified());
+            fi.fileSize = dirent->size;
+            fi.isDehydratedPlaceholder = isDehydratedPlaceholder(absolutePathItem);
+            fi.setLastModified(QDateTime::fromSecsSinceEpoch(dirent->modtime, QTimeZone::utc()));
             if (fi.isDehydratedPlaceholder) {
                 fi.contentChar = '\0';
                 fi.contentSize = 0;
             } else {
-                QFile f { diskChild.filePath() };
+                QFile f{absolutePathItem};
                 OC_ENFORCE(f.open(QFile::ReadOnly));
                 auto content = f.read(1);
                 if (content.size() == 0) {
-                    qWarning() << "Empty file at:" << diskChild.filePath();
+                    qWarning() << "Empty file at:" << dirent->path;
                     fi.contentChar = FileInfo::DefaultContentChar;
                 } else {
                     fi.contentChar = content.at(0);