Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix adding account and skipping folder configuration crash. #7436

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 44 additions & 44 deletions src/gui/accountmanager.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/gui/accountmanager.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/gui/accountmanager.cpp

File src/gui/accountmanager.cpp does not conform to Custom style guidelines. (lines 126, 354, 355, 431)
* Copyright (C) by Olivier Goffart <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
Expand Down Expand Up @@ -123,7 +123,7 @@
if (const auto acc = loadAccountHelper(*settings)) {
acc->_id = accountId;
const auto accState = new AccountState(acc);
const auto jar = qobject_cast<CookieJar*>(acc->_am->cookieJar());
const auto jar = qobject_cast<CookieJar*>(acc->_networkAccessManager->cookieJar());
Q_ASSERT(jar);
if (jar) {
jar->restore(acc->cookieJarPath());
Expand Down Expand Up @@ -305,12 +305,12 @@
qCInfo(lcAccountManager) << "Saved all account settings, status:" << settings->status();
}

void AccountManager::saveAccount(Account *a)
void AccountManager::saveAccount(Account *newAccountData)
{
qCDebug(lcAccountManager) << "Saving account" << a->url().toString();
qCDebug(lcAccountManager) << "Saving account" << newAccountData->url().toString();
const auto settings = ConfigFile::settingsWithGroup(QLatin1String(accountsC));
settings->beginGroup(a->id());
saveAccountHelper(a, *settings, false); // don't save credentials they might not have been loaded yet
settings->beginGroup(newAccountData->id());
saveAccountHelper(newAccountData, *settings, false); // don't save credentials they might not have been loaded yet
settings->endGroup();

settings->sync();
Expand All @@ -328,36 +328,36 @@
qCDebug(lcAccountManager) << "Saved account state settings, status:" << settings->status();
}

void AccountManager::saveAccountHelper(Account *acc, QSettings &settings, bool saveCredentials)
void AccountManager::saveAccountHelper(Account *account, QSettings &settings, bool saveCredentials)
{
qCDebug(lcAccountManager) << "Saving settings to" << settings.fileName();
settings.setValue(QLatin1String(versionC), maxAccountVersion);
settings.setValue(QLatin1String(urlC), acc->_url.toString());
settings.setValue(QLatin1String(davUserC), acc->_davUser);
settings.setValue(QLatin1String(displayNameC), acc->_displayName);
settings.setValue(QLatin1String(serverVersionC), acc->_serverVersion);
settings.setValue(QLatin1String(serverColorC), acc->_serverColor);
settings.setValue(QLatin1String(serverTextColorC), acc->_serverTextColor);
settings.setValue(QLatin1String(serverHasValidSubscriptionC), acc->serverHasValidSubscription());

if (!acc->_skipE2eeMetadataChecksumValidation) {
settings.setValue(QLatin1String(urlC), account->_url.toString());
settings.setValue(QLatin1String(davUserC), account->_davUser);
settings.setValue(QLatin1String(displayNameC), account->davDisplayName());
settings.setValue(QLatin1String(serverVersionC), account->_serverVersion);
settings.setValue(QLatin1String(serverColorC), account->_serverColor);
settings.setValue(QLatin1String(serverTextColorC), account->_serverTextColor);
settings.setValue(QLatin1String(serverHasValidSubscriptionC), account->serverHasValidSubscription());

if (!account->_skipE2eeMetadataChecksumValidation) {
settings.remove(QLatin1String(skipE2eeMetadataChecksumValidationC));
} else {
settings.setValue(QLatin1String(skipE2eeMetadataChecksumValidationC), acc->_skipE2eeMetadataChecksumValidation);
settings.setValue(QLatin1String(skipE2eeMetadataChecksumValidationC), account->_skipE2eeMetadataChecksumValidation);
}
settings.setValue(networkProxySettingC, static_cast<std::underlying_type_t<Account::AccountNetworkProxySetting>>(acc->networkProxySetting()));
settings.setValue(networkProxyTypeC, acc->proxyType());
settings.setValue(networkProxyHostNameC, acc->proxyHostName());
settings.setValue(networkProxyPortC, acc->proxyPort());
settings.setValue(networkProxyNeedsAuthC, acc->proxyNeedsAuth());
settings.setValue(networkProxyUserC, acc->proxyUser());
settings.setValue(networkUploadLimitSettingC, static_cast<std::underlying_type_t<Account::AccountNetworkTransferLimitSetting>>(acc->uploadLimitSetting()));
settings.setValue(networkDownloadLimitSettingC, static_cast<std::underlying_type_t<Account::AccountNetworkTransferLimitSetting>>(acc->downloadLimitSetting()));
settings.setValue(networkUploadLimitC, acc->uploadLimit());
settings.setValue(networkDownloadLimitC, acc->downloadLimit());

const auto proxyPasswordKey = QString(acc->userIdAtHostWithPort() + networkProxyPasswordKeychainKeySuffixC);
if (const auto proxyPassword = acc->proxyPassword(); proxyPassword.isEmpty()) {
settings.setValue(networkProxySettingC, static_cast<std::underlying_type_t<Account::AccountNetworkProxySetting>>(account->networkProxySetting()));
settings.setValue(networkProxyTypeC, account->proxyType());
settings.setValue(networkProxyHostNameC, account->proxyHostName());
settings.setValue(networkProxyPortC, account->proxyPort());
settings.setValue(networkProxyNeedsAuthC, account->proxyNeedsAuth());
settings.setValue(networkProxyUserC, account->proxyUser());
settings.setValue(networkUploadLimitSettingC, static_cast<std::underlying_type_t<Account::AccountNetworkTransferLimitSetting>>(account->uploadLimitSetting()));
settings.setValue(networkDownloadLimitSettingC, static_cast<std::underlying_type_t<Account::AccountNetworkTransferLimitSetting>>(account->downloadLimitSetting()));
settings.setValue(networkUploadLimitC, account->uploadLimit());
settings.setValue(networkDownloadLimitC, account->downloadLimit());

const auto proxyPasswordKey = QString(account->userIdAtHostWithPort() + networkProxyPasswordKeychainKeySuffixC);
if (const auto proxyPassword = account->proxyPassword(); proxyPassword.isEmpty()) {
const auto job = new QKeychain::DeletePasswordJob(Theme::instance()->appName(), this);
job->setKey(proxyPasswordKey);
connect(job, &QKeychain::Job::finished, this, [](const QKeychain::Job *const incomingJob) {
Expand All @@ -384,37 +384,37 @@
job->start();
}

if (acc->_credentials) {
if (account->_credentials) {
if (saveCredentials) {
// Only persist the credentials if the parameter is set, on migration from 1.8.x
// we want to save the accounts but not overwrite the credentials
// (This is easier than asynchronously fetching the credentials from keychain and then
// re-persisting them)
acc->_credentials->persist();
account->_credentials->persist();
}

const auto settingsMapKeys = acc->_settingsMap.keys();
const auto settingsMapKeys = account->_settingsMap.keys();
for (const auto &key : settingsMapKeys) {
if (!acc->_settingsMap.value(key).isValid()) {
if (!account->_settingsMap.value(key).isValid()) {
continue;
}

settings.setValue(key, acc->_settingsMap.value(key));
settings.setValue(key, account->_settingsMap.value(key));
}
settings.setValue(QLatin1String(authTypeC), acc->_credentials->authType());
settings.setValue(QLatin1String(authTypeC), account->_credentials->authType());

// HACK: Save http_user also as user
const auto settingsMap = acc->_settingsMap;
const auto settingsMap = account->_settingsMap;
if (settingsMap.contains(httpUserC) && settingsMap.value(httpUserC).isValid()) {
settings.setValue(userC, settingsMap.value(httpUserC));
}
}

// Save accepted certificates.
settings.beginGroup(QLatin1String(generalC));
qCInfo(lcAccountManager) << "Saving " << acc->approvedCerts().count() << " unknown certs.";
qCInfo(lcAccountManager) << "Saving " << account->approvedCerts().count() << " unknown certs.";
QByteArray certs;
const auto approvedCerts = acc->approvedCerts();
const auto approvedCerts = account->approvedCerts();
for (const auto &cert : approvedCerts) {
certs += cert.toPem() + '\n';
}
Expand All @@ -424,13 +424,13 @@
settings.endGroup();

// Save cookies.
if (acc->_am) {
auto *jar = qobject_cast<CookieJar *>(acc->_am->cookieJar());
if (account->_networkAccessManager) {
const auto jar = qobject_cast<CookieJar *>(account->_networkAccessManager->cookieJar());
if (jar) {
qCInfo(lcAccountManager) << "Saving cookies." << acc->cookieJarPath();
if (!jar->save(acc->cookieJarPath()))
qCInfo(lcAccountManager) << "Saving cookies." << account->cookieJarPath();
if (!jar->save(account->cookieJarPath()))
{
qCWarning(lcAccountManager) << "Failed to save cookies to" << acc->cookieJarPath();
qCWarning(lcAccountManager) << "Failed to save cookies to" << account->cookieJarPath();
}
}
}
Expand Down Expand Up @@ -498,7 +498,7 @@
acc->_davUser = settings.value(QLatin1String(davUserC)).toString();

acc->_settingsMap.insert(QLatin1String(userC), settings.value(userC));
acc->_displayName = settings.value(QLatin1String(displayNameC), "").toString();
acc->setDavDisplayName(settings.value(QLatin1String(displayNameC), "").toString());
const QString authTypePrefix = authType + "_";
const auto settingsChildKeys = settings.childKeys();
for (const auto &key : settingsChildKeys) {
Expand Down
4 changes: 2 additions & 2 deletions src/gui/accountmanager.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

#pragma once

#include "account.h"

Check failure on line 17 in src/gui/accountmanager.h

View workflow job for this annotation

GitHub Actions / build

src/gui/accountmanager.h:17:10 [clang-diagnostic-error]

'account.h' file not found
#include "accountstate.h"

namespace OCC {
Expand Down Expand Up @@ -91,8 +91,8 @@
static void backwardMigrationSettingsKeys(QStringList *deleteKeys, QStringList *ignoreKeys);

public slots:
/// Saves account data, not including the credentials
void saveAccount(OCC::Account *a);
/// Saves account data when adding user, when updating e.g. dav user, not including the credentials
void saveAccount(OCC::Account *newAccountData);

/// Saves account state data, not including the account
void saveAccountState(OCC::AccountState *a);
Expand Down
4 changes: 3 additions & 1 deletion src/gui/accountstate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,10 @@ void AccountState::signOutByUi()

void AccountState::freshConnectionAttempt()
{
if (isConnected())
if (isConnected()) {
setState(Disconnected);
}

checkConnectivity();
}

Expand Down
11 changes: 3 additions & 8 deletions src/gui/application.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ Application::Application(int &argc, char **argv)
}

_theme->setSystrayUseMonoIcons(ConfigFile().monoIcons());
connect(_theme, &Theme::systrayUseMonoIconsChanged, this, &Application::slotUseMonoIconsChanged);
connect(_theme, &Theme::systrayUseMonoIconsChanged, _gui, &ownCloudGui::slotComputeOverallSyncStatus);

#if defined(Q_OS_WIN)
_shellExtensionsServer.reset(new ShellExtensionsServer);
Expand Down Expand Up @@ -605,7 +605,7 @@ void Application::slotAccountStateRemoved(AccountState *accountState)
{
if (_gui) {
disconnect(accountState, &AccountState::stateChanged,
_gui.data(), &ownCloudGui::slotAccountStateChanged);
_gui.data(), &ownCloudGui::slotComputeOverallSyncStatus);
disconnect(accountState->account().data(), &Account::serverVersionChanged,
_gui.data(), &ownCloudGui::slotTrayMessageIfServerUnsupported);
}
Expand All @@ -627,7 +627,7 @@ void Application::slotAccountStateRemoved(AccountState *accountState)
void Application::slotAccountStateAdded(AccountState *accountState)
{
connect(accountState, &AccountState::stateChanged,
_gui.data(), &ownCloudGui::slotAccountStateChanged);
_gui.data(), &ownCloudGui::slotComputeOverallSyncStatus);
connect(accountState->account().data(), &Account::serverVersionChanged,
_gui.data(), &ownCloudGui::slotTrayMessageIfServerUnsupported);
connect(accountState, &AccountState::termsOfServiceChanged,
Expand Down Expand Up @@ -732,11 +732,6 @@ void Application::setupLogging()
qCInfo(lcApplication) << "Arguments:" << qApp->arguments();
}

void Application::slotUseMonoIconsChanged(bool)
{
_gui->slotComputeOverallSyncStatus();
}

void Application::slotParseMessage(const QString &msg, QObject *)
{
if (msg.startsWith(QLatin1String("MSG_PARSEOPTIONS:"))) {
Expand Down
1 change: 0 additions & 1 deletion src/gui/application.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ public slots:
protected slots:
void slotParseMessage(const QString &, QObject *);
void slotCheckConnection();
void slotUseMonoIconsChanged(bool);
void slotCleanup();
void slotAccountStateAdded(OCC::AccountState *accountState);
void slotAccountStateRemoved(OCC::AccountState *accountState);
Expand Down
16 changes: 3 additions & 13 deletions src/gui/owncloudgui.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/gui/owncloudgui.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/gui/owncloudgui.cpp

File src/gui/owncloudgui.cpp does not conform to Custom style guidelines. (lines 121)
* Copyright (C) by Klaas Freitag <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
Expand Down Expand Up @@ -118,6 +118,8 @@

FolderMan *folderMan = FolderMan::instance();
connect(folderMan, &FolderMan::folderSyncStateChange, this, &ownCloudGui::slotSyncStateChange);
connect(folderMan, &FolderMan::folderSyncStateChange, this, &ownCloudGui::slotComputeOverallSyncStatus);


#ifdef BUILD_FILE_PROVIDER_MODULE
connect(Mac::FileProvider::instance()->socketServer(), &Mac::FileProviderSocketServer::syncStateChanged, this, &ownCloudGui::slotComputeOverallSyncStatus);
Expand Down Expand Up @@ -240,8 +242,6 @@

void ownCloudGui::slotSyncStateChange(Folder *folder)
{
slotComputeOverallSyncStatus();

if (!folder) {
return; // Valid, just a general GUI redraw was needed.
}
Expand All @@ -258,21 +258,11 @@
}
}

void ownCloudGui::slotFoldersChanged()
{
slotComputeOverallSyncStatus();
}

void ownCloudGui::slotOpenPath(const QString &path)
{
showInFileManager(path);
}

void ownCloudGui::slotAccountStateChanged()
{
slotComputeOverallSyncStatus();
}

void ownCloudGui::slotTrayMessageIfServerUnsupported(Account *account)
{
if (account->serverVersionUnsupported()) {
Expand Down Expand Up @@ -371,7 +361,7 @@
#else
QStringList messages;
messages.append(tr("Disconnected from accounts:"));
for (const auto &accountState : problemAccounts) {
for (const auto &accountState : qAsConst(problemAccounts)) {
QString message = tr("Account %1: %2").arg(accountState->account()->displayName(), accountState->stateString(accountState->state()));
if (!accountState->connectionErrors().empty()) {
message += QLatin1String("\n");
Expand Down
2 changes: 0 additions & 2 deletions src/gui/owncloudgui.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ public slots:
void slotFolderOpenAction(const QString &alias);
void slotUpdateProgress(const QString &folder, const OCC::ProgressInfo &progress);
void slotShowGuiMessage(const QString &title, const QString &message);
void slotFoldersChanged();
void slotShowSettings();
void slotShowSyncProtocol();
void slotShutdown();
Expand All @@ -92,7 +91,6 @@ public slots:
void slotSettingsDialogActivated();
void slotHelp();
void slotOpenPath(const QString &path);
void slotAccountStateChanged();
void slotTrayMessageIfServerUnsupported(OCC::Account *account);
void slotNeedToAcceptTermsOfService(OCC::AccountPtr account,
OCC::AccountState::State state);
Expand Down
26 changes: 15 additions & 11 deletions src/gui/owncloudsetupwizard.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/gui/owncloudsetupwizard.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/gui/owncloudsetupwizard.cpp

File src/gui/owncloudsetupwizard.cpp does not conform to Custom style guidelines. (lines 88)
* Copyright (C) by Klaas Freitag <[email protected]>
* Copyright (C) by Krzesimir Nowak <[email protected]>
*
Expand All @@ -13,7 +13,7 @@
* for more details.
*/

#include <QAbstractButton>

Check failure on line 16 in src/gui/owncloudsetupwizard.cpp

View workflow job for this annotation

GitHub Actions / build

src/gui/owncloudsetupwizard.cpp:16:10 [clang-diagnostic-error]

'QAbstractButton' file not found
#include <QtCore>
#include <QProcess>
#include <QMessageBox>
Expand Down Expand Up @@ -66,7 +66,7 @@
_ocWizard->deleteLater();
}

static QPointer<OwncloudSetupWizard> wiz = nullptr;
static QPointer<OwncloudSetupWizard> owncloudSetupWizard = nullptr;

Check warning on line 69 in src/gui/owncloudsetupwizard.cpp

View workflow job for this annotation

GitHub Actions / build

src/gui/owncloudsetupwizard.cpp:69:38 [cppcoreguidelines-avoid-non-const-global-variables]

variable 'owncloudSetupWizard' is non-const and globally accessible, consider making it const

void OwncloudSetupWizard::runWizard(QObject *obj, const char *amember, QWidget *parent)
{
Expand All @@ -78,25 +78,26 @@

Theme::instance()->setStartLoginFlowAutomatically(true);
}
if (!wiz.isNull()) {
if (!owncloudSetupWizard.isNull()) {
bringWizardToFrontIfVisible();
return;
}

wiz = new OwncloudSetupWizard(parent);
connect(wiz, SIGNAL(ownCloudWizardDone(int)), obj, amember);
connect(wiz->_ocWizard, &OwncloudWizard::wizardClosed, obj, [] { wiz.clear(); });
owncloudSetupWizard = new OwncloudSetupWizard(parent);
connect(owncloudSetupWizard, SIGNAL(ownCloudWizardDone(int)), obj, amember);
connect(owncloudSetupWizard->_ocWizard, &OwncloudWizard::wizardClosed, obj, [] { owncloudSetupWizard.clear(); });

FolderMan::instance()->setSyncEnabled(false);
wiz->startWizard();
owncloudSetupWizard->startWizard();
}

bool OwncloudSetupWizard::bringWizardToFrontIfVisible()
{
if (wiz.isNull()) {
if (owncloudSetupWizard.isNull()) {
return false;

Check warning on line 97 in src/gui/owncloudsetupwizard.cpp

View workflow job for this annotation

GitHub Actions / build

src/gui/owncloudsetupwizard.cpp:97:16 [readability-simplify-boolean-expr]

redundant boolean literal in conditional return statement
}

ownCloudGui::raiseDialog(wiz->_ocWizard);
ownCloudGui::raiseDialog(owncloudSetupWizard->_ocWizard);
return true;
}

Expand Down Expand Up @@ -700,8 +701,8 @@
}

// notify others.
_ocWizard->done(QWizard::Accepted);
_ocWizard->done(result);
emit ownCloudWizardDone(result);

Check warning on line 705 in src/gui/owncloudsetupwizard.cpp

View workflow job for this annotation

GitHub Actions / build

src/gui/owncloudsetupwizard.cpp:705:10 [cppcoreguidelines-init-variables]

variable 'ownCloudWizardDone' is not initialized
}

void OwncloudSetupWizard::slotSkipFolderConfiguration()
Expand All @@ -710,8 +711,11 @@

disconnect(_ocWizard, &OwncloudWizard::basicSetupFinished,
this, &OwncloudSetupWizard::slotAssistantFinished);
_ocWizard->close();

_ocWizard->done(QDialog::Rejected);

// Accept to check connectivity, only skip folder setup
emit ownCloudWizardDone(QDialog::Accepted);

Check warning on line 718 in src/gui/owncloudsetupwizard.cpp

View workflow job for this annotation

GitHub Actions / build

src/gui/owncloudsetupwizard.cpp:718:10 [modernize-use-trailing-return-type]

use a trailing return type for this function
}

AccountState *OwncloudSetupWizard::applyAccountChanges()
Expand All @@ -727,7 +731,7 @@
auto manager = AccountManager::instance();

auto newState = manager->addAccount(newAccount);
manager->save();
manager->saveAccount(newAccount.data());
return newState;
}

Expand Down
2 changes: 1 addition & 1 deletion src/gui/settingsdialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ void SettingsDialog::accountAdded(AccountState *s)
_actionForAccount.insert(s->account().data(), accountAction);
accountAction->trigger();

connect(accountSettings, &AccountSettings::folderChanged, _gui, &ownCloudGui::slotFoldersChanged);
connect(accountSettings, &AccountSettings::folderChanged, _gui, &ownCloudGui::slotComputeOverallSyncStatus);
connect(accountSettings, &AccountSettings::openFolderAlias,
_gui, &ownCloudGui::slotFolderOpenAction);
connect(accountSettings, &AccountSettings::showIssuesList, this, &SettingsDialog::showIssuesList);
Expand Down
1 change: 1 addition & 0 deletions src/gui/tray/TrayFoldersMenuButton.qml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
* or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* for more details.
*/
pragma NativeMethodBehavior: AcceptThisObject
import QtQuick
import QtQuick.Controls
import QtQuick.Layouts
Expand Down
Loading
Loading