Merge remote-tracking branch 'origin/5'

owncloud · Oct 27, 2023 · 8613814 · 8613814
2 parents 31f0459 + 37b37de
commit 8613814
Show file tree

Hide file tree

Showing 46 changed files with 1,620 additions and 1,338 deletions.
diff --git a/.github/workflows/.craft.ps1 b/.github/workflows/.craft.ps1
@@ -1,5 +1,5 @@
 if ($IsWindows) {
-    $python = (Get-Command py).Source
+    $python=(py -V:3.11 -c "import sys; print(sys.executable)")
 } else {
     $python = (Get-Command python3).Source
 }

diff --git a/changelog/unreleased/11288 b/changelog/unreleased/11288
@@ -0,0 +1,8 @@
+Bugfix: Fix crash on start-up when starting shell integration
+
+A possible crash has been fixed that could occur during start-up, when
+the shell integration started doing requests before the client itself
+completed starting up.
+
+https://github.com/owncloud/client/issues/11280
+https://github.com/owncloud/client/pull/11288
diff --git a/changelog/unreleased/11308 b/changelog/unreleased/11308
@@ -0,0 +1,6 @@
+Bugfix: Properly schedule the sync after an account was added
+
+We fixed a bug where a folder was scheduled to be synced before the account reported it was ready.
+This resulted in the sync having no effect and the folder was then only synced once we polled the etag.
+
+https://github.com/owncloud/client/issues/11308
diff --git a/changelog/unreleased/11313 b/changelog/unreleased/11313
@@ -0,0 +1,6 @@
+Bugfix: Don't start credentials save jobs during shutdown
+
+Due to a bug we "re saved" the credentials during application shutdown.
+As the application was quitting while the jobs where running we might have encountered corruped credentials or crashes.
+
+https://github.com/owncloud/client/pull/11313
diff --git a/src/gui/accountsettings.cpp b/src/gui/accountsettings.cpp
@@ -828,9 +828,10 @@ void AccountSettings::slotAccountStateChanged()
         _toggleSignInOutAction->setText(tr("Log out"));
     }
 
-    ui->addButton->setEnabled(state == AccountState::Connected);
     if (state == AccountState::Connected) {
         ui->_folderList->setItemsExpandable(true);
+        ui->addButton->setEnabled(true);
+
         if (_accountState->supportsSpaces()) {
             ui->addButton->setText(tr("Add Space"));
             ui->addButton->setToolTip(tr("Click this button to add a Space."));
@@ -840,8 +841,15 @@ void AccountSettings::slotAccountStateChanged()
         }
     } else {
         ui->_folderList->setItemsExpandable(false);
-        ui->addButton->setText(tr("Add Folder"));
-        ui->addButton->setToolTip(tr("You need to be connected to add a folder."));
+        ui->addButton->setEnabled(false);
+
+        if (_accountState->supportsSpaces()) {
+            ui->addButton->setText(tr("Add Space"));
+            ui->addButton->setToolTip(tr("You need to be connected to add a Space."));
+        } else {
+            ui->addButton->setText(tr("Add Folder"));
+            ui->addButton->setToolTip(tr("You need to be connected to add a folder."));
+        }
 
         /* check if there are expanded root items, if so, close them */
         ui->_folderList->collapseAll();

diff --git a/src/gui/application.cpp b/src/gui/application.cpp
@@ -174,7 +174,7 @@ void Application::slotAccountStateAdded(AccountStatePtr accountState) const
         });
 
     // Hook up the folder manager slots to the account state's signals:
-    connect(accountState.data(), &AccountState::stateChanged, FolderMan::instance(), &FolderMan::slotAccountStateChanged);
+    connect(accountState.data(), &AccountState::isConnectedChanged, FolderMan::instance(), &FolderMan::slotIsConnectedChanged);
     connect(accountState->account().data(), &Account::serverVersionChanged, FolderMan::instance(),
         [account = accountState->account().data()] { FolderMan::instance()->slotServerVersionChanged(account); });
     accountState->checkConnectivity();
@@ -186,7 +186,10 @@ void Application::slotCleanup()
     _gui->slotShutdown();
     delete _gui;
 
