Merge pull request #6833 from nextcloud/bugfix/fp-account-crash

Fix crash when updating enabled File Provider domains (macOS VFS)
nextcloud · Jun 25, 2024 · 566e8b7 · 566e8b7
2 parents 7db343b + 73fc23f
commit 566e8b7
Show file tree

Hide file tree

Showing 6 changed files with 23 additions and 30 deletions.
diff --git a/src/gui/accountmanager.cpp b/src/gui/accountmanager.cpp
@@ -105,14 +105,13 @@ AccountManager::AccountsRestoreResult AccountManager::restore(const bool alsoRes
         if (!skipSettingsKeys.contains(settings->group())) {
             if (const auto acc = loadAccountHelper(*settings)) {
                 acc->_id = accountId;
-                if (auto accState = AccountState::loadFromSettings(acc, *settings)) {
-                    auto jar = qobject_cast<CookieJar*>(acc->_am->cookieJar());
-                    ASSERT(jar);
-                    if (jar) {
-                        jar->restore(acc->cookieJarPath());
-                    }
-                    addAccountState(accState);
+                const auto accState = new AccountState(acc);
+                const auto jar = qobject_cast<CookieJar*>(acc->_am->cookieJar());
+                Q_ASSERT(jar);
+                if (jar) {
+                    jar->restore(acc->cookieJarPath());
                 }
+                addAccountState(accState);
             }
         } else {
             qCInfo(lcAccountManager) << "Account" << accountId << "is too new, ignoring";
@@ -569,11 +568,12 @@ QString AccountManager::generateFreeAccountId() const
     }
 }
 
-void AccountManager::addAccountState(AccountState *accountState)
+void AccountManager::addAccountState(AccountState *const accountState)
 {
-    QObject::connect(accountState->account().data(),
-        &Account::wantsAccountSaved,
-        this, &AccountManager::saveAccount);
+    Q_ASSERT(accountState);
+    Q_ASSERT(accountState->account());
+
+    QObject::connect(accountState->account().data(), &Account::wantsAccountSaved, this, &AccountManager::saveAccount);
 
     AccountStatePtr ptr(accountState);
     _accounts << ptr;

diff --git a/src/gui/accountmanager.h b/src/gui/accountmanager.h
@@ -126,7 +126,7 @@ public slots:
     [[nodiscard]] QString generateFreeAccountId() const;
 
     // Adds an account to the tracked list, emitting accountAdded()
-    void addAccountState(AccountState *accountState);
+    void addAccountState(AccountState *const accountState);
 
     AccountManager() = default;
     QList<AccountStatePtr> _accounts;

diff --git a/src/gui/accountstate.cpp b/src/gui/accountstate.cpp
@@ -42,7 +42,7 @@ namespace OCC {
 
 Q_LOGGING_CATEGORY(lcAccountState, "nextcloud.gui.account.state", QtInfoMsg)
 
-AccountState::AccountState(AccountPtr account)
+AccountState::AccountState(const AccountPtr &account)
     : QObject()
     , _account(account)
     , _state(AccountState::Disconnected)
@@ -85,12 +85,6 @@ AccountState::AccountState(AccountPtr account)
 
 AccountState::~AccountState() = default;
 
-AccountState *AccountState::loadFromSettings(AccountPtr account, QSettings & /*settings*/)
-{
-    auto accountState = new AccountState(account);
-    return accountState;
-}
-
 AccountPtr AccountState::account() const
 {
     return _account;

diff --git a/src/gui/accountstate.h b/src/gui/accountstate.h
@@ -89,15 +89,9 @@ class AccountState : public QObject, public QSharedData
     using ConnectionStatus = ConnectionValidator::Status;
 
     /// Use the account as parent
-    explicit AccountState(AccountPtr account);
+    explicit AccountState(const AccountPtr &account);
     ~AccountState() override;
 
-    /** Creates an account state from settings and an Account object.
-     *
-     * Use from AccountManager with a prepared QSettings object only.
-     */
-    static AccountState *loadFromSettings(AccountPtr account, QSettings &settings);
-
     AccountPtr account() const;
 
     ConnectionStatus connectionStatus() const;

diff --git a/src/gui/macOS/fileproviderdomainmanager_mac.mm b/src/gui/macOS/fileproviderdomainmanager_mac.mm
@@ -453,13 +453,19 @@ QStringList configuredDomainIds() const
     auto configuredDomains = d->configuredDomainIds();
 
     for (const auto &accountUserIdAtHost : vfsEnabledAccounts) {
+        // If the domain has already been set up for this account, then don't set it up again
         if (configuredDomains.contains(accountUserIdAtHost)) {
             configuredDomains.removeAll(accountUserIdAtHost);
             continue;
         }
 
-        const auto accountState = AccountManager::instance()->accountFromUserId(accountUserIdAtHost);
-        addFileProviderDomainForAccount(accountState.data());
+        if (const auto accountState = AccountManager::instance()->accountFromUserId(accountUserIdAtHost)) {
+            addFileProviderDomainForAccount(accountState.data());
+        } else {
+            qCWarning(lcMacFileProviderDomainManager) << "Could not find account for file provider domain:" << accountUserIdAtHost
+                                                      << "removing account from list of vfs-enabled accounts.";
+            FileProviderSettingsController::instance()->setVfsEnabledForAccount(accountUserIdAtHost, false);
+        }
     }
 
     for (const auto &remainingDomainUserId : configuredDomains) {

diff --git a/src/libsync/account.h b/src/libsync/account.h
@@ -16,6 +16,7 @@
 #ifndef SERVERCONNECTION_H
 #define SERVERCONNECTION_H
 
+#include "accountfwd.h"
 #include "capabilities.h"
 #include "clientsideencryption.h"
 #include "clientstatusreporting.h"
@@ -55,8 +56,6 @@ class ReadPasswordJob;
 namespace OCC {
 
 class AbstractCredentials;
-class Account;
-using AccountPtr = QSharedPointer<Account>;
 class AccessManager;
 class SimpleNetworkJob;
 class PushNotifications;