Folder: Add remoteFolderTrailingSlash()

There were cases where the "/" exception wasn't handled correctly
and there'd be extra slashes in generated paths.

9 files changed, 44 insertions, 31 deletions

diff --git a/src/common/vfs.h b/src/common/vfs.h
index 179f0174d..c57e9349f 100644
--- a/src/common/vfs.h
+++ b/src/common/vfs.h
@@ -37,9 +37,16 @@ class SyncFileItem;
 /** Collection of parameters for initializing a Vfs instance. */
 struct OCSYNC_EXPORT VfsSetupParams
 {
-    /// The full path to the folder on the local filesystem
+    /** The full path to the folder on the local filesystem
+     *
+     * Always ends with /.
+     */
     QString filesystemPath;
-    /// The path to the synced folder on the account
+
+    /** The path to the synced folder on the account
+     *
+     * Always ends with /.
+     */
     QString remotePath;
 
     /// Account url, credentials etc for network calls
diff --git a/src/gui/accountsettings.cpp b/src/gui/accountsettings.cpp
index e16bcde95..3002945c0 100644
--- a/src/gui/accountsettings.cpp
+++ b/src/gui/accountsettings.cpp
@@ -279,10 +279,7 @@ void AccountSettings::slotCustomContextMenuRequested(const QPoint &pos)
         ac = menu->addAction(tr("Open folder in browser"));
         auto info = _model->infoForIndex(index);
         ASSERT(info);
-        QString path = info->_folder->remotePath();
-        if (!path.endsWith(QLatin1Char('/'))) {
-                path += QLatin1Char('/');
-        }
+        QString path = info->_folder->remotePathTrailingSlash();
         path += info->_path;
         connect(ac, &QAction::triggered, this, [this, path]{
             fetchPrivateLinkUrl(_accountState->account(), path, {}, this, [](const QString &url) {
diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp
index c00838939..b22bbde12 100644
--- a/src/gui/folder.cpp
+++ b/src/gui/folder.cpp
@@ -256,6 +256,14 @@ QString Folder::remotePath() const
     return _definition.targetPath;
 }
 
+QString Folder::remotePathTrailingSlash() const
+{
+    QString result = remotePath();
+    if (!result.endsWith('/'))
+        result.append('/');
+    return result;
+}
+
 QUrl Folder::remoteUrl() const
 {
     return Utility::concatUrlPath(_accountState->account()->davUrl(), remotePath());
@@ -463,7 +471,7 @@ void Folder::startVfs()
 
     VfsSetupParams vfsParams;
     vfsParams.filesystemPath = path();
-    vfsParams.remotePath = remotePath();
+    vfsParams.remotePath = remotePathTrailingSlash();
     vfsParams.account = _accountState->account();
     vfsParams.journal = &_journal;
     vfsParams.providerName = Theme::instance()->appNameGUI();
@@ -1304,7 +1312,7 @@ bool FolderDefinition::load(QSettings &settings, const QString &alias,
     }
 
     // Old settings can contain paths with native separators. In the rest of the
-    // code we assum /, so clean it up now.
+    // code we assume /, so clean it up now.
     folder->localPath = prepareLocalPath(folder->localPath);
 
     // Target paths also have a convention
diff --git a/src/gui/folder.h b/src/gui/folder.h
index 8d7877bf1..4fa7ccb85 100644
--- a/src/gui/folder.h
+++ b/src/gui/folder.h
@@ -57,11 +57,11 @@ public:
 
     /// The name of the folder in the ui and internally
     QString alias;
-    /// path on local machine
+    /// path on local machine (always trailing /)
     QString localPath;
     /// path to the journal, usually relative to localPath
     QString journalPath;
-    /// path on remote
+    /// path on remote (usually no trailing /, exception "/")
     QString targetPath;
     /// whether the folder is paused
     bool paused;
@@ -94,7 +94,7 @@ public:
     /// Ensure / as separator and trailing /.
     static QString prepareLocalPath(const QString &path);
 
-    /// Ensure starting / and no ending /.
+    /// Remove ending /, then ensure starting '/': so "/foo/bar" and "/".
     static QString prepareTargetPath(const QString &path);
 
     /// journalPath relative to localPath.
@@ -152,10 +152,15 @@ public:
     QString cleanPath() const;
 
     /**
-     * remote folder path
+     * remote folder path, usually without trailing /, exception "/"
      */
     QString remotePath() const;
 
+    /**
+     * remote folder path, always with a trailing /
+     */
+    QString remotePathTrailingSlash() const;
+
     void setNavigationPaneClsid(const QUuid &clsid) { _definition.navigationPaneClsid = clsid; }
     QUuid navigationPaneClsid() const { return _definition.navigationPaneClsid; }
 
diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp
index 64cfc06be..71bff8cd0 100644
--- a/src/gui/folderman.cpp
+++ b/src/gui/folderman.cpp
@@ -1060,16 +1060,20 @@ QStringList FolderMan::findFileInLocalFolders(const QString &relPath, const Acco
 {
     QStringList re;
 
+    // We'll be comparing against Folder::remotePath which always starts with /
+    QString serverPath = relPath;
+    if (!serverPath.startsWith('/'))
+        serverPath.prepend('/');
+
     foreach (Folder *folder, this->map().values()) {
         if (acc != 0 && folder->accountState()->account() != acc) {
             continue;
         }
-        QString path = folder->cleanPath();
-        QString remRelPath;
-        // cut off the remote path from the server path.
-        remRelPath = relPath.mid(folder->remotePath().length());
-        path += "/";
-        path += remRelPath;
+        if (!serverPath.startsWith(folder->remotePath()))
+            continue;
+
+        QString path = folder->cleanPath() + '/';
+        path += serverPath.midRef(folder->remotePathTrailingSlash().length());
         if (QFile::exists(path)) {
             re.append(path);
         }
diff --git a/src/gui/folderstatusmodel.cpp b/src/gui/folderstatusmodel.cpp
index d6fe7a671..4ea8150f1 100644
--- a/src/gui/folderstatusmodel.cpp
+++ b/src/gui/folderstatusmodel.cpp
@@ -562,11 +562,8 @@ void FolderStatusModel::fetchMore(const QModelIndex &parent)
     if (!info || info->_fetched || info->_fetchingJob)
         return;
     info->resetSubs(this, parent);
-    QString path = info->_folder->remotePath();
+    QString path = info->_folder->remotePathTrailingSlash();
     if (info->_path != QLatin1String("/")) {
-        if (!path.endsWith(QLatin1Char('/'))) {
-            path += QLatin1Char('/');
-        }
         path += info->_path;
     }
     LsColJob *job = new LsColJob(_accountState->account(), path, this);
diff --git a/src/gui/folderwizard.cpp b/src/gui/folderwizard.cpp
index b3f0199b1..e21a2afaf 100644
--- a/src/gui/folderwizard.cpp
+++ b/src/gui/folderwizard.cpp
@@ -440,13 +440,10 @@ bool FolderWizardRemotePath::isComplete() const
         if (f->accountState()->account() != _account) {
             continue;
         }
-        QString curDir = f->remotePath();
-        if (!curDir.startsWith(QLatin1Char('/'))) {
-            curDir.prepend(QLatin1Char('/'));
-        }
+        QString curDir = f->remotePathTrailingSlash();
         if (QDir::cleanPath(dir) == QDir::cleanPath(curDir)) {
             warnStrings.append(tr("This folder is already being synced."));
-        } else if (dir.startsWith(curDir + QLatin1Char('/'))) {
+        } else if (dir.startsWith(curDir)) {
             warnStrings.append(tr("You are already syncing <i>%1</i>, which is a parent folder of <i>%2</i>.").arg(Utility::escape(curDir), Utility::escape(dir)));
         }
     }
diff --git a/src/gui/sharemanager.cpp b/src/gui/sharemanager.cpp
index 4bc32182a..555dd03f8 100644
--- a/src/gui/sharemanager.cpp
+++ b/src/gui/sharemanager.cpp
@@ -37,9 +37,7 @@ static void updateFolder(const AccountPtr &account, const QString &path)
         if (path.startsWith(folderPath) && (path == folderPath || folderPath.endsWith('/') || path[folderPath.size()] == '/')) {
             // Workaround the fact that the server does not invalidate the etags of parent directories
             // when something is shared.
-            auto relative = path.midRef(folderPath.size());
-            if (relative.startsWith('/'))
-                relative = relative.mid(1);
+            auto relative = path.midRef(f->remotePathTrailingSlash().length());
             f->journalDb()->avoidReadFromDbOnNextSync(relative.toString());
 
             // Schedule a sync so it can update the remote permission flag and let the socket API
diff --git a/test/syncenginetestutils.h b/test/syncenginetestutils.h
index 549d0092f..bf6f67f96 100644
--- a/test/syncenginetestutils.h
+++ b/test/syncenginetestutils.h
@@ -1161,7 +1161,7 @@ public:
 
         OCC::VfsSetupParams vfsParams;
         vfsParams.filesystemPath = localPath();
-        vfsParams.remotePath = "";
+        vfsParams.remotePath = "/";
         vfsParams.account = _account;
         vfsParams.journal = _journalDb.get();
         vfsParams.providerName = "OC-TEST";