-    AccountManager::instance()->save();
+    // by now the credentials are supposed to be persisted
+    // don't start async credentials jobs during shutdown
+    AccountManager::instance()->save(false);
+
     FolderMan::instance()->unloadAndDeleteAllFolders();
 
     // Remove the account from the account manager so it can be deleted.

diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp
@@ -365,7 +365,7 @@ void FolderMan::slotSyncOnceFileUnlocks(const QString &path, FileSystem::LockMod
     _lockWatcher->addFile(path, mode);
 }
 
-void FolderMan::slotAccountStateChanged()
+void FolderMan::slotIsConnectedChanged()
 {
     AccountStatePtr accountState(qobject_cast<AccountState *>(sender()));
     if (!accountState) {

diff --git a/src/gui/folderman.h b/src/gui/folderman.h
@@ -251,7 +251,7 @@ public slots:
      * Schedules folders of newly connected accounts, terminates and
      * de-schedules folders of disconnected accounts.
      */
-    void slotAccountStateChanged();
+    void slotIsConnectedChanged();
 
     /**
      * restart the client as soon as it is possible, ie. no folders syncing.

diff --git a/src/gui/main.cpp b/src/gui/main.cpp
@@ -26,6 +26,7 @@
 #include "common/version.h"
 #include "gui/translations.h"
 #include "libsync/logger.h"
+#include "socketapi/socketapi.h"
 
 #include <kdsingleapplication.h>
 
@@ -359,7 +360,7 @@ int main(int argc, char **argv)
         return -1;
     }
 
-    FolderMan::instance()->setSyncEnabled(true);
+    folderManager->setSyncEnabled(true);
 
     auto ocApp = Application::createInstance(platform.get(), options.debugMode);
 
@@ -412,5 +413,8 @@ int main(int argc, char **argv)
         QTimer::singleShot(0, ocApp->gui(), &ownCloudGui::runNewAccountWizard);
     }
 
+    // Now that everything is up and running, start accepting connections/requests from the shell integration.
+    folderManager->socketApi()->startShellIntegration();
+
     return app.exec();
 }
diff --git a/src/gui/scheduling/etagwatcher.cpp b/src/gui/scheduling/etagwatcher.cpp
@@ -83,11 +83,13 @@ void ETagWatcher::updateEtag(Folder *f, const QString &etag)
     if (OC_ENSURE_NOT(etag.isEmpty())) {
         auto &info = _lastEtagJob[f];
         if (f->canSync() && info.etag != etag) {
-            qCDebug(lcEtagWatcher) << "Scheduling sync of" << f->displayName() << "due to an etag change";
+            qCDebug(lcEtagWatcher) << "Scheduling sync of" << f->displayName() << f->path() << "due to an etag change";
             info.etag = etag;
             _folderMan->scheduler()->enqueueFolder(f);
         }
         info.lastUpdate.reset();
+    } else {
+        qCWarning(lcEtagWatcher) << "Invalid empty etag received for" << f->displayName() << f->path();
     }
 }
 
@@ -102,11 +104,15 @@ void ETagWatcher::startOC10EtagJob(Folder *f)
             requestEtagJob->setTimeout(pollTimeoutC);
             connect(requestEtagJob, &RequestEtagJob::finishedSignal, this, [requestEtagJob, f, this] {
                 if (requestEtagJob->httpStatusCode() == 207) {
-                    updateEtag(f, requestEtagJob->etag());
-                    f->accountState()->tagLastSuccessfullETagRequest(requestEtagJob->responseQTimeStamp());
+                    if (OC_ENSURE_NOT(requestEtagJob->etag().isEmpty())) {
+                        f->accountState()->tagLastSuccessfullETagRequest(requestEtagJob->responseQTimeStamp());
+                        updateEtag(f, requestEtagJob->etag());
+                    } else {
+                        qCWarning(lcEtagWatcher) << "Invalid empty etag received for" << f->displayName() << f->path() << requestEtagJob;
+                    }
                 }
             });
-            qCDebug(lcEtagWatcher) << "Starting etag check for folder" << f->displayName();
+            qCDebug(lcEtagWatcher) << "Starting etag check for folder" << f->displayName() << f->path();
             requestEtagJob->start();
         }
     }

diff --git a/src/gui/socketapi/socketapi.cpp b/src/gui/socketapi/socketapi.cpp
@@ -140,39 +140,28 @@ SocketApi::SocketApi(QObject *parent)
     qRegisterMetaType<QSharedPointer<SocketApiJob>>("QSharedPointer<SocketApiJob>");
     qRegisterMetaType<QSharedPointer<SocketApiJobV2>>("QSharedPointer<SocketApiJobV2>");
 
-    const QString socketPath = Utility::socketApiSocketPath();
+    _socketPath = Utility::socketApiSocketPath();
 
     // Remove any old socket that might be lying around:
-    SocketApiServer::removeServer(socketPath);
+    SocketApiServer::removeServer(_socketPath);
 
     // Create the socket path:
     if (!Utility::isMac()) {
         // Not on macOS: there the directory is there, and created for us by the sandboxing
         // environment, because we belong to an App Group.
-        QFileInfo info(socketPath);
+        QFileInfo info(_socketPath);
         if (!info.dir().exists()) {
             bool result = info.dir().mkpath(QStringLiteral("."));
             qCDebug(lcSocketApi) << "creating" << info.dir().path() << result;
             if (result) {
-                QFile::setPermissions(socketPath,
-                    QFile::Permissions(QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOwner));
+                QFile::setPermissions(_socketPath, QFile::Permissions(QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOwner));
             }
         }
     }
 
     // Wire up the server instance to us, so we can accept new connections:
     connect(&_localServer, &SocketApiServer::newConnection, this, &SocketApi::slotNewConnection);
 
-    // Start listeneing:
-    if (_localServer.listen(socketPath)) {
-        qCInfo(lcSocketApi) << "server started, listening at " << socketPath;
-    } else {
-        qCWarning(lcSocketApi) << "can't start server" << socketPath;
-    }
-
-    // Now we're ready to start the native shell integration:
-    Utility::startShellIntegration();
-
     connect(AccountManager::instance(), &AccountManager::accountRemoved, this, [this](const auto &accountState) {
         if (_registeredAccounts.contains(accountState->account())) {
             unregisterAccount(accountState->account());
@@ -189,6 +178,19 @@ SocketApi::~SocketApi()
     _listeners.clear();
 }
 
+void SocketApi::startShellIntegration()
+{
+    // Start listeneing:
+    if (_localServer.listen(_socketPath)) {
+        qCInfo(lcSocketApi) << "server started, listening at " << _socketPath;
+    } else {
+        qCWarning(lcSocketApi) << "can't start server" << _socketPath;
+    }
+
+    // Now we're ready to start the native shell integration:
+    Utility::startShellIntegration();
+}
+
 void SocketApi::slotNewConnection()
 {
     // Note that on macOS this is not actually a line-based QIODevice, it's a SocketApiSocket which is our

diff --git a/src/gui/socketapi/socketapi.h b/src/gui/socketapi/socketapi.h
@@ -53,6 +53,8 @@ class SocketApi : public QObject
     explicit SocketApi(QObject *parent = nullptr);
     ~SocketApi() override;
 
+    void startShellIntegration();
+
 public slots:
     void registerAccount(const AccountPtr &a);
     void unregisterAccount(const AccountPtr &a);
@@ -155,6 +157,7 @@ private slots:
 
     QString buildRegisterPathMessage(const QString &path);
 
+    QString _socketPath;
     QSet<Folder *> _registeredFolders;
     QSet<AccountPtr> _registeredAccounts;
     QMap<SocketApiSocket *, QSharedPointer<SocketListener>> _listeners;

diff --git a/src/libsync/creds/credentialmanager.cpp b/src/libsync/creds/credentialmanager.cpp
@@ -19,6 +19,7 @@ using namespace OCC;
 Q_LOGGING_CATEGORY(lcCredentialsManager, "sync.credentials.manager", QtDebugMsg)
 
 namespace {
+constexpr auto tiemoutC = 5s;
 QString credentialKeyC()
 {
     return QStringLiteral("%1_credentials").arg(Theme::instance()->appName());
@@ -67,18 +68,36 @@ QKeychain::Job *CredentialManager::set(const QString &key, const QVariant &data)
     qCInfo(lcCredentialsManager) << "set" << scopedKey(this, key);
     auto writeJob = new QKeychain::WritePasswordJob(Theme::instance()->appName());
     writeJob->setKey(scopedKey(this, key));
-    connect(writeJob, &QKeychain::WritePasswordJob::finished, this, [writeJob, key, this] {
+
+    auto timer = new QTimer(writeJob);
+    timer->setInterval(tiemoutC);
+    timer->setSingleShot(true);
+
+    auto timedOut = std::make_unique<bool>(false);
+    connect(timer, &QTimer::timeout, writeJob, [writeJob, timedOut = timedOut.get()] {
+        *timedOut = true;
+        Q_EMIT writeJob->finished(writeJob);
+        writeJob->deleteLater();
+    });
+    connect(writeJob, &QKeychain::WritePasswordJob::finished, this, [writeJob, timer, key, timedOut = std::move(timedOut), this] {
+        timer->stop();
         if (writeJob->error() == QKeychain::NoError) {
-            qCInfo(lcCredentialsManager) << "added" << scopedKey(this, key);
-            // just a list, the values don't matter
-            credentialsList().setValue(key, true);
+            if (*timedOut.get()) {
+                qCInfo(lcCredentialsManager) << "set" << writeJob->key() << "timed out";
+            } else {
+                qCInfo(lcCredentialsManager) << "added" << writeJob->key();
+                // just a list, the values don't matter
+                credentialsList().setValue(key, true);
+            }
         } else {
-            qCWarning(lcCredentialsManager) << "Failed to set:" << scopedKey(this, key) << writeJob->errorString();
+            qCWarning(lcCredentialsManager) << "Failed to set:" << writeJob->key() << writeJob->errorString();
         }
     });
     writeJob->setBinaryData(QCborValue::fromVariant(data).toCbor());
     // start is delayed so we can directly call it
     writeJob->start();
+    timer->start();
+
     return writeJob;
 }
 
@@ -183,6 +202,7 @@ void CredentialJob::start()
     if (!_parent->contains(_key)) {
         _error = QKeychain::EntryNotFound;
         // QKeychain is started delayed, emit the signal delayed to make sure we are connected
+        qCDebug(lcCredentialsManager) << "We don't know" << _key << "skipping retrieval from keychain";
         QTimer::singleShot(0, this, &CredentialJob::finished);
         return;
     }

diff --git a/test/gui/shared/scripts/helpers/SetupClientHelper.py b/test/gui/shared/scripts/helpers/SetupClientHelper.py
@@ -69,6 +69,10 @@ def getResourcePath(resource='', user='', space=''):
     )
 
 
+def getTempResourcePath(resourceName):
+    return join(get_config('tempFolderPath'), resourceName)
+
+
 def getCurrentUserSyncPath():
     return get_config('currentUserSyncPath')
 

diff --git a/test/gui/shared/scripts/pageObjects/Activity.py b/test/gui/shared/scripts/pageObjects/Activity.py
@@ -26,15 +26,17 @@ class Activity:
         "visible": 1,
     }
 
+    @staticmethod
+    def getTabObject(tab_index):
+        return {
+            "container": Activity.SUBTAB_CONTAINER,
+            "index": tab_index,
+            "type": "TabItem",
+        }
+
     @staticmethod
     def getTabText(tab_index):
-        return squish.waitForObjectExists(
-            {
-                "container": Activity.SUBTAB_CONTAINER,
-                "index": tab_index,
-                "type": "TabItem",
-            }
-        ).text
+        return squish.waitForObjectExists(Activity.getTabObject(tab_index)).text
 
     @staticmethod
     def getNotSyncedFileSelector(resource):
@@ -69,7 +71,13 @@ def clickTab(tabName):
 
             if tabName in tabText:
                 tabFound = True
-                squish.clickTab(squish.waitForObject(Activity.SUBTAB), tabText)
+                squish.mouseClick(
+                    squish.waitForObjectExists(Activity.getTabObject(index)),
+                    0,
+                    0,
+                    squish.Qt.NoModifier,
+                    squish.Qt.LeftButton,
+                )
                 break
 
         if not tabFound:
@@ -85,7 +93,6 @@ def checkFileExist(filename):
 
     @staticmethod
     def checkBlackListedResourceExist(filename):
-
         result = squish.waitFor(
             lambda: Activity.isResourceBlackListed(filename),
             get_config('maxSyncTimeout') * 1000,