Fix canonicalPath for drives (c:)

src/gui/folderman.cpp

@@ -421,7 +421,7 @@ Folder *FolderMan::folderForPath(const QString &path, QString *relativePath)
     for (auto *folder : std::as_const(_folders)) {
         const QString folderPath = folder->cleanPath() + QLatin1Char('/');
 
-        if (absolutePath.startsWith(folderPath, (Utility::isWindows() || Utility::isMac()) ? Qt::CaseInsensitive : Qt::CaseSensitive)) {
+        if (FileSystem::isChildPathOf2(absolutePath, folderPath).testAnyFlag(FileSystem::ChildResult::IsChild)) {
             if (relativePath) {
                 *relativePath = absolutePath.mid(folderPath.length());
                 relativePath->chop(1); // we added a '/' above
@@ -541,27 +541,6 @@ QString FolderMan::trayTooltipStatusString(
     return folderMessage;
 }
 
-// QFileInfo::canonicalPath returns an empty string if the file does not exist.
-// This function also works with files that does not exist and resolve the symlinks in the
-// parent directories.
-static QString canonicalPath(const QString &path)
-{
-    QFileInfo selFile(path);
-    if (!selFile.exists()) {
-        const auto parentPath = selFile.dir().path();
-
-        // It's possible for the parentPath to match the path
-        // (possibly we've arrived at a non-existant drive root on Windows)
-        // and recursing would be fatal.
-        if (parentPath == path) {
-            return path;
-        }
-
-        return canonicalPath(parentPath) + QLatin1Char('/') + selFile.fileName();
-    }
-    return selFile.canonicalFilePath();
-}
-
 static QString checkPathForSyncRootMarkingRecursive(const QString &path, FolderMan::NewFolderType folderType, const QUuid &accountUuid)
 {
     std::pair<QString, QUuid> existingTags = Utility::getDirectorySyncRootMarkings(path);
@@ -645,17 +624,18 @@ QString FolderMan::checkPathValidityRecursive(const QString &path, FolderMan::Ne
 QString FolderMan::checkPathValidityForNewFolder(const QString &path, NewFolderType folderType, const QUuid &accountUuid) const
 {
     // check if the local directory isn't used yet in another sync
-    const auto cs = Utility::fsCaseSensitivity();
-
-    const QString userDir = QDir::cleanPath(canonicalPath(path)) + QLatin1Char('/');
+    if (path.isEmpty()) {
+        return u"Passingg an empty path is not supported"_s;
+    }
+    const QString userDir = FileSystem::canonicalPath(path) + QLatin1Char('/');
     for (auto f : _folders) {
-        const QString folderDir = QDir::cleanPath(canonicalPath(f->path())) + QLatin1Char('/');
+        const QString folderDir = FileSystem::canonicalPath(f->path()) + QLatin1Char('/');
 
-        if (QString::compare(folderDir, userDir, cs) == 0) {
+        const auto isChild = FileSystem::isChildPathOf2(folderDir, userDir);
+        if (isChild.testFlag(FileSystem::ChildResult::IsEqual)) {
             return tr("There is already a sync from the server to this local folder. "
                       "Please pick another local folder!");
-        }
-        if (FileSystem::isChildPathOf(folderDir, userDir)) {
+        } else if (isChild.testFlag(FileSystem::ChildResult::IsChild)) {
             return tr("The local folder »%1« already contains a folder used in a folder sync connection. "
                       "Please pick another local folder!")
                 .arg(QDir::toNativeSeparators(path));
@@ -681,30 +661,32 @@ QString FolderMan::findGoodPathForNewSyncFolder(
     OC_ASSERT(!accountUuid.isNull() || folderType == FolderMan::NewFolderType::SpacesSyncRoot);
 
     // reserve extra characters to allow appending of a number
-    const QString normalisedPath = FileSystem::createPortableFileName(basePath, FileSystem::pathEscape(newFolder), std::string_view(" (100)").size());
+    const QString normalisedPath = FileSystem::createPortableFileName(basePath, newFolder, std::string_view(" (100)").size());
 
     // If the parent folder is a sync folder or contained in one, we can't
     // possibly find a valid sync folder inside it.
     // Example: Someone syncs their home directory. Then ~/foobar is not
     // going to be an acceptable sync folder path for any value of foobar.
-    if (FolderMan::instance()->folderForPath(QFileInfo(normalisedPath).canonicalPath())) {
+    // If relativePath is empty, the path is equal to newFolder, and we will find a name in the following loop
+    QString relativePath;
+    if (FolderMan::instance()->folderForPath(FileSystem::canonicalPath(normalisedPath), &relativePath) && !relativePath.isEmpty()) {
         // Any path with that parent is going to be unacceptable,
         // so just keep it as-is.
-        return canonicalPath(normalisedPath);
+        return FileSystem::canonicalPath(normalisedPath);
     }
     // Count attempts and give up eventually
     {
         QString folder = normalisedPath;
         for (int attempt = 2; attempt <= 100; ++attempt) {
             if (!QFileInfo::exists(folder) && FolderMan::instance()->checkPathValidityForNewFolder(folder, folderType, accountUuid).isEmpty()) {
-                return canonicalPath(folder);
+                return FileSystem::canonicalPath(folder);
             }
             folder = normalisedPath + QStringLiteral(" (%1)").arg(attempt);
         }
     }
     // we failed to find a non existing path
     Q_ASSERT(false);
-    return canonicalPath(normalisedPath);
+    return FileSystem::canonicalPath(normalisedPath);
 }
 
 bool FolderMan::ignoreHiddenFiles() const

src/libsync/common/filesystembase.cpp

@@ -64,11 +64,11 @@ QString FileSystem::fromFilesystemPath(const std::filesystem::path &path)
 #ifdef Q_OS_WIN
     constexpr std::wstring_view prefix = LR"(\\?\)";
     std::wstring nativePath = path.native();
+    auto view = std::wstring_view(nativePath);
     if (nativePath.starts_with(prefix)) {
-        const auto view = std::wstring_view(nativePath).substr(prefix.size());
-        return QString::fromWCharArray(view.data(), view.length());
+        view = view.substr(prefix.size());
     }
-    return QString::fromStdWString(nativePath);
+    return QDir::fromNativeSeparators(QString::fromWCharArray(view.data(), view.length()));
 #elif defined(Q_OS_MACOS)
     // based on QFile::decodeName
     return QString::fromStdString(path.native()).normalized(QString::NormalizationForm_C);
@@ -86,20 +86,24 @@ QString FileSystem::longWinPath(const QString &inpath)
     if (inpath.isEmpty()) {
         return inpath;
     }
-    const QString str = QDir::toNativeSeparators(inpath);
+    QString out = QDir::toNativeSeparators(inpath);
     const QLatin1Char sep('\\');
+    if (out.size() == 2 && out.at(1) == ':'_L1) {
+        // std::path handles C: incorrectly
+        out += sep;
+    }
 
     // we already have a unc path
-    if (str.startsWith(sep + sep)) {
-        return str;
+    if (out.startsWith(sep + sep)) {
+        return out;
     }
     // prepend \\?\ and to support long names
 
-    if (str.at(0) == sep) {
+    if (out.at(0) == sep) {
         // should not happen as we require the path to be absolute
-        return QStringLiteral("\\\\?") + str;
+        return QStringLiteral("\\\\?") + out;
     }
-    return QStringLiteral("\\\\?\\") + str;
+    return QStringLiteral("\\\\?\\") + out;
 #endif
 }
 
@@ -556,7 +560,6 @@ QString FileSystem::createPortableFileName(const QString &path, const QString &f
     tmp.resize(std::min<qsizetype>(tmp.size(), fileNameMaxC - reservedSize));
     // remove eventual trailing whitespace after the resize
     tmp = tmp.trimmed();
-
     return QDir::cleanPath(path + QLatin1Char('/') + tmp);
 }
 

src/libsync/filesystem.cpp

@@ -179,6 +179,46 @@ bool FileSystem::fileChanged(const std::filesystem::path &path, const FileChange
     return false;
 }
 
+std::filesystem::path FileSystem::canonicalPath(const std::filesystem::path &p)
+{
+    std::error_code ec;
+    if (!std::filesystem::exists(p, ec) && !ec) {
+        const auto normalized = p.lexically_normal();
+        const auto parentPath = normalized.parent_path();
+        // last invocation will return /
+        if (parentPath == p) {
+            return p;
+        }
+        if (normalized.filename().empty()) {
+            return canonicalPath(parentPath);
+        } else {
+            return canonicalPath(parentPath) / normalized.filename();
+        }
+    }
+    if (ec) {
+        qCWarning(lcFileSystem) << "Failed to check existence of path:" << p << ec.message();
+    }
+    const auto out = std::filesystem::canonical(p, ec);
+    if (ec) {
+        qCWarning(lcFileSystem) << "Failed to canonicalize path:" << p << ec.message();
+        return p;
+    }
+#ifdef Q_OS_WIN
+    // std::filesystem::canonical removes the ucn prefix
+    const auto ucn = std::wstring(LR"(\\?\)");
+    Q_ASSERT(!out.native().starts_with(ucn));
+    return std::filesystem::path(ucn + out.native());
+#else
+    return out;
+#endif
+}
+
+QString FileSystem::canonicalPath(const QString &p)
+{
+    // clean path to normalize path back to Qt form
+    return FileSystem::fromFilesystemPath(canonicalPath(FileSystem::toFilesystemPath(p)));
+}
+
 qint64 FileSystem::getSize(const std::filesystem::path &filename)
 {
     std::error_code ec;

src/libsync/filesystem.h

@@ -106,6 +106,14 @@ namespace FileSystem {
     bool OPENCLOUD_SYNC_EXPORT fileChanged(const std::filesystem::path &path, const FileChangedInfo &previousInfo);
 
 
+    // canonicalPath returns an empty string if the file does not exist.
+    // This function also works with files that does not exist and resolve the symlinks in the
+    // parent directories.
+    std::filesystem::path OPENCLOUD_SYNC_EXPORT canonicalPath(const std::filesystem::path &p);
+
+    QString OPENCLOUD_SYNC_EXPORT canonicalPath(const QString &p);
+
+
     struct RemoveEntry
     {
         const QString path;

src/libsync/propagateuploadtus.cpp

@@ -235,8 +235,6 @@ void PropagateUploadFileTUS::slotChunkFinished()
         propagator()->_anotherSyncNeeded = true;
         if (!_finished) {
             abortWithError(SyncFileItem::Message, fileChangedMessage());
-            // FIXME:  the legacy code was retrying for a few seconds.
-            //         and also checking that after the last chunk, and removed the file in case of INSTRUCTION_NEW
             return;
         }
     }

test/testfolderman.cpp

@@ -105,7 +105,7 @@ private Q_SLOTS:
         QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/sub/OpenCloud1/some/sub/path"), type, uuid).isNull());
         QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/OpenCloud2/blublu"), type, uuid).isNull());
         QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/sub/OpenCloud1/folder/g/h"), type, uuid).isNull());
-        QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/link3/folder/neu_folder"), type, uuid).isNull());
+        QCOMPARE_NE(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/link3/folder/neu_folder"), type, uuid), QString());
 
         // Subfolder of links
         QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + QStringLiteral("/link1/subfolder"), type, uuid).isNull());

test/testutility.cpp

@@ -470,6 +470,85 @@ private Q_SLOTS:
         QVERIFY(inode.has_value());
         QCOMPARE(fileInfo.inode(), inode.value());
     }
+
+    void testCanonicalPath()
+    {
+        // we compare .native() for std::fiileystem::path, else Qt does actual file comparison
+        std::error_code ec;
+        // our build dir might be symlinked, ensure the input path is already canonical
+        auto path = OCC::FileSystem::fromFilesystemPath(std::filesystem::canonical(qApp->applicationFilePath().toStdString(), ec));
+        QVERIFY(ec.value() == 0);
+        QCOMPARE(OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native(), OCC::FileSystem::toFilesystemPath(path).native());
+        QCOMPARE(path, OCC::FileSystem::canonicalPath(path));
+
+#ifdef Q_OS_WIN
+        path = u"C:/"_s;
+        QCOMPARE(path, OCC::FileSystem::canonicalPath(path));
+        QCOMPARE(OCC::FileSystem::toFilesystemPath(path).native(), OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native());
+
+        path = u"C:"_s;
+        QCOMPARE("C:/"_L1, OCC::FileSystem::canonicalPath(path));
+        QCOMPARE(OCC::FileSystem::toFilesystemPath(path).native(), OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native());
+
+
+        // test non-existing file, which relies on lexical normalization rather than actual canonical path
+        path = u"C:/fooo_bar"_s;
+        QVERIFY(!QFileInfo::exists(path));
+        QCOMPARE(path, OCC::FileSystem::canonicalPath(path));
+        QCOMPARE(OCC::FileSystem::toFilesystemPath(path).native(), OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native());
+
+        path = u"C:/fooo_bar/../foo/./../fooo_bar"_s;
+        QVERIFY(!QFileInfo::exists(path));
+        QCOMPARE(u"C:/fooo_bar"_s, OCC::FileSystem::canonicalPath(path));
+        QCOMPARE(
+            OCC::FileSystem::toFilesystemPath(u"C:/fooo_bar"_s).native(), OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native());
+
+        // test multiple consecutive slashes
+        path = u"C:///fooo_bar//test///file"_s;
+        QVERIFY(!QFileInfo::exists(path));
+        QCOMPARE(u"C:/fooo_bar/test/file"_s, OCC::FileSystem::canonicalPath(path));
+        QCOMPARE(OCC::FileSystem::toFilesystemPath(u"C:/fooo_bar/test/file"_s).native(),
+            OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native());
+
+        // test trailing slashes
+        path = u"C:/fooo_bar///"_s;
+        QVERIFY(!QFileInfo::exists(path));
+        QCOMPARE(u"C:/fooo_bar"_s, OCC::FileSystem::canonicalPath(path));
+        QCOMPARE(
+            OCC::FileSystem::toFilesystemPath(u"C:/fooo_bar"_s).native(), OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native());
+
+        // test dot segments in middle of path
+        path = u"C:/fooo_bar/./test/./file"_s;
+        QVERIFY(!QFileInfo::exists(path));
+        QCOMPARE(u"C:/fooo_bar/test/file"_s, OCC::FileSystem::canonicalPath(path));
+        QCOMPARE(OCC::FileSystem::toFilesystemPath(u"C:/fooo_bar/test/file"_s).native(),
+            OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native());
+
+        // test mixed slashes on Windows
+        path = u"C:\\fooo_bar/test\\file"_s;
+        QVERIFY(!QFileInfo::exists(path));
+        QCOMPARE(u"C:/fooo_bar/test/file"_s, OCC::FileSystem::canonicalPath(path));
+        QCOMPARE(OCC::FileSystem::toFilesystemPath(u"C:/fooo_bar/test/file"_s).native(),
+            OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native());
+
+        // test UNC path
+        path = u"\\\\server\\share\\path"_s;
+        QCOMPARE(u"//server/share/path"_s, OCC::FileSystem::canonicalPath(path));
+        QCOMPARE(OCC::FileSystem::toFilesystemPath(u"//server/share/path"_s).native(),
+            OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native());
+#else
+        // test non-existing file, which relies on lexical normalization rather than actual canonical path
+        path = u"/fooo_bar"_s;
+        QVERIFY(!QFileInfo::exists(path));
+        QCOMPARE(path, OCC::FileSystem::canonicalPath(path));
+        QCOMPARE(OCC::FileSystem::toFilesystemPath(path).native(), OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native());
+
+        path = u"/fooo_bar/../foo/./../fooo_bar"_s;
+        QVERIFY(!QFileInfo::exists(path));
+        QCOMPARE(u"/fooo_bar"_s, OCC::FileSystem::canonicalPath(path));
+        QCOMPARE(OCC::FileSystem::toFilesystemPath(u"/fooo_bar"_s).native(), OCC::FileSystem::canonicalPath(OCC::FileSystem::toFilesystemPath(path)).native());
+#endif
+    }
 };
 
 QTEST_GUILESS_MAIN(TestUtility)