From 2421003841717c529d76d1b654d9d62379585232 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Wed, 26 Oct 2022 19:10:21 +0200 Subject: [PATCH 1/9] Refactor large editFileLocally FolderMan method into smaller, clearer separate methods Signed-off-by: Claudio Cambra --- src/gui/accountmanager.cpp | 15 ++ src/gui/accountmanager.h | 6 + src/gui/folderman.cpp | 277 +++++++++++++++++++++++-------------- src/gui/folderman.h | 28 +++- 4 files changed, 218 insertions(+), 108 deletions(-) diff --git a/src/gui/accountmanager.cpp b/src/gui/accountmanager.cpp index b2a90a39cb49d..d47efd4f8cc39 100644 --- a/src/gui/accountmanager.cpp +++ b/src/gui/accountmanager.cpp @@ -348,6 +348,21 @@ AccountStatePtr AccountManager::account(const QString &name) return it != _accounts.cend() ? *it : AccountStatePtr(); } +AccountStatePtr AccountManager::accountFromUserId(const QString &id) const +{ + for (const auto &account : accounts()) { + const auto isUserIdWithPort = id.split(QLatin1Char(':')).size() > 1; + const auto port = isUserIdWithPort ? account->account()->url().port() : -1; + const auto portString = (port > 0 && port != 80 && port != 443) ? QStringLiteral(":%1").arg(port) : QStringLiteral(""); + const QString davUserId = QStringLiteral("%1@%2").arg(account->account()->davUser(), account->account()->url().host()) + portString; + + if (davUserId == id) { + return account; + } + } + return {}; +} + AccountState *AccountManager::addAccount(const AccountPtr &newAccount) { auto id = newAccount->id(); diff --git a/src/gui/accountmanager.h b/src/gui/accountmanager.h index 0cec19053dc7a..ef257a4ce0156 100644 --- a/src/gui/accountmanager.h +++ b/src/gui/accountmanager.h @@ -65,6 +65,12 @@ class AccountManager : public QObject */ AccountStatePtr account(const QString &name); + /** + * Return the account state pointer for an account from its id + */ + + [[nodiscard]] AccountStatePtr accountFromUserId(const QString &id) const; + /** * Delete the AccountState */ diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index fbbaed489c63d..8c02f38b4d86c 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -1424,55 +1424,79 @@ void FolderMan::setDirtyNetworkLimits() void FolderMan::editFileLocally(const QString &userId, const QString &relPath, const QString &token) { - const auto showError = [this](const OCC::AccountStatePtr accountState, const QString &errorMessage, const QString &subject) { - if (accountState && accountState->account()) { - const auto foundFolder = std::find_if(std::cbegin(map()), std::cend(map()), [accountState](const auto &folder) { - return accountState->account()->davUrl() == folder->remoteUrl(); - }); - - if (foundFolder != std::cend(map())) { - (*foundFolder)->syncEngine().addErrorToGui(SyncFileItem::SoftError, errorMessage, subject); - } - } + const auto accountFound = AccountManager::instance()->accountFromUserId(userId); - // to make sure the error is not missed, show a message box in addition - const auto messageBox = new QMessageBox; - messageBox->setAttribute(Qt::WA_DeleteOnClose); - messageBox->setText(errorMessage); - messageBox->setInformativeText(subject); - messageBox->setIcon(QMessageBox::Warning); - messageBox->addButton(QMessageBox::StandardButton::Ok); - messageBox->show(); - messageBox->activateWindow(); - messageBox->raise(); - }; + if (!accountFound) { + qCWarning(lcFolderMan) << "Could not find an account " << userId << " to edit file " << relPath << " locally."; + showEditLocallyError(accountFound, tr("Could not find an account for local editing"), userId); + return; + } - if (token.isEmpty()) { - qCWarning(lcFolderMan) << "Edit locally request is missing a valid token. Impossible to open the file."; - showError({}, tr("Edit locally request is not valid. Opening the file is forbidden."), userId); + if (!isEditLocallyTokenValid(token)) { + qCWarning(lcFolderMan) << "Edit locally request is missing a valid token, will not open file. Token received was:" << token; + showEditLocallyError(accountFound, tr("Invalid token received."), tr("Please try again.")); return; } - const auto accountFound = [&userId]() { - for (const auto &account : AccountManager::instance()->accounts()) { - const auto isUserIdWithPort = userId.split(QLatin1Char(':')).size() > 1; - const auto port = isUserIdWithPort ? account->account()->url().port() : -1; - const auto portString = (port > 0 && port != 80 && port != 443) ? QStringLiteral(":%1").arg(port) : QStringLiteral(""); - const QString davUserId = QStringLiteral("%1@%2").arg(account->account()->davUser(), account->account()->url().host()) + portString; + if (!isEditLocallyRelPathValid(relPath)) { + qCWarning(lcFolderMan) << "Provided relPath was:" << relPath << "which is not canonical."; + showEditLocallyError(accountFound, tr("Invalid file path was provided."), tr("Please try again.")); + return; + } - if (davUserId == userId) { - return account; - } + const auto foundFiles = findFileInLocalFolders(relPath, accountFound->account()); + + if (foundFiles.isEmpty()) { + if (isEditLocallyRelPathExcluded(relPath)) { + showEditLocallyError(accountFound, tr("Could not find a file for local editing. Make sure it is not excluded via selective sync."), relPath); + } else { + showEditLocallyError(accountFound, tr("Could not find a file for local editing. Make sure its path is valid and it is synced locally."), relPath); } - return AccountStatePtr{}; - }(); + return; + } - if (!accountFound) { - qCWarning(lcFolderMan) << "Could not find an account " << userId << " to edit file " << relPath << " locally."; - showError(accountFound, tr("Could not find an account for local editing"), userId); + const auto localFilePath = foundFiles.first(); + const auto folderForFile = folderForPath(localFilePath); + + if (!folderForFile) { + showEditLocallyError(accountFound, tr("Could not find a folder to sync."), relPath); return; } + const auto relPathSplit = relPath.split(QLatin1Char('/')); + if (relPathSplit.size() == 0) { + showEditLocallyError(accountFound, tr("Could not find a file for local editing. Make sure its path is valid and it is synced locally."), relPath); + return; + } + + startEditLocally(accountFound, relPath, token, relPathSplit.last(), localFilePath, folderForFile); +} + +bool FolderMan::isEditLocallyTokenValid(const QString &token) const +{ + if (token.isEmpty()) { + return false; + } + + // Token is an alphanumeric string 128 chars long. + // Ensure that is what we received and what we are sending to the server. + const QRegularExpression tokenRegex("^[a-zA-Z0-9]{128}$"); + const auto regexMatch = tokenRegex.match(token); + + // Means invalid token type received, be cautious with bad token + if(!regexMatch.hasMatch()) { + return false; + } + + return true; +} + +bool FolderMan::isEditLocallyRelPathValid(const QString &relPath) const +{ + if (relPath.isEmpty()) { + return false; + } + // We want to check that the path is canonical and not relative // (i.e. that it doesn't contain ../../) but we always receive // a relative path, so let's make it absolute by prepending a @@ -1488,93 +1512,132 @@ void FolderMan::editFileLocally(const QString &userId, const QString &relPath, c const auto cleanedPath = QDir::cleanPath(slashPrefixedPath); if (cleanedPath != slashPrefixedPath) { - qCWarning(lcFolderMan) << "Provided relPath was:" << relPath - << "which is not canonical (cleaned path was:" << cleanedPath << ")"; - showError(accountFound, tr("Invalid file path was provided."), tr("Please try again.")); - return; + return false; } - const auto foundFiles = findFileInLocalFolders(relPath, accountFound->account()); + return true; +} - if (foundFiles.isEmpty()) { - for (const auto &folder : map()) { - bool result = false; - const auto excludedThroughSelectiveSync = folder->journalDb()->getSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, &result); - for (const auto &excludedPath : excludedThroughSelectiveSync) { - if (relPath.startsWith(excludedPath)) { - showError(accountFound, tr("Could not find a file for local editing. Make sure it is not excluded via selective sync."), relPath); - return; - } +bool FolderMan::isEditLocallyRelPathExcluded(const QString &relPath) const +{ + if (relPath.isEmpty()) { + return true; + } + + for (const auto &folder : map()) { + bool result = false; + const auto excludedThroughSelectiveSync = folder->journalDb()->getSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, &result); + for (const auto &excludedPath : excludedThroughSelectiveSync) { + if (relPath.startsWith(excludedPath)) { + return false; } } - - showError(accountFound, tr("Could not find a file for local editing. Make sure its path is valid and it is synced locally."), relPath); - return; } - const auto localFilePath = foundFiles.first(); - const auto folderForFile = folderForPath(localFilePath); + return true; +} - if (!folderForFile) { - showError(accountFound, tr("Could not find a folder to sync."), relPath); - return; - } +void FolderMan::showEditLocallyError(const AccountStatePtr &accountState, const QString &message, const QString &informativeText) const +{ + showEditLocallyErrorNotification(accountState, message, informativeText); + // to make sure the error is not missed, show a message box in addition + showEditLocallyErrorMessageBox(message, informativeText); +} - // Token is an alphanumeric string 128 chars long. - // Ensure that is what we received and what we are sending to the server. - const QRegularExpression tokenRegex("^[a-zA-Z0-9]{128}$"); - const auto regexMatch = tokenRegex.match(token); +void FolderMan::showEditLocallyErrorNotification(const AccountStatePtr &accountState, const QString &message, const QString &informativeText) const +{ + if (accountState && accountState->account()) { + const auto foundFolder = std::find_if(std::cbegin(map()), std::cend(map()), [accountState](const auto &folder) { + return accountState->account()->davUrl() == folder->remoteUrl(); + }); - // Means invalid token type received, be cautious with bad token - if(!regexMatch.hasMatch()) { - showError(accountFound, tr("Invalid token received."), tr("Please try again.")); - return; + if (foundFolder != std::cend(map())) { + (*foundFolder)->syncEngine().addErrorToGui(SyncFileItem::SoftError, message, informativeText); + } } +} - const auto relPathSplit = relPath.split(QLatin1Char('/')); - if (relPathSplit.size() > 0) { - Systray::instance()->createEditFileLocallyLoadingDialog(relPathSplit.last()); - } else { - showError(accountFound, tr("Could not find a file for local editing. Make sure its path is valid and it is synced locally."), relPath); - return; - } +void FolderMan::showEditLocallyErrorMessageBox(const QString &message, const QString &informativeText) const +{ + const auto messageBox = new QMessageBox; + messageBox->setAttribute(Qt::WA_DeleteOnClose); + messageBox->setText(message); + messageBox->setInformativeText(informativeText); + messageBox->setIcon(QMessageBox::Warning); + messageBox->addButton(QMessageBox::StandardButton::Ok); + messageBox->show(); + messageBox->activateWindow(); + messageBox->raise(); +} + +void FolderMan::startEditLocally(const AccountStatePtr &accountState, + const QString &relPath, + const QString &token, + const QString &fileName, + const QString &localFilePath, + Folder *folderForFile) +{ + Systray::instance()->createEditFileLocallyLoadingDialog(fileName); const auto encodedToken = QString::fromUtf8(QUrl::toPercentEncoding(token)); // Sanitise the token - const auto encodedRelPath = QUrl::toPercentEncoding(slashPrefixedPath); // Sanitise the relPath - const auto checkTokenForEditLocally = new SimpleApiJob(accountFound->account(), QStringLiteral("/ocs/v2.php/apps/files/api/v1/openlocaleditor/%1").arg(encodedToken)); + const auto encodedRelPath = QUrl::toPercentEncoding(relPath); // Sanitise the relPath + const auto checkEditLocallyToken = new SimpleApiJob(accountState->account(), QStringLiteral("/ocs/v2.php/apps/files/api/v1/openlocaleditor/%1").arg(encodedToken)); QUrlQuery params; - params.addQueryItem(QStringLiteral("path"), slashPrefixedPath); - checkTokenForEditLocally->addQueryParams(params); - checkTokenForEditLocally->setVerb(SimpleApiJob::Verb::Post); - connect(checkTokenForEditLocally, &SimpleApiJob::resultReceived, checkTokenForEditLocally, [this, folderForFile, localFilePath, showError, accountFound, relPath] (int statusCode) { - constexpr auto HTTP_OK_CODE = 200; - if (statusCode != HTTP_OK_CODE) { - Systray::instance()->destroyEditFileLocallyLoadingDialog(); - showError(accountFound, tr("Could not validate the request to open a file from server."), relPath); - qCInfo(lcFolderMan()) << "token check result" << statusCode; - return; - } + params.addQueryItem(QStringLiteral("path"), QString{"/" + relPath}); + checkEditLocallyToken->addQueryParams(params); - folderForFile->startSync(); - _localFileEditingSyncFinishedConnections.insert(localFilePath, QObject::connect(folderForFile, &Folder::syncFinished, this, - [this, localFilePath](const OCC::SyncResult &result) { - Q_UNUSED(result); - const auto foundConnectionIt = _localFileEditingSyncFinishedConnections.find(localFilePath); - if (foundConnectionIt != std::end(_localFileEditingSyncFinishedConnections) && foundConnectionIt.value()) { - QObject::disconnect(foundConnectionIt.value()); - _localFileEditingSyncFinishedConnections.erase(foundConnectionIt); - } - // In case the VFS mode is enabled and a file is not yet hydrated, we must call QDesktopServices::openUrl - // from a separate thread, or, there will be a freeze. To avoid searching for a specific folder and checking - // if the VFS is enabled - we just always call it from a separate thread. - QtConcurrent::run([localFilePath]() { - QDesktopServices::openUrl(QUrl::fromLocalFile(localFilePath)); - Systray::instance()->destroyEditFileLocallyLoadingDialog(); - }); - })); + connect(checkEditLocallyToken, &SimpleApiJob::resultReceived, checkEditLocallyToken, [this, folderForFile, localFilePath, accountState, relPath] (int statusCode) { + editLocallyTokenCheckFinished(accountState, relPath, localFilePath, folderForFile, statusCode); + }); + checkEditLocallyToken->start(); +} + +void FolderMan::editLocallyTokenCheckFinished(const AccountStatePtr &accountState, + const QString &relPath, + const QString &localFilePath, + Folder *folderForFile, + const int statusCode) +{ + constexpr auto HTTP_OK_CODE = 200; + if (statusCode != HTTP_OK_CODE) { + Systray::instance()->destroyEditFileLocallyLoadingDialog(); + showEditLocallyError(accountState, tr("Could not validate the request to open a file from server."), relPath); + qCInfo(lcFolderMan()) << "token check result" << statusCode; + return; + } + + folderForFile->startSync(); + const auto syncFinishedConnection = connect(folderForFile, &Folder::syncFinished, this, [this, localFilePath](const OCC::SyncResult &result) { + Q_UNUSED(result); + editLocallyFolderSyncFinished(localFilePath); + }); + _editLocallySyncFinishedConnections.insert(localFilePath, syncFinishedConnection); +} + +void FolderMan::editLocallyFolderSyncFinished(const QString &localFilePath) +{ + disconnectEditLocallySyncFinishedConnections(localFilePath); + openEditLocallyFile(localFilePath); +} + +void FolderMan::disconnectEditLocallySyncFinishedConnections(const QString &localFilePath) +{ + if (const auto existingConnection = _editLocallySyncFinishedConnections.value(localFilePath)) { + disconnect(existingConnection); + _editLocallySyncFinishedConnections.remove(localFilePath); + } +} + +void FolderMan::openEditLocallyFile(const QString &localFilePath) const +{ + // In case the VFS mode is enabled and a file is not yet hydrated, we must call QDesktopServices::openUrl + // from a separate thread, or, there will be a freeze. To avoid searching for a specific folder and checking + // if the VFS is enabled - we just always call it from a separate thread. + QtConcurrent::run([localFilePath]() { + QDesktopServices::openUrl(QUrl::fromLocalFile(localFilePath)); + Systray::instance()->destroyEditFileLocallyLoadingDialog(); }); - checkTokenForEditLocally->start(); } void FolderMan::trayOverallStatus(const QList &folders, diff --git a/src/gui/folderman.h b/src/gui/folderman.h index 2bbc2c0f17100..682d475ba06e3 100644 --- a/src/gui/folderman.h +++ b/src/gui/folderman.h @@ -309,6 +309,27 @@ private slots: void slotProcessFilesPushNotification(Account *account); void slotConnectToPushNotifications(Account *account); + void showEditLocallyError(const AccountStatePtr &accountState, const QString &message, const QString &informativeText) const; + void showEditLocallyErrorNotification(const AccountStatePtr &accountState, const QString &message, const QString &informativeText) const; + void showEditLocallyErrorMessageBox(const QString &message, const QString &informativeText) const; + + void startEditLocally(const AccountStatePtr &accountState, + const QString &relPath, + const QString &token, + const QString &fileName, + const QString &localFilePath, + Folder *folderForFile); + + void editLocallyTokenCheckFinished(const AccountStatePtr &accountState, + const QString &relPath, + const QString &localFilePath, + Folder *folderForFile, + const int statusCode); + + void disconnectEditLocallySyncFinishedConnections(const QString &localFilePath); + void editLocallyFolderSyncFinished(const QString &localFilePath); + void openEditLocallyFile(const QString &localFilePath) const; + private: /** Adds a new folder, does not add it to the account settings and * does not set an account on the new folder. @@ -341,6 +362,11 @@ private slots: bool isSwitchToVfsNeeded(const FolderDefinition &folderDefinition) const; + [[nodiscard]] bool isEditLocallyTokenValid(const QString &token) const; + [[nodiscard]] bool isEditLocallyRelPathValid(const QString &relPath) const; + [[nodiscard]] bool isEditLocallyRelPathExcluded(const QString &relPath) const; + [[nodiscard]] bool editLocallyAccount(const AccountStatePtr &acountState) const; + QSet _disabledFolders; Folder::Map _folderMap; QString _folderConfigPath; @@ -375,7 +401,7 @@ private slots: bool _appRestartRequired = false; - QMap _localFileEditingSyncFinishedConnections; + QHash _editLocallySyncFinishedConnections; static FolderMan *_instance; explicit FolderMan(QObject *parent = nullptr); From 52ee2607002f7ef39405577b44d3070a59aec755 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Wed, 26 Oct 2022 20:13:38 +0200 Subject: [PATCH 2/9] Refactor edit locally into its own class, rather than bolting onto FolderMan Signed-off-by: Claudio Cambra --- src/gui/CMakeLists.txt | 2 + src/gui/application.cpp | 10 +- src/gui/editlocallyhandler.cpp | 271 +++++++++++++++++++++++++++++++++ src/gui/editlocallyhandler.h | 71 +++++++++ src/gui/folderman.cpp | 220 -------------------------- src/gui/folderman.h | 31 ---- 6 files changed, 353 insertions(+), 252 deletions(-) create mode 100644 src/gui/editlocallyhandler.cpp create mode 100644 src/gui/editlocallyhandler.h diff --git a/src/gui/CMakeLists.txt b/src/gui/CMakeLists.txt index 5a11578130490..4b1b60a6a9e72 100644 --- a/src/gui/CMakeLists.txt +++ b/src/gui/CMakeLists.txt @@ -80,6 +80,8 @@ set(client_SRCS conflictsolver.cpp connectionvalidator.h connectionvalidator.cpp + editlocallyhandler.h + editlocallyhandler.cpp folder.h folder.cpp foldercreationdialog.h diff --git a/src/gui/application.cpp b/src/gui/application.cpp index e1a1ccbc917c2..22e7588406799 100644 --- a/src/gui/application.cpp +++ b/src/gui/application.cpp @@ -22,6 +22,7 @@ #include "config.h" #include "account.h" #include "accountstate.h" +#include "editlocallyhandler.h" #include "connectionvalidator.h" #include "folder.h" #include "folderman.h" @@ -774,7 +775,14 @@ void Application::handleEditLocally(const QUrl &url) const qCWarning(lcApplication) << "Invalid URL for file local editing: missing token"; } - FolderMan::instance()->editFileLocally(userId, fileRemotePath, token); + // We need to make sure the handler sticks around until it is finished + const auto editLocallyHandler = new EditLocallyHandler(userId, fileRemotePath, token); + if (editLocallyHandler->ready()) { + connect(editLocallyHandler, &EditLocallyHandler::finished, this, [&editLocallyHandler] { delete editLocallyHandler; }); + editLocallyHandler->startEditLocally(); + } else { + delete editLocallyHandler; + } } QString substLang(const QString &lang) diff --git a/src/gui/editlocallyhandler.cpp b/src/gui/editlocallyhandler.cpp new file mode 100644 index 0000000000000..3bacf816cf236 --- /dev/null +++ b/src/gui/editlocallyhandler.cpp @@ -0,0 +1,271 @@ +/* + * Copyright (C) by Claudio Cambra + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + */ + +#include "editlocallyhandler.h" + +#include +#include +#include + +#include "accountmanager.h" +#include "folder.h" +#include "folderman.h" +#include "syncengine.h" +#include "systray.h" + +namespace OCC { + +Q_LOGGING_CATEGORY(lcEditLocallyHandler, "nextcloud.gui.editlocallyhandler", QtInfoMsg) + +static QHash editLocallySyncFinishedConnections; + +EditLocallyHandler::EditLocallyHandler(const QString &userId, + const QString &relPath, + const QString &token, + QObject *parent) + : QObject{parent} + , _relPath(relPath) + , _token(token) +{ + _accountState = AccountManager::instance()->accountFromUserId(userId); + + if (!_accountState) { + qCWarning(lcEditLocallyHandler) << "Could not find an account " << userId << " to edit file " << relPath << " locally."; + showError(tr("Could not find an account for local editing"), userId); + return; + } + + if (!isTokenValid(token)) { + qCWarning(lcEditLocallyHandler) << "Edit locally request is missing a valid token, will not open file. Token received was:" << token; + showError(tr("Invalid token received."), tr("Please try again.")); + return; + } + + if (!isRelPathValid(relPath)) { + qCWarning(lcEditLocallyHandler) << "Provided relPath was:" << relPath << "which is not canonical."; + showError(tr("Invalid file path was provided."), tr("Please try again.")); + return; + } + + const auto foundFiles = FolderMan::instance()->findFileInLocalFolders(relPath, _accountState->account()); + + if (foundFiles.isEmpty()) { + if (isRelPathExcluded(relPath)) { + showError(tr("Could not find a file for local editing. Make sure it is not excluded via selective sync."), relPath); + } else { + showError(tr("Could not find a file for local editing. Make sure its path is valid and it is synced locally."), relPath); + } + return; + } + + _localFilePath = foundFiles.first(); + _folderForFile = FolderMan::instance()->folderForPath(_localFilePath); + + if (!_folderForFile) { + showError(tr("Could not find a folder to sync."), relPath); + return; + } + + const auto relPathSplit = relPath.split(QLatin1Char('/')); + if (relPathSplit.size() == 0) { + showError(tr("Could not find a file for local editing. Make sure its path is valid and it is synced locally."), relPath); + return; + } + + _fileName = relPathSplit.last(); + + _ready = true; +} + +bool EditLocallyHandler::ready() const +{ + return _ready; +} + +QString EditLocallyHandler::prefixSlashToPath(const QString &path) +{ + auto slashPrefixedPath = path; + if (!slashPrefixedPath.startsWith('/')) { + slashPrefixedPath.prepend('/'); + } + + return slashPrefixedPath; +} + +bool EditLocallyHandler::isTokenValid(const QString &token) +{ + if (token.isEmpty()) { + return false; + } + + // Token is an alphanumeric string 128 chars long. + // Ensure that is what we received and what we are sending to the server. + const QRegularExpression tokenRegex("^[a-zA-Z0-9]{128}$"); + const auto regexMatch = tokenRegex.match(token); + + // Means invalid token type received, be cautious with bad token + if(!regexMatch.hasMatch()) { + return false; + } + + return true; +} + +bool EditLocallyHandler::isRelPathValid(const QString &relPath) +{ + if (relPath.isEmpty()) { + return false; + } + + // We want to check that the path is canonical and not relative + // (i.e. that it doesn't contain ../../) but we always receive + // a relative path, so let's make it absolute by prepending a + // slash + const auto slashPrefixedPath = prefixSlashToPath(relPath); + + // Let's check that the filepath is canonical, and that the request + // contains no funny behaviour regarding paths + const auto cleanedPath = QDir::cleanPath(slashPrefixedPath); + + if (cleanedPath != slashPrefixedPath) { + return false; + } + + return true; +} + +bool EditLocallyHandler::isRelPathExcluded(const QString &relPath) +{ + if (relPath.isEmpty()) { + return true; + } + + const auto folderMap = FolderMan::instance()->map(); + for (const auto &folder : folderMap) { + bool result = false; + const auto excludedThroughSelectiveSync = folder->journalDb()->getSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, &result); + for (const auto &excludedPath : excludedThroughSelectiveSync) { + if (relPath.startsWith(excludedPath)) { + return false; + } + } + } + + return true; +} + +void EditLocallyHandler::showError(const QString &message, const QString &informativeText) const +{ + showErrorNotification(message, informativeText); + // to make sure the error is not missed, show a message box in addition + showErrorMessageBox(message, informativeText); +} + +void EditLocallyHandler::showErrorNotification(const QString &message, const QString &informativeText) const +{ + if (!_accountState || !_accountState->account()) { + return; + } + + const auto folderMap = FolderMan::instance()->map(); + const auto foundFolder = std::find_if(folderMap.cbegin(), folderMap.cend(), [this](const auto &folder) { + return _accountState->account()->davUrl() == folder->remoteUrl(); + }); + + if (foundFolder != folderMap.cend()) { + (*foundFolder)->syncEngine().addErrorToGui(SyncFileItem::SoftError, message, informativeText); + } +} + +void EditLocallyHandler::showErrorMessageBox(const QString &message, const QString &informativeText) const +{ + const auto messageBox = new QMessageBox; + messageBox->setAttribute(Qt::WA_DeleteOnClose); + messageBox->setText(message); + messageBox->setInformativeText(informativeText); + messageBox->setIcon(QMessageBox::Warning); + messageBox->addButton(QMessageBox::StandardButton::Ok); + messageBox->show(); + messageBox->activateWindow(); + messageBox->raise(); +} + +void EditLocallyHandler::startEditLocally() +{ + if (!_ready) { + return; + } + + Systray::instance()->createEditFileLocallyLoadingDialog(_fileName); + + const auto encodedToken = QString::fromUtf8(QUrl::toPercentEncoding(_token)); // Sanitise the token + const auto encodedRelPath = QUrl::toPercentEncoding(_relPath); // Sanitise the relPath + const auto checkEditLocallyToken = new SimpleApiJob(_accountState->account(), QStringLiteral("/ocs/v2.php/apps/files/api/v1/openlocaleditor/%1").arg(encodedToken)); + + QUrlQuery params; + params.addQueryItem(QStringLiteral("path"), prefixSlashToPath(_relPath)); + checkEditLocallyToken->addQueryParams(params); + checkEditLocallyToken->setVerb(SimpleApiJob::Verb::Post); + connect(checkEditLocallyToken, &SimpleApiJob::resultReceived, this, &EditLocallyHandler::remoteTokenCheckFinished); + + checkEditLocallyToken->start(); +} + +void EditLocallyHandler::remoteTokenCheckFinished(const int statusCode) +{ + constexpr auto HTTP_OK_CODE = 200; + if (statusCode != HTTP_OK_CODE) { + Systray::instance()->destroyEditFileLocallyLoadingDialog(); + + showError(tr("Could not validate the request to open a file from server."), _relPath); + qCInfo(lcEditLocallyHandler) << "token check result" << statusCode; + return; + } + + _folderForFile->startSync(); + const auto syncFinishedConnection = connect(_folderForFile, &Folder::syncFinished, + this, &EditLocallyHandler::folderSyncFinished); + editLocallySyncFinishedConnections.insert(_localFilePath, syncFinishedConnection); +} + +void EditLocallyHandler::folderSyncFinished(const OCC::SyncResult &result) +{ + Q_UNUSED(result) + disconnectSyncFinished(); + openFile(); +} + +void EditLocallyHandler::disconnectSyncFinished() const +{ + if (const auto existingConnection = editLocallySyncFinishedConnections.value(_localFilePath)) { + disconnect(existingConnection); + editLocallySyncFinishedConnections.remove(_localFilePath); + } +} + +void EditLocallyHandler::openFile() +{ + const auto localFilePath = _localFilePath; + // In case the VFS mode is enabled and a file is not yet hydrated, we must call QDesktopServices::openUrl + // from a separate thread, or, there will be a freeze. To avoid searching for a specific folder and checking + // if the VFS is enabled - we just always call it from a separate thread. + QtConcurrent::run([localFilePath]() { + QDesktopServices::openUrl(QUrl::fromLocalFile(localFilePath)); + Systray::instance()->destroyEditFileLocallyLoadingDialog(); + }); + + Q_EMIT finished(); +} + +} diff --git a/src/gui/editlocallyhandler.h b/src/gui/editlocallyhandler.h new file mode 100644 index 0000000000000..9d6f9677c235e --- /dev/null +++ b/src/gui/editlocallyhandler.h @@ -0,0 +1,71 @@ +/* + * Copyright (C) by Claudio Cambra + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + */ + +#pragma once + +#include + +#include "accountmanager.h" +#include "folder.h" + +namespace OCC { + +class EditLocallyHandler : public QObject +{ + Q_OBJECT + +public: + explicit EditLocallyHandler(const QString &userId, + const QString &relPath, + const QString &token, + QObject *parent = nullptr); + + [[nodiscard]] static bool isTokenValid(const QString &token); + [[nodiscard]] static bool isRelPathValid(const QString &relPath); + [[nodiscard]] static bool isRelPathExcluded(const QString &relPath); + [[nodiscard]] static QString prefixSlashToPath(const QString &path); + + [[nodiscard]] bool ready() const; + +signals: + void finished(); + +public slots: + void startEditLocally(); + void startTokenRemoteCheck(); + +private slots: + void showError(const QString &message, const QString &informativeText) const; + void showErrorNotification(const QString &message, const QString &informativeText) const; + void showErrorMessageBox(const QString &message, const QString &informativeText) const; + + void remoteTokenCheckFinished(const int statusCode); + void folderSyncFinished(const OCC::SyncResult &result); + + void disconnectSyncFinished() const; + void openFile(); + +private: + bool _ready = false; + + AccountStatePtr _accountState; + QString _relPath; + QString _token; + + QString _fileName; + QString _localFilePath; + Folder *_folderForFile = nullptr; +}; + +} diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index 8c02f38b4d86c..66f1486604746 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -36,8 +36,6 @@ #include #include #include -#include -#include static const char versionC[] = "version"; static const int maxFoldersVersion = 1; @@ -1422,224 +1420,6 @@ void FolderMan::setDirtyNetworkLimits() } } -void FolderMan::editFileLocally(const QString &userId, const QString &relPath, const QString &token) -{ - const auto accountFound = AccountManager::instance()->accountFromUserId(userId); - - if (!accountFound) { - qCWarning(lcFolderMan) << "Could not find an account " << userId << " to edit file " << relPath << " locally."; - showEditLocallyError(accountFound, tr("Could not find an account for local editing"), userId); - return; - } - - if (!isEditLocallyTokenValid(token)) { - qCWarning(lcFolderMan) << "Edit locally request is missing a valid token, will not open file. Token received was:" << token; - showEditLocallyError(accountFound, tr("Invalid token received."), tr("Please try again.")); - return; - } - - if (!isEditLocallyRelPathValid(relPath)) { - qCWarning(lcFolderMan) << "Provided relPath was:" << relPath << "which is not canonical."; - showEditLocallyError(accountFound, tr("Invalid file path was provided."), tr("Please try again.")); - return; - } - - const auto foundFiles = findFileInLocalFolders(relPath, accountFound->account()); - - if (foundFiles.isEmpty()) { - if (isEditLocallyRelPathExcluded(relPath)) { - showEditLocallyError(accountFound, tr("Could not find a file for local editing. Make sure it is not excluded via selective sync."), relPath); - } else { - showEditLocallyError(accountFound, tr("Could not find a file for local editing. Make sure its path is valid and it is synced locally."), relPath); - } - return; - } - - const auto localFilePath = foundFiles.first(); - const auto folderForFile = folderForPath(localFilePath); - - if (!folderForFile) { - showEditLocallyError(accountFound, tr("Could not find a folder to sync."), relPath); - return; - } - - const auto relPathSplit = relPath.split(QLatin1Char('/')); - if (relPathSplit.size() == 0) { - showEditLocallyError(accountFound, tr("Could not find a file for local editing. Make sure its path is valid and it is synced locally."), relPath); - return; - } - - startEditLocally(accountFound, relPath, token, relPathSplit.last(), localFilePath, folderForFile); -} - -bool FolderMan::isEditLocallyTokenValid(const QString &token) const -{ - if (token.isEmpty()) { - return false; - } - - // Token is an alphanumeric string 128 chars long. - // Ensure that is what we received and what we are sending to the server. - const QRegularExpression tokenRegex("^[a-zA-Z0-9]{128}$"); - const auto regexMatch = tokenRegex.match(token); - - // Means invalid token type received, be cautious with bad token - if(!regexMatch.hasMatch()) { - return false; - } - - return true; -} - -bool FolderMan::isEditLocallyRelPathValid(const QString &relPath) const -{ - if (relPath.isEmpty()) { - return false; - } - - // We want to check that the path is canonical and not relative - // (i.e. that it doesn't contain ../../) but we always receive - // a relative path, so let's make it absolute by prepending a - // slash - - auto slashPrefixedPath = relPath; - if (!slashPrefixedPath.startsWith('/')) { - slashPrefixedPath.prepend('/'); - } - - // Let's check that the filepath is canonical, and that the request - // contains no funny behaviour regarding paths - const auto cleanedPath = QDir::cleanPath(slashPrefixedPath); - - if (cleanedPath != slashPrefixedPath) { - return false; - } - - return true; -} - -bool FolderMan::isEditLocallyRelPathExcluded(const QString &relPath) const -{ - if (relPath.isEmpty()) { - return true; - } - - for (const auto &folder : map()) { - bool result = false; - const auto excludedThroughSelectiveSync = folder->journalDb()->getSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, &result); - for (const auto &excludedPath : excludedThroughSelectiveSync) { - if (relPath.startsWith(excludedPath)) { - return false; - } - } - } - - return true; -} - -void FolderMan::showEditLocallyError(const AccountStatePtr &accountState, const QString &message, const QString &informativeText) const -{ - showEditLocallyErrorNotification(accountState, message, informativeText); - // to make sure the error is not missed, show a message box in addition - showEditLocallyErrorMessageBox(message, informativeText); -} - -void FolderMan::showEditLocallyErrorNotification(const AccountStatePtr &accountState, const QString &message, const QString &informativeText) const -{ - if (accountState && accountState->account()) { - const auto foundFolder = std::find_if(std::cbegin(map()), std::cend(map()), [accountState](const auto &folder) { - return accountState->account()->davUrl() == folder->remoteUrl(); - }); - - if (foundFolder != std::cend(map())) { - (*foundFolder)->syncEngine().addErrorToGui(SyncFileItem::SoftError, message, informativeText); - } - } -} - -void FolderMan::showEditLocallyErrorMessageBox(const QString &message, const QString &informativeText) const -{ - const auto messageBox = new QMessageBox; - messageBox->setAttribute(Qt::WA_DeleteOnClose); - messageBox->setText(message); - messageBox->setInformativeText(informativeText); - messageBox->setIcon(QMessageBox::Warning); - messageBox->addButton(QMessageBox::StandardButton::Ok); - messageBox->show(); - messageBox->activateWindow(); - messageBox->raise(); -} - -void FolderMan::startEditLocally(const AccountStatePtr &accountState, - const QString &relPath, - const QString &token, - const QString &fileName, - const QString &localFilePath, - Folder *folderForFile) -{ - Systray::instance()->createEditFileLocallyLoadingDialog(fileName); - - const auto encodedToken = QString::fromUtf8(QUrl::toPercentEncoding(token)); // Sanitise the token - const auto encodedRelPath = QUrl::toPercentEncoding(relPath); // Sanitise the relPath - const auto checkEditLocallyToken = new SimpleApiJob(accountState->account(), QStringLiteral("/ocs/v2.php/apps/files/api/v1/openlocaleditor/%1").arg(encodedToken)); - - QUrlQuery params; - params.addQueryItem(QStringLiteral("path"), QString{"/" + relPath}); - checkEditLocallyToken->addQueryParams(params); - - connect(checkEditLocallyToken, &SimpleApiJob::resultReceived, checkEditLocallyToken, [this, folderForFile, localFilePath, accountState, relPath] (int statusCode) { - editLocallyTokenCheckFinished(accountState, relPath, localFilePath, folderForFile, statusCode); - }); - checkEditLocallyToken->start(); -} - -void FolderMan::editLocallyTokenCheckFinished(const AccountStatePtr &accountState, - const QString &relPath, - const QString &localFilePath, - Folder *folderForFile, - const int statusCode) -{ - constexpr auto HTTP_OK_CODE = 200; - if (statusCode != HTTP_OK_CODE) { - Systray::instance()->destroyEditFileLocallyLoadingDialog(); - showEditLocallyError(accountState, tr("Could not validate the request to open a file from server."), relPath); - qCInfo(lcFolderMan()) << "token check result" << statusCode; - return; - } - - folderForFile->startSync(); - const auto syncFinishedConnection = connect(folderForFile, &Folder::syncFinished, this, [this, localFilePath](const OCC::SyncResult &result) { - Q_UNUSED(result); - editLocallyFolderSyncFinished(localFilePath); - }); - _editLocallySyncFinishedConnections.insert(localFilePath, syncFinishedConnection); -} - -void FolderMan::editLocallyFolderSyncFinished(const QString &localFilePath) -{ - disconnectEditLocallySyncFinishedConnections(localFilePath); - openEditLocallyFile(localFilePath); -} - -void FolderMan::disconnectEditLocallySyncFinishedConnections(const QString &localFilePath) -{ - if (const auto existingConnection = _editLocallySyncFinishedConnections.value(localFilePath)) { - disconnect(existingConnection); - _editLocallySyncFinishedConnections.remove(localFilePath); - } -} - -void FolderMan::openEditLocallyFile(const QString &localFilePath) const -{ - // In case the VFS mode is enabled and a file is not yet hydrated, we must call QDesktopServices::openUrl - // from a separate thread, or, there will be a freeze. To avoid searching for a specific folder and checking - // if the VFS is enabled - we just always call it from a separate thread. - QtConcurrent::run([localFilePath]() { - QDesktopServices::openUrl(QUrl::fromLocalFile(localFilePath)); - Systray::instance()->destroyEditFileLocallyLoadingDialog(); - }); -} - void FolderMan::trayOverallStatus(const QList &folders, SyncResult::Status *status, bool *unresolvedConflicts) { diff --git a/src/gui/folderman.h b/src/gui/folderman.h index 682d475ba06e3..c15be05295b64 100644 --- a/src/gui/folderman.h +++ b/src/gui/folderman.h @@ -213,9 +213,6 @@ class FolderMan : public QObject void setDirtyProxy(); void setDirtyNetworkLimits(); - /** opens a file with default app, if the file is present **/ - void editFileLocally(const QString &userId, const QString &relPath, const QString &token); - signals: /** * signal to indicate a folder has changed its sync state. @@ -309,27 +306,6 @@ private slots: void slotProcessFilesPushNotification(Account *account); void slotConnectToPushNotifications(Account *account); - void showEditLocallyError(const AccountStatePtr &accountState, const QString &message, const QString &informativeText) const; - void showEditLocallyErrorNotification(const AccountStatePtr &accountState, const QString &message, const QString &informativeText) const; - void showEditLocallyErrorMessageBox(const QString &message, const QString &informativeText) const; - - void startEditLocally(const AccountStatePtr &accountState, - const QString &relPath, - const QString &token, - const QString &fileName, - const QString &localFilePath, - Folder *folderForFile); - - void editLocallyTokenCheckFinished(const AccountStatePtr &accountState, - const QString &relPath, - const QString &localFilePath, - Folder *folderForFile, - const int statusCode); - - void disconnectEditLocallySyncFinishedConnections(const QString &localFilePath); - void editLocallyFolderSyncFinished(const QString &localFilePath); - void openEditLocallyFile(const QString &localFilePath) const; - private: /** Adds a new folder, does not add it to the account settings and * does not set an account on the new folder. @@ -362,11 +338,6 @@ private slots: bool isSwitchToVfsNeeded(const FolderDefinition &folderDefinition) const; - [[nodiscard]] bool isEditLocallyTokenValid(const QString &token) const; - [[nodiscard]] bool isEditLocallyRelPathValid(const QString &relPath) const; - [[nodiscard]] bool isEditLocallyRelPathExcluded(const QString &relPath) const; - [[nodiscard]] bool editLocallyAccount(const AccountStatePtr &acountState) const; - QSet _disabledFolders; Folder::Map _folderMap; QString _folderConfigPath; @@ -401,8 +372,6 @@ private slots: bool _appRestartRequired = false; - QHash _editLocallySyncFinishedConnections; - static FolderMan *_instance; explicit FolderMan(QObject *parent = nullptr); friend class OCC::Application; From 311dd8ee003d56cf788a7f22a1d0a174081009d1 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 28 Oct 2022 16:42:57 +0200 Subject: [PATCH 3/9] Verify token before checking disk or modifying state Signed-off-by: Claudio Cambra --- src/gui/application.cpp | 14 ++-- src/gui/editlocallyhandler.cpp | 148 ++++++++++++++++++++++----------- src/gui/editlocallyhandler.h | 16 ++-- 3 files changed, 117 insertions(+), 61 deletions(-) diff --git a/src/gui/application.cpp b/src/gui/application.cpp index 22e7588406799..93b33655bef79 100644 --- a/src/gui/application.cpp +++ b/src/gui/application.cpp @@ -777,12 +777,14 @@ void Application::handleEditLocally(const QUrl &url) const // We need to make sure the handler sticks around until it is finished const auto editLocallyHandler = new EditLocallyHandler(userId, fileRemotePath, token); - if (editLocallyHandler->ready()) { - connect(editLocallyHandler, &EditLocallyHandler::finished, this, [&editLocallyHandler] { delete editLocallyHandler; }); - editLocallyHandler->startEditLocally(); - } else { - delete editLocallyHandler; - } + const auto editLocallyHandlerDeleter = [editLocallyHandler]{ delete editLocallyHandler; }; + connect(editLocallyHandler, &EditLocallyHandler::error, this, editLocallyHandlerDeleter); + connect(editLocallyHandler, &EditLocallyHandler::fileOpened, this, editLocallyHandlerDeleter); + + connect(editLocallyHandler, &EditLocallyHandler::setupFinished, + editLocallyHandler, [editLocallyHandler]{ editLocallyHandler->startEditLocally(); }); + editLocallyHandler->startSetup(); + } QString substLang(const QString &lang) diff --git a/src/gui/editlocallyhandler.cpp b/src/gui/editlocallyhandler.cpp index 3bacf816cf236..65f12911c4f9c 100644 --- a/src/gui/editlocallyhandler.cpp +++ b/src/gui/editlocallyhandler.cpp @@ -35,36 +35,101 @@ EditLocallyHandler::EditLocallyHandler(const QString &userId, const QString &token, QObject *parent) : QObject{parent} + , _userId(userId) , _relPath(relPath) , _token(token) { - _accountState = AccountManager::instance()->accountFromUserId(userId); +} - if (!_accountState) { - qCWarning(lcEditLocallyHandler) << "Could not find an account " << userId << " to edit file " << relPath << " locally."; - showError(tr("Could not find an account for local editing"), userId); +void EditLocallyHandler::startSetup() +{ + if (_token.isEmpty() || _relPath.isEmpty() || _userId.isEmpty()) { + qCWarning(lcEditLocallyHandler) << "Could not start setup." + << "token:" << _token + << "relPath:" << _relPath + << "userId" << _userId; return; } - if (!isTokenValid(token)) { - qCWarning(lcEditLocallyHandler) << "Edit locally request is missing a valid token, will not open file. Token received was:" << token; + // We check the input data locally first, without modifying any state or + // showing any potentially misleading data to the user + if (!isTokenValid(_token)) { + qCWarning(lcEditLocallyHandler) << "Edit locally request is missing a valid token, will not open file. " + << "Token received was:" << _token; showError(tr("Invalid token received."), tr("Please try again.")); return; } - if (!isRelPathValid(relPath)) { - qCWarning(lcEditLocallyHandler) << "Provided relPath was:" << relPath << "which is not canonical."; + if (!isRelPathValid(_relPath)) { + qCWarning(lcEditLocallyHandler) << "Provided relPath was:" << _relPath << "which is not canonical."; showError(tr("Invalid file path was provided."), tr("Please try again.")); return; } - const auto foundFiles = FolderMan::instance()->findFileInLocalFolders(relPath, _accountState->account()); + _accountState = AccountManager::instance()->accountFromUserId(_userId); + + if (!_accountState) { + qCWarning(lcEditLocallyHandler) << "Could not find an account " << _userId << " to edit file " << _relPath << " locally."; + showError(tr("Could not find an account for local editing."), _userId); + return; + } + + // We now ask the server to verify the token, before we again modify any + // state or look at local files + startTokenRemoteCheck(); +} + +void EditLocallyHandler::startTokenRemoteCheck() +{ + if (!_accountState || _relPath.isEmpty() || _token.isEmpty()) { + qCWarning(lcEditLocallyHandler) << "Could not start token check." + << "accountState:" << _accountState + << "relPath:" << _relPath + << "token:" << _token; + return; + } + + const auto encodedToken = QString::fromUtf8(QUrl::toPercentEncoding(_token)); // Sanitise the token + const auto encodedRelPath = QUrl::toPercentEncoding(_relPath); // Sanitise the relPath + const auto checkEditLocallyToken = new SimpleApiJob(_accountState->account(), QStringLiteral("/ocs/v2.php/apps/files/api/v1/openlocaleditor/%1").arg(encodedToken)); + + QUrlQuery params; + params.addQueryItem(QStringLiteral("path"), prefixSlashToPath(encodedRelPath)); + checkEditLocallyToken->addQueryParams(params); + checkEditLocallyToken->setVerb(SimpleApiJob::Verb::Post); + connect(checkEditLocallyToken, &SimpleApiJob::resultReceived, this, &EditLocallyHandler::remoteTokenCheckResultReceived); + + checkEditLocallyToken->start(); +} + +void EditLocallyHandler::remoteTokenCheckResultReceived(const int statusCode) +{ + qCInfo(lcEditLocallyHandler) << "token check result" << statusCode; + + constexpr auto HTTP_OK_CODE = 200; + _tokenVerified = statusCode == HTTP_OK_CODE; + + if (!_tokenVerified) { + showError(tr("Could not validate the request to open a file from server."), _relPath); + } + + proceedWithSetup(); +} + +void EditLocallyHandler::proceedWithSetup() +{ + if (!_tokenVerified) { + qCWarning(lcEditLocallyHandler) << "Could not proceed with setup as token is not verified."; + return; + } + + const auto foundFiles = FolderMan::instance()->findFileInLocalFolders(_relPath, _accountState->account()); if (foundFiles.isEmpty()) { - if (isRelPathExcluded(relPath)) { - showError(tr("Could not find a file for local editing. Make sure it is not excluded via selective sync."), relPath); + if (isRelPathExcluded(_relPath)) { + showError(tr("Could not find a file for local editing. Make sure it is not excluded via selective sync."), _relPath); } else { - showError(tr("Could not find a file for local editing. Make sure its path is valid and it is synced locally."), relPath); + showError(tr("Could not find a file for local editing. Make sure its path is valid and it is synced locally."), _relPath); } return; } @@ -73,24 +138,19 @@ EditLocallyHandler::EditLocallyHandler(const QString &userId, _folderForFile = FolderMan::instance()->folderForPath(_localFilePath); if (!_folderForFile) { - showError(tr("Could not find a folder to sync."), relPath); + showError(tr("Could not find a folder to sync."), _relPath); return; } - const auto relPathSplit = relPath.split(QLatin1Char('/')); - if (relPathSplit.size() == 0) { - showError(tr("Could not find a file for local editing. Make sure its path is valid and it is synced locally."), relPath); + const auto relPathSplit = _relPath.split(QLatin1Char('/')); + if (relPathSplit.isEmpty()) { + showError(tr("Could not find a file for local editing. Make sure its path is valid and it is synced locally."), _relPath); return; } _fileName = relPathSplit.last(); - _ready = true; -} - -bool EditLocallyHandler::ready() const -{ - return _ready; + Q_EMIT setupFinished(); } QString EditLocallyHandler::prefixSlashToPath(const QString &path) @@ -165,11 +225,12 @@ bool EditLocallyHandler::isRelPathExcluded(const QString &relPath) return true; } -void EditLocallyHandler::showError(const QString &message, const QString &informativeText) const +void EditLocallyHandler::showError(const QString &message, const QString &informativeText) { showErrorNotification(message, informativeText); // to make sure the error is not missed, show a message box in addition showErrorMessageBox(message, informativeText); + Q_EMIT error(message, informativeText); } void EditLocallyHandler::showErrorNotification(const QString &message, const QString &informativeText) const @@ -203,36 +264,16 @@ void EditLocallyHandler::showErrorMessageBox(const QString &message, const QStri void EditLocallyHandler::startEditLocally() { - if (!_ready) { + if (_fileName.isEmpty() || _localFilePath.isEmpty() || !_folderForFile) { + qCWarning(lcEditLocallyHandler) << "Could not start to edit locally." + << "fileName:" << _fileName + << "localFilePath:" << _localFilePath + << "folderForFile:" << _folderForFile; return; } Systray::instance()->createEditFileLocallyLoadingDialog(_fileName); - const auto encodedToken = QString::fromUtf8(QUrl::toPercentEncoding(_token)); // Sanitise the token - const auto encodedRelPath = QUrl::toPercentEncoding(_relPath); // Sanitise the relPath - const auto checkEditLocallyToken = new SimpleApiJob(_accountState->account(), QStringLiteral("/ocs/v2.php/apps/files/api/v1/openlocaleditor/%1").arg(encodedToken)); - - QUrlQuery params; - params.addQueryItem(QStringLiteral("path"), prefixSlashToPath(_relPath)); - checkEditLocallyToken->addQueryParams(params); - checkEditLocallyToken->setVerb(SimpleApiJob::Verb::Post); - connect(checkEditLocallyToken, &SimpleApiJob::resultReceived, this, &EditLocallyHandler::remoteTokenCheckFinished); - - checkEditLocallyToken->start(); -} - -void EditLocallyHandler::remoteTokenCheckFinished(const int statusCode) -{ - constexpr auto HTTP_OK_CODE = 200; - if (statusCode != HTTP_OK_CODE) { - Systray::instance()->destroyEditFileLocallyLoadingDialog(); - - showError(tr("Could not validate the request to open a file from server."), _relPath); - qCInfo(lcEditLocallyHandler) << "token check result" << statusCode; - return; - } - _folderForFile->startSync(); const auto syncFinishedConnection = connect(_folderForFile, &Folder::syncFinished, this, &EditLocallyHandler::folderSyncFinished); @@ -248,6 +289,10 @@ void EditLocallyHandler::folderSyncFinished(const OCC::SyncResult &result) void EditLocallyHandler::disconnectSyncFinished() const { + if(_localFilePath.isEmpty()) { + return; + } + if (const auto existingConnection = editLocallySyncFinishedConnections.value(_localFilePath)) { disconnect(existingConnection); editLocallySyncFinishedConnections.remove(_localFilePath); @@ -256,6 +301,11 @@ void EditLocallyHandler::disconnectSyncFinished() const void EditLocallyHandler::openFile() { + if(_localFilePath.isEmpty()) { + qCWarning(lcEditLocallyHandler) << "Could not edit locally. Invalid local file path."; + return; + } + const auto localFilePath = _localFilePath; // In case the VFS mode is enabled and a file is not yet hydrated, we must call QDesktopServices::openUrl // from a separate thread, or, there will be a freeze. To avoid searching for a specific folder and checking @@ -265,7 +315,7 @@ void EditLocallyHandler::openFile() Systray::instance()->destroyEditFileLocallyLoadingDialog(); }); - Q_EMIT finished(); + Q_EMIT fileOpened(); } } diff --git a/src/gui/editlocallyhandler.h b/src/gui/editlocallyhandler.h index 9d6f9677c235e..04791ce93e031 100644 --- a/src/gui/editlocallyhandler.h +++ b/src/gui/editlocallyhandler.h @@ -36,30 +36,34 @@ class EditLocallyHandler : public QObject [[nodiscard]] static bool isRelPathExcluded(const QString &relPath); [[nodiscard]] static QString prefixSlashToPath(const QString &path); - [[nodiscard]] bool ready() const; - signals: - void finished(); + void setupFinished(); + void error(const QString &message, const QString &informativeText); + void fileOpened(); public slots: + void startSetup(); void startEditLocally(); void startTokenRemoteCheck(); private slots: - void showError(const QString &message, const QString &informativeText) const; + void proceedWithSetup(); + + void showError(const QString &message, const QString &informativeText); void showErrorNotification(const QString &message, const QString &informativeText) const; void showErrorMessageBox(const QString &message, const QString &informativeText) const; - void remoteTokenCheckFinished(const int statusCode); + void remoteTokenCheckResultReceived(const int statusCode); void folderSyncFinished(const OCC::SyncResult &result); void disconnectSyncFinished() const; void openFile(); private: - bool _ready = false; + bool _tokenVerified = false; AccountStatePtr _accountState; + QString _userId; QString _relPath; QString _token; From f093e1cfab064e0804874b179ca7ab69433d1d27 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 28 Oct 2022 16:43:30 +0200 Subject: [PATCH 4/9] Don't show the user any potenitally questionable remote data Signed-off-by: Claudio Cambra --- src/gui/editlocallyhandler.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gui/editlocallyhandler.cpp b/src/gui/editlocallyhandler.cpp index 65f12911c4f9c..80d38b07c3245 100644 --- a/src/gui/editlocallyhandler.cpp +++ b/src/gui/editlocallyhandler.cpp @@ -70,7 +70,7 @@ void EditLocallyHandler::startSetup() if (!_accountState) { qCWarning(lcEditLocallyHandler) << "Could not find an account " << _userId << " to edit file " << _relPath << " locally."; - showError(tr("Could not find an account for local editing."), _userId); + showError(tr("Could not find an account for local editing."), tr("Please try again.")); return; } @@ -110,7 +110,7 @@ void EditLocallyHandler::remoteTokenCheckResultReceived(const int statusCode) _tokenVerified = statusCode == HTTP_OK_CODE; if (!_tokenVerified) { - showError(tr("Could not validate the request to open a file from server."), _relPath); + showError(tr("Could not validate the request to open a file from server."), tr("Please try again.")); } proceedWithSetup(); From 376b978cfdbc9f77e7369e3915198e09f9c44c24 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 28 Oct 2022 19:40:06 +0200 Subject: [PATCH 5/9] Refactor edit locally handler management away from application and into own class Signed-off-by: Claudio Cambra --- src/gui/CMakeLists.txt | 2 + src/gui/application.cpp | 39 ++------------ src/gui/application.h | 2 - src/gui/editlocallyhandler.cpp | 13 +++-- src/gui/editlocallyhandler.h | 9 +++- src/gui/editlocallymanager.cpp | 93 ++++++++++++++++++++++++++++++++++ src/gui/editlocallymanager.h | 56 ++++++++++++++++++++ 7 files changed, 169 insertions(+), 45 deletions(-) create mode 100644 src/gui/editlocallymanager.cpp create mode 100644 src/gui/editlocallymanager.h diff --git a/src/gui/CMakeLists.txt b/src/gui/CMakeLists.txt index 4b1b60a6a9e72..f6189c0722181 100644 --- a/src/gui/CMakeLists.txt +++ b/src/gui/CMakeLists.txt @@ -82,6 +82,8 @@ set(client_SRCS connectionvalidator.cpp editlocallyhandler.h editlocallyhandler.cpp + editlocallymanager.h + editlocallymanager.cpp folder.h folder.cpp foldercreationdialog.h diff --git a/src/gui/application.cpp b/src/gui/application.cpp index 93b33655bef79..1f2ab6eb6147d 100644 --- a/src/gui/application.cpp +++ b/src/gui/application.cpp @@ -22,7 +22,7 @@ #include "config.h" #include "account.h" #include "accountstate.h" -#include "editlocallyhandler.h" +#include "editlocallymanager.h" #include "connectionvalidator.h" #include "folder.h" #include "folderman.h" @@ -750,43 +750,10 @@ void Application::handleEditLocallyFromOptions() return; } - handleEditLocally(_editFileLocallyUrl); + EditLocallyManager::instance()->editLocally(_editFileLocallyUrl); _editFileLocallyUrl.clear(); } -void Application::handleEditLocally(const QUrl &url) const -{ - auto pathSplit = url.path().split('/', Qt::SkipEmptyParts); - - if (pathSplit.size() < 2) { - qCWarning(lcApplication) << "Invalid URL for file local editing: " + pathSplit.join('/'); - return; - } - - // for a sample URL "nc://open/admin@nextcloud.lan:8080/Photos/lovely.jpg", QUrl::path would return "admin@nextcloud.lan:8080/Photos/lovely.jpg" - const auto userId = pathSplit.takeFirst(); - const auto fileRemotePath = pathSplit.join('/'); - const auto urlQuery = QUrlQuery{url}; - - auto token = QString{}; - if (urlQuery.hasQueryItem(QStringLiteral("token"))) { - token = urlQuery.queryItemValue(QStringLiteral("token")); - } else { - qCWarning(lcApplication) << "Invalid URL for file local editing: missing token"; - } - - // We need to make sure the handler sticks around until it is finished - const auto editLocallyHandler = new EditLocallyHandler(userId, fileRemotePath, token); - const auto editLocallyHandlerDeleter = [editLocallyHandler]{ delete editLocallyHandler; }; - connect(editLocallyHandler, &EditLocallyHandler::error, this, editLocallyHandlerDeleter); - connect(editLocallyHandler, &EditLocallyHandler::fileOpened, this, editLocallyHandlerDeleter); - - connect(editLocallyHandler, &EditLocallyHandler::setupFinished, - editLocallyHandler, [editLocallyHandler]{ editLocallyHandler->startEditLocally(); }); - editLocallyHandler->startSetup(); - -} - QString substLang(const QString &lang) { // Map the more appropriate script codes @@ -927,7 +894,7 @@ bool Application::event(QEvent *event) // On macOS, Qt does not handle receiving a custom URI as it does on other systems (as an application argument). // Instead, it sends out a QFileOpenEvent. We therefore need custom handling for our URI handling on macOS. qCInfo(lcApplication) << "macOS: Opening local file for editing: " << openEvent->url(); - handleEditLocally(openEvent->url()); + EditLocallyManager::instance()->editLocally(openEvent->url()); } else { const auto errorParsingLocalFileEditingUrl = QStringLiteral("The supplied url for local file editing '%1' is invalid!").arg(openEvent->url().toString()); qCInfo(lcApplication) << errorParsingLocalFileEditingUrl; diff --git a/src/gui/application.h b/src/gui/application.h index edbb0b7121398..d00d8bc0e28ed 100644 --- a/src/gui/application.h +++ b/src/gui/application.h @@ -87,8 +87,6 @@ public slots: /// Attempt to show() the tray icon again. Used if no systray was available initially. void tryTrayAgain(); - void handleEditLocally(const QUrl &url) const; - protected: void parseOptions(const QStringList &); void setupTranslations(); diff --git a/src/gui/editlocallyhandler.cpp b/src/gui/editlocallyhandler.cpp index 80d38b07c3245..64210a7ff21ab 100644 --- a/src/gui/editlocallyhandler.cpp +++ b/src/gui/editlocallyhandler.cpp @@ -19,6 +19,7 @@ #include #include "accountmanager.h" +#include "editlocallymanager.h" #include "folder.h" #include "folderman.h" #include "syncengine.h" @@ -28,8 +29,6 @@ namespace OCC { Q_LOGGING_CATEGORY(lcEditLocallyHandler, "nextcloud.gui.editlocallyhandler", QtInfoMsg) -static QHash editLocallySyncFinishedConnections; - EditLocallyHandler::EditLocallyHandler(const QString &userId, const QString &relPath, const QString &token, @@ -277,7 +276,9 @@ void EditLocallyHandler::startEditLocally() _folderForFile->startSync(); const auto syncFinishedConnection = connect(_folderForFile, &Folder::syncFinished, this, &EditLocallyHandler::folderSyncFinished); - editLocallySyncFinishedConnections.insert(_localFilePath, syncFinishedConnection); + + EditLocallyManager::instance()->folderSyncFinishedConnections.insert(_localFilePath, + syncFinishedConnection); } void EditLocallyHandler::folderSyncFinished(const OCC::SyncResult &result) @@ -293,9 +294,11 @@ void EditLocallyHandler::disconnectSyncFinished() const return; } - if (const auto existingConnection = editLocallySyncFinishedConnections.value(_localFilePath)) { + const auto manager = EditLocallyManager::instance(); + + if (const auto existingConnection = manager->folderSyncFinishedConnections.value(_localFilePath)) { disconnect(existingConnection); - editLocallySyncFinishedConnections.remove(_localFilePath); + manager->folderSyncFinishedConnections.remove(_localFilePath); } } diff --git a/src/gui/editlocallyhandler.h b/src/gui/editlocallyhandler.h index 04791ce93e031..f7b27b71a2fff 100644 --- a/src/gui/editlocallyhandler.h +++ b/src/gui/editlocallyhandler.h @@ -16,11 +16,16 @@ #include -#include "accountmanager.h" -#include "folder.h" +#include "accountstate.h" namespace OCC { +class EditLocallyHandler; +using EditLocallyHandlerPtr = QSharedPointer; + +class Folder; +class SyncResult; + class EditLocallyHandler : public QObject { Q_OBJECT diff --git a/src/gui/editlocallymanager.cpp b/src/gui/editlocallymanager.cpp new file mode 100644 index 0000000000000..1d8bea8c9c0fa --- /dev/null +++ b/src/gui/editlocallymanager.cpp @@ -0,0 +1,93 @@ +/* + * Copyright (C) by Claudio Cambra + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + */ + +#include "editlocallymanager.h" + +#include +#include + +#include "editlocallyhandler.h" + +namespace OCC { + +Q_LOGGING_CATEGORY(lcEditLocallyManager, "nextcloud.gui.editlocallymanager", QtInfoMsg) + +EditLocallyManager *EditLocallyManager::_instance = nullptr; + +EditLocallyManager::EditLocallyManager(QObject *parent) + : QObject{parent} +{ +} + +EditLocallyManager *EditLocallyManager::instance() +{ + if (!_instance) { + _instance = new EditLocallyManager(); + } + return _instance; +} + +void EditLocallyManager::editLocally(const QUrl &url) +{ + const auto inputs = parseEditLocallyUrl(url); + createHandler(inputs.userId, inputs.relPath, inputs.token); +} + +EditLocallyManager::EditLocallyInputData EditLocallyManager::parseEditLocallyUrl(const QUrl &url) +{ + const auto separator = QChar::fromLatin1('/'); + auto pathSplit = url.path().split(separator, Qt::SkipEmptyParts); + + if (pathSplit.size() < 2) { + qCWarning(lcEditLocallyManager) << "Invalid URL for file local editing: " + pathSplit.join(separator); + return {}; + } + + // for a sample URL "nc://open/admin@nextcloud.lan:8080/Photos/lovely.jpg", QUrl::path would return "admin@nextcloud.lan:8080/Photos/lovely.jpg" + const auto userId = pathSplit.takeFirst(); + const auto fileRemotePath = pathSplit.join(separator); + const auto urlQuery = QUrlQuery{url}; + + auto token = QString{}; + if (urlQuery.hasQueryItem(QStringLiteral("token"))) { + token = urlQuery.queryItemValue(QStringLiteral("token")); + } else { + qCWarning(lcEditLocallyManager) << "Invalid URL for file local editing: missing token"; + } + + return {userId, fileRemotePath, token}; +} + +void EditLocallyManager::createHandler(const QString &userId, + const QString &relPath, + const QString &token) +{ + const EditLocallyHandlerPtr handler(new EditLocallyHandler(userId, relPath, token)); + // We need to make sure the handler sticks around until it is finished + _handlers.insert(token, handler); + + const auto removeHandler = [this, token] { _handlers.remove(token); }; + const auto setupHandler = [handler] { handler->startEditLocally(); }; + + connect(handler.data(), &EditLocallyHandler::error, + this, removeHandler); + connect(handler.data(), &EditLocallyHandler::fileOpened, + this, removeHandler); + connect(handler.data(), &EditLocallyHandler::setupFinished, + handler.data(), setupHandler); + + handler->startSetup(); +} + +} diff --git a/src/gui/editlocallymanager.h b/src/gui/editlocallymanager.h new file mode 100644 index 0000000000000..3ba414fca77b3 --- /dev/null +++ b/src/gui/editlocallymanager.h @@ -0,0 +1,56 @@ +/* + * Copyright (C) by Claudio Cambra + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + */ + +#pragma once + +#include +#include + +#include "editlocallyhandler.h" + +namespace OCC { + +class EditLocallyManager : public QObject +{ + Q_OBJECT + +public: + [[nodiscard]] static EditLocallyManager *instance(); + + QHash folderSyncFinishedConnections; + +public slots: + void editLocally(const QUrl &url); + +private slots: + void createHandler(const QString &userId, + const QString &relPath, + const QString &token); + +private: + explicit EditLocallyManager(QObject *parent = nullptr); + static EditLocallyManager *_instance; + + struct EditLocallyInputData { + QString userId; + QString relPath; + QString token; + }; + + [[nodiscard]] static EditLocallyInputData parseEditLocallyUrl(const QUrl &url); + + QHash _handlers; +}; + +} From 7df2cdba3363dadae04b0a131cf171860516c83d Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Fri, 28 Oct 2022 20:22:27 +0200 Subject: [PATCH 6/9] Show edit locally loading dialog sooner, improve feedback for user Signed-off-by: Claudio Cambra --- src/gui/editlocallyhandler.cpp | 28 ++++++++----------- src/gui/editlocallyhandler.h | 2 +- src/gui/tray/EditFileLocallyLoadingDialog.qml | 3 +- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/gui/editlocallyhandler.cpp b/src/gui/editlocallyhandler.cpp index 64210a7ff21ab..74396b20ceab5 100644 --- a/src/gui/editlocallyhandler.cpp +++ b/src/gui/editlocallyhandler.cpp @@ -18,7 +18,6 @@ #include #include -#include "accountmanager.h" #include "editlocallymanager.h" #include "folder.h" #include "folderman.h" @@ -50,6 +49,10 @@ void EditLocallyHandler::startSetup() return; } + // Show the loading dialog but don't show the filename until we have + // verified the token + Systray::instance()->createEditFileLocallyLoadingDialog({}); + // We check the input data locally first, without modifying any state or // showing any potentially misleading data to the user if (!isTokenValid(_token)) { @@ -110,6 +113,7 @@ void EditLocallyHandler::remoteTokenCheckResultReceived(const int statusCode) if (!_tokenVerified) { showError(tr("Could not validate the request to open a file from server."), tr("Please try again.")); + return; } proceedWithSetup(); @@ -149,17 +153,13 @@ void EditLocallyHandler::proceedWithSetup() _fileName = relPathSplit.last(); + Systray::instance()->destroyEditFileLocallyLoadingDialog(); Q_EMIT setupFinished(); } QString EditLocallyHandler::prefixSlashToPath(const QString &path) { - auto slashPrefixedPath = path; - if (!slashPrefixedPath.startsWith('/')) { - slashPrefixedPath.prepend('/'); - } - - return slashPrefixedPath; + return path.startsWith('/') ? path : QChar::fromLatin1('/') + path; } bool EditLocallyHandler::isTokenValid(const QString &token) @@ -173,12 +173,7 @@ bool EditLocallyHandler::isTokenValid(const QString &token) const QRegularExpression tokenRegex("^[a-zA-Z0-9]{128}$"); const auto regexMatch = tokenRegex.match(token); - // Means invalid token type received, be cautious with bad token - if(!regexMatch.hasMatch()) { - return false; - } - - return true; + return regexMatch.hasMatch(); } bool EditLocallyHandler::isRelPathValid(const QString &relPath) @@ -207,7 +202,7 @@ bool EditLocallyHandler::isRelPathValid(const QString &relPath) bool EditLocallyHandler::isRelPathExcluded(const QString &relPath) { if (relPath.isEmpty()) { - return true; + return false; } const auto folderMap = FolderMan::instance()->map(); @@ -216,16 +211,17 @@ bool EditLocallyHandler::isRelPathExcluded(const QString &relPath) const auto excludedThroughSelectiveSync = folder->journalDb()->getSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, &result); for (const auto &excludedPath : excludedThroughSelectiveSync) { if (relPath.startsWith(excludedPath)) { - return false; + return true; } } } - return true; + return false; } void EditLocallyHandler::showError(const QString &message, const QString &informativeText) { + Systray::instance()->destroyEditFileLocallyLoadingDialog(); showErrorNotification(message, informativeText); // to make sure the error is not missed, show a message box in addition showErrorMessageBox(message, informativeText); diff --git a/src/gui/editlocallyhandler.h b/src/gui/editlocallyhandler.h index f7b27b71a2fff..05b3076e7b9c1 100644 --- a/src/gui/editlocallyhandler.h +++ b/src/gui/editlocallyhandler.h @@ -49,9 +49,9 @@ class EditLocallyHandler : public QObject public slots: void startSetup(); void startEditLocally(); - void startTokenRemoteCheck(); private slots: + void startTokenRemoteCheck(); void proceedWithSetup(); void showError(const QString &message, const QString &informativeText); diff --git a/src/gui/tray/EditFileLocallyLoadingDialog.qml b/src/gui/tray/EditFileLocallyLoadingDialog.qml index f7fbe965b3a49..12e8f757b4bb5 100644 --- a/src/gui/tray/EditFileLocallyLoadingDialog.qml +++ b/src/gui/tray/EditFileLocallyLoadingDialog.qml @@ -65,13 +65,14 @@ Window { font.pixelSize: root.fontPixelSize color: Style.ncTextColor horizontalAlignment: Text.AlignHCenter + visible: root.fileName !== "" } Label { id: labelMessage Layout.alignment: Qt.AlignHCenter Layout.fillWidth: true Layout.bottomMargin: Style.standardSpacing - text: qsTr("Opening for local editing") + text: qsTr("Opening file for local editing") elide: Text.ElideRight font.pixelSize: root.fontPixelSize color: Style.ncTextColor From 3777abac9df28147bcdd4207f6540020940ab19b Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Sat, 29 Oct 2022 00:19:04 +0200 Subject: [PATCH 7/9] Ensure the checkTokenJob object gets deallocated Signed-off-by: Claudio Cambra --- src/gui/editlocallyhandler.cpp | 12 +++++++----- src/gui/editlocallyhandler.h | 1 + 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/gui/editlocallyhandler.cpp b/src/gui/editlocallyhandler.cpp index 74396b20ceab5..50e22698a823d 100644 --- a/src/gui/editlocallyhandler.cpp +++ b/src/gui/editlocallyhandler.cpp @@ -93,15 +93,17 @@ void EditLocallyHandler::startTokenRemoteCheck() const auto encodedToken = QString::fromUtf8(QUrl::toPercentEncoding(_token)); // Sanitise the token const auto encodedRelPath = QUrl::toPercentEncoding(_relPath); // Sanitise the relPath - const auto checkEditLocallyToken = new SimpleApiJob(_accountState->account(), QStringLiteral("/ocs/v2.php/apps/files/api/v1/openlocaleditor/%1").arg(encodedToken)); + + _checkTokenJob.reset(new SimpleApiJob(_accountState->account(), + QStringLiteral("/ocs/v2.php/apps/files/api/v1/openlocaleditor/%1").arg(encodedToken))); QUrlQuery params; params.addQueryItem(QStringLiteral("path"), prefixSlashToPath(encodedRelPath)); - checkEditLocallyToken->addQueryParams(params); - checkEditLocallyToken->setVerb(SimpleApiJob::Verb::Post); - connect(checkEditLocallyToken, &SimpleApiJob::resultReceived, this, &EditLocallyHandler::remoteTokenCheckResultReceived); + _checkTokenJob->addQueryParams(params); + _checkTokenJob->setVerb(SimpleApiJob::Verb::Post); + connect(_checkTokenJob.get(), &SimpleApiJob::resultReceived, this, &EditLocallyHandler::remoteTokenCheckResultReceived); - checkEditLocallyToken->start(); + _checkTokenJob->start(); } void EditLocallyHandler::remoteTokenCheckResultReceived(const int statusCode) diff --git a/src/gui/editlocallyhandler.h b/src/gui/editlocallyhandler.h index 05b3076e7b9c1..0093191b80849 100644 --- a/src/gui/editlocallyhandler.h +++ b/src/gui/editlocallyhandler.h @@ -75,6 +75,7 @@ private slots: QString _fileName; QString _localFilePath; Folder *_folderForFile = nullptr; + std::unique_ptr _checkTokenJob; }; } From bbc35b359edb08410fa58a404cca056c8bd7459c Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Sat, 29 Oct 2022 13:21:56 +0200 Subject: [PATCH 8/9] Rename EditLocallyHandler to EditLocallyJob Signed-off-by: Claudio Cambra --- src/gui/CMakeLists.txt | 4 +- ...tlocallyhandler.cpp => editlocallyjob.cpp} | 58 +++++++++---------- ...{editlocallyhandler.h => editlocallyjob.h} | 14 ++--- src/gui/editlocallymanager.cpp | 30 +++++----- src/gui/editlocallymanager.h | 10 ++-- 5 files changed, 57 insertions(+), 59 deletions(-) rename src/gui/{editlocallyhandler.cpp => editlocallyjob.cpp} (81%) rename src/gui/{editlocallyhandler.h => editlocallyjob.h} (84%) diff --git a/src/gui/CMakeLists.txt b/src/gui/CMakeLists.txt index f6189c0722181..b9a177807b46c 100644 --- a/src/gui/CMakeLists.txt +++ b/src/gui/CMakeLists.txt @@ -80,8 +80,8 @@ set(client_SRCS conflictsolver.cpp connectionvalidator.h connectionvalidator.cpp - editlocallyhandler.h - editlocallyhandler.cpp + editlocallyjob.h + editlocallyjob.cpp editlocallymanager.h editlocallymanager.cpp folder.h diff --git a/src/gui/editlocallyhandler.cpp b/src/gui/editlocallyjob.cpp similarity index 81% rename from src/gui/editlocallyhandler.cpp rename to src/gui/editlocallyjob.cpp index 50e22698a823d..63f9515f8e471 100644 --- a/src/gui/editlocallyhandler.cpp +++ b/src/gui/editlocallyjob.cpp @@ -12,7 +12,7 @@ * for more details. */ -#include "editlocallyhandler.h" +#include "editlocallyjob.h" #include #include @@ -26,9 +26,9 @@ namespace OCC { -Q_LOGGING_CATEGORY(lcEditLocallyHandler, "nextcloud.gui.editlocallyhandler", QtInfoMsg) +Q_LOGGING_CATEGORY(lcEditLocallyJob, "nextcloud.gui.editlocallyjob", QtInfoMsg) -EditLocallyHandler::EditLocallyHandler(const QString &userId, +EditLocallyJob::EditLocallyJob(const QString &userId, const QString &relPath, const QString &token, QObject *parent) @@ -39,10 +39,10 @@ EditLocallyHandler::EditLocallyHandler(const QString &userId, { } -void EditLocallyHandler::startSetup() +void EditLocallyJob::startSetup() { if (_token.isEmpty() || _relPath.isEmpty() || _userId.isEmpty()) { - qCWarning(lcEditLocallyHandler) << "Could not start setup." + qCWarning(lcEditLocallyJob) << "Could not start setup." << "token:" << _token << "relPath:" << _relPath << "userId" << _userId; @@ -56,14 +56,14 @@ void EditLocallyHandler::startSetup() // We check the input data locally first, without modifying any state or // showing any potentially misleading data to the user if (!isTokenValid(_token)) { - qCWarning(lcEditLocallyHandler) << "Edit locally request is missing a valid token, will not open file. " + qCWarning(lcEditLocallyJob) << "Edit locally request is missing a valid token, will not open file. " << "Token received was:" << _token; showError(tr("Invalid token received."), tr("Please try again.")); return; } if (!isRelPathValid(_relPath)) { - qCWarning(lcEditLocallyHandler) << "Provided relPath was:" << _relPath << "which is not canonical."; + qCWarning(lcEditLocallyJob) << "Provided relPath was:" << _relPath << "which is not canonical."; showError(tr("Invalid file path was provided."), tr("Please try again.")); return; } @@ -71,7 +71,7 @@ void EditLocallyHandler::startSetup() _accountState = AccountManager::instance()->accountFromUserId(_userId); if (!_accountState) { - qCWarning(lcEditLocallyHandler) << "Could not find an account " << _userId << " to edit file " << _relPath << " locally."; + qCWarning(lcEditLocallyJob) << "Could not find an account " << _userId << " to edit file " << _relPath << " locally."; showError(tr("Could not find an account for local editing."), tr("Please try again.")); return; } @@ -81,10 +81,10 @@ void EditLocallyHandler::startSetup() startTokenRemoteCheck(); } -void EditLocallyHandler::startTokenRemoteCheck() +void EditLocallyJob::startTokenRemoteCheck() { if (!_accountState || _relPath.isEmpty() || _token.isEmpty()) { - qCWarning(lcEditLocallyHandler) << "Could not start token check." + qCWarning(lcEditLocallyJob) << "Could not start token check." << "accountState:" << _accountState << "relPath:" << _relPath << "token:" << _token; @@ -101,14 +101,14 @@ void EditLocallyHandler::startTokenRemoteCheck() params.addQueryItem(QStringLiteral("path"), prefixSlashToPath(encodedRelPath)); _checkTokenJob->addQueryParams(params); _checkTokenJob->setVerb(SimpleApiJob::Verb::Post); - connect(_checkTokenJob.get(), &SimpleApiJob::resultReceived, this, &EditLocallyHandler::remoteTokenCheckResultReceived); + connect(_checkTokenJob.get(), &SimpleApiJob::resultReceived, this, &EditLocallyJob::remoteTokenCheckResultReceived); _checkTokenJob->start(); } -void EditLocallyHandler::remoteTokenCheckResultReceived(const int statusCode) +void EditLocallyJob::remoteTokenCheckResultReceived(const int statusCode) { - qCInfo(lcEditLocallyHandler) << "token check result" << statusCode; + qCInfo(lcEditLocallyJob) << "token check result" << statusCode; constexpr auto HTTP_OK_CODE = 200; _tokenVerified = statusCode == HTTP_OK_CODE; @@ -121,10 +121,10 @@ void EditLocallyHandler::remoteTokenCheckResultReceived(const int statusCode) proceedWithSetup(); } -void EditLocallyHandler::proceedWithSetup() +void EditLocallyJob::proceedWithSetup() { if (!_tokenVerified) { - qCWarning(lcEditLocallyHandler) << "Could not proceed with setup as token is not verified."; + qCWarning(lcEditLocallyJob) << "Could not proceed with setup as token is not verified."; return; } @@ -159,12 +159,12 @@ void EditLocallyHandler::proceedWithSetup() Q_EMIT setupFinished(); } -QString EditLocallyHandler::prefixSlashToPath(const QString &path) +QString EditLocallyJob::prefixSlashToPath(const QString &path) { return path.startsWith('/') ? path : QChar::fromLatin1('/') + path; } -bool EditLocallyHandler::isTokenValid(const QString &token) +bool EditLocallyJob::isTokenValid(const QString &token) { if (token.isEmpty()) { return false; @@ -178,7 +178,7 @@ bool EditLocallyHandler::isTokenValid(const QString &token) return regexMatch.hasMatch(); } -bool EditLocallyHandler::isRelPathValid(const QString &relPath) +bool EditLocallyJob::isRelPathValid(const QString &relPath) { if (relPath.isEmpty()) { return false; @@ -201,7 +201,7 @@ bool EditLocallyHandler::isRelPathValid(const QString &relPath) return true; } -bool EditLocallyHandler::isRelPathExcluded(const QString &relPath) +bool EditLocallyJob::isRelPathExcluded(const QString &relPath) { if (relPath.isEmpty()) { return false; @@ -221,7 +221,7 @@ bool EditLocallyHandler::isRelPathExcluded(const QString &relPath) return false; } -void EditLocallyHandler::showError(const QString &message, const QString &informativeText) +void EditLocallyJob::showError(const QString &message, const QString &informativeText) { Systray::instance()->destroyEditFileLocallyLoadingDialog(); showErrorNotification(message, informativeText); @@ -230,7 +230,7 @@ void EditLocallyHandler::showError(const QString &message, const QString &inform Q_EMIT error(message, informativeText); } -void EditLocallyHandler::showErrorNotification(const QString &message, const QString &informativeText) const +void EditLocallyJob::showErrorNotification(const QString &message, const QString &informativeText) const { if (!_accountState || !_accountState->account()) { return; @@ -246,7 +246,7 @@ void EditLocallyHandler::showErrorNotification(const QString &message, const QSt } } -void EditLocallyHandler::showErrorMessageBox(const QString &message, const QString &informativeText) const +void EditLocallyJob::showErrorMessageBox(const QString &message, const QString &informativeText) const { const auto messageBox = new QMessageBox; messageBox->setAttribute(Qt::WA_DeleteOnClose); @@ -259,10 +259,10 @@ void EditLocallyHandler::showErrorMessageBox(const QString &message, const QStri messageBox->raise(); } -void EditLocallyHandler::startEditLocally() +void EditLocallyJob::startEditLocally() { if (_fileName.isEmpty() || _localFilePath.isEmpty() || !_folderForFile) { - qCWarning(lcEditLocallyHandler) << "Could not start to edit locally." + qCWarning(lcEditLocallyJob) << "Could not start to edit locally." << "fileName:" << _fileName << "localFilePath:" << _localFilePath << "folderForFile:" << _folderForFile; @@ -273,20 +273,20 @@ void EditLocallyHandler::startEditLocally() _folderForFile->startSync(); const auto syncFinishedConnection = connect(_folderForFile, &Folder::syncFinished, - this, &EditLocallyHandler::folderSyncFinished); + this, &EditLocallyJob::folderSyncFinished); EditLocallyManager::instance()->folderSyncFinishedConnections.insert(_localFilePath, syncFinishedConnection); } -void EditLocallyHandler::folderSyncFinished(const OCC::SyncResult &result) +void EditLocallyJob::folderSyncFinished(const OCC::SyncResult &result) { Q_UNUSED(result) disconnectSyncFinished(); openFile(); } -void EditLocallyHandler::disconnectSyncFinished() const +void EditLocallyJob::disconnectSyncFinished() const { if(_localFilePath.isEmpty()) { return; @@ -300,10 +300,10 @@ void EditLocallyHandler::disconnectSyncFinished() const } } -void EditLocallyHandler::openFile() +void EditLocallyJob::openFile() { if(_localFilePath.isEmpty()) { - qCWarning(lcEditLocallyHandler) << "Could not edit locally. Invalid local file path."; + qCWarning(lcEditLocallyJob) << "Could not edit locally. Invalid local file path."; return; } diff --git a/src/gui/editlocallyhandler.h b/src/gui/editlocallyjob.h similarity index 84% rename from src/gui/editlocallyhandler.h rename to src/gui/editlocallyjob.h index 0093191b80849..258382a80a2bf 100644 --- a/src/gui/editlocallyhandler.h +++ b/src/gui/editlocallyjob.h @@ -20,21 +20,21 @@ namespace OCC { -class EditLocallyHandler; -using EditLocallyHandlerPtr = QSharedPointer; +class EditLocallyJob; +using EditLocallyJobPtr = QSharedPointer; class Folder; class SyncResult; -class EditLocallyHandler : public QObject +class EditLocallyJob : public QObject { Q_OBJECT public: - explicit EditLocallyHandler(const QString &userId, - const QString &relPath, - const QString &token, - QObject *parent = nullptr); + explicit EditLocallyJob(const QString &userId, + const QString &relPath, + const QString &token, + QObject *parent = nullptr); [[nodiscard]] static bool isTokenValid(const QString &token); [[nodiscard]] static bool isRelPathValid(const QString &relPath); diff --git a/src/gui/editlocallymanager.cpp b/src/gui/editlocallymanager.cpp index 1d8bea8c9c0fa..567a4abbd80a0 100644 --- a/src/gui/editlocallymanager.cpp +++ b/src/gui/editlocallymanager.cpp @@ -17,8 +17,6 @@ #include #include -#include "editlocallyhandler.h" - namespace OCC { Q_LOGGING_CATEGORY(lcEditLocallyManager, "nextcloud.gui.editlocallymanager", QtInfoMsg) @@ -41,7 +39,7 @@ EditLocallyManager *EditLocallyManager::instance() void EditLocallyManager::editLocally(const QUrl &url) { const auto inputs = parseEditLocallyUrl(url); - createHandler(inputs.userId, inputs.relPath, inputs.token); + createJob(inputs.userId, inputs.relPath, inputs.token); } EditLocallyManager::EditLocallyInputData EditLocallyManager::parseEditLocallyUrl(const QUrl &url) @@ -69,25 +67,25 @@ EditLocallyManager::EditLocallyInputData EditLocallyManager::parseEditLocallyUrl return {userId, fileRemotePath, token}; } -void EditLocallyManager::createHandler(const QString &userId, +void EditLocallyManager::createJob(const QString &userId, const QString &relPath, const QString &token) { - const EditLocallyHandlerPtr handler(new EditLocallyHandler(userId, relPath, token)); - // We need to make sure the handler sticks around until it is finished - _handlers.insert(token, handler); + const EditLocallyJobPtr job(new EditLocallyJob(userId, relPath, token)); + // We need to make sure the job sticks around until it is finished + _jobs.insert(token, job); - const auto removeHandler = [this, token] { _handlers.remove(token); }; - const auto setupHandler = [handler] { handler->startEditLocally(); }; + const auto removeJob = [this, token] { _jobs.remove(token); }; + const auto setupJob = [job] { job->startEditLocally(); }; - connect(handler.data(), &EditLocallyHandler::error, - this, removeHandler); - connect(handler.data(), &EditLocallyHandler::fileOpened, - this, removeHandler); - connect(handler.data(), &EditLocallyHandler::setupFinished, - handler.data(), setupHandler); + connect(job.data(), &EditLocallyJob::error, + this, removeJob); + connect(job.data(), &EditLocallyJob::fileOpened, + this, removeJob); + connect(job.data(), &EditLocallyJob::setupFinished, + job.data(), setupJob); - handler->startSetup(); + job->startSetup(); } } diff --git a/src/gui/editlocallymanager.h b/src/gui/editlocallymanager.h index 3ba414fca77b3..cf42f1ed76fb8 100644 --- a/src/gui/editlocallymanager.h +++ b/src/gui/editlocallymanager.h @@ -17,7 +17,7 @@ #include #include -#include "editlocallyhandler.h" +#include "editlocallyjob.h" namespace OCC { @@ -34,9 +34,9 @@ public slots: void editLocally(const QUrl &url); private slots: - void createHandler(const QString &userId, - const QString &relPath, - const QString &token); + void createJob(const QString &userId, + const QString &relPath, + const QString &token); private: explicit EditLocallyManager(QObject *parent = nullptr); @@ -50,7 +50,7 @@ private slots: [[nodiscard]] static EditLocallyInputData parseEditLocallyUrl(const QUrl &url); - QHash _handlers; + QHash _jobs; }; } From 13b704316393660556ce4c00d32c7a613330f18a Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Mon, 31 Oct 2022 11:48:45 +0100 Subject: [PATCH 9/9] remove use of attribute nodiscard Signed-off-by: Matthieu Gallien --- src/gui/accountmanager.h | 2 +- src/gui/editlocallyjob.h | 8 ++++---- src/gui/editlocallymanager.h | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/gui/accountmanager.h b/src/gui/accountmanager.h index ef257a4ce0156..1941041a25182 100644 --- a/src/gui/accountmanager.h +++ b/src/gui/accountmanager.h @@ -69,7 +69,7 @@ class AccountManager : public QObject * Return the account state pointer for an account from its id */ - [[nodiscard]] AccountStatePtr accountFromUserId(const QString &id) const; + AccountStatePtr accountFromUserId(const QString &id) const; /** * Delete the AccountState diff --git a/src/gui/editlocallyjob.h b/src/gui/editlocallyjob.h index 258382a80a2bf..4ee92bea7f451 100644 --- a/src/gui/editlocallyjob.h +++ b/src/gui/editlocallyjob.h @@ -36,10 +36,10 @@ class EditLocallyJob : public QObject const QString &token, QObject *parent = nullptr); - [[nodiscard]] static bool isTokenValid(const QString &token); - [[nodiscard]] static bool isRelPathValid(const QString &relPath); - [[nodiscard]] static bool isRelPathExcluded(const QString &relPath); - [[nodiscard]] static QString prefixSlashToPath(const QString &path); + static bool isTokenValid(const QString &token); + static bool isRelPathValid(const QString &relPath); + static bool isRelPathExcluded(const QString &relPath); + static QString prefixSlashToPath(const QString &path); signals: void setupFinished(); diff --git a/src/gui/editlocallymanager.h b/src/gui/editlocallymanager.h index cf42f1ed76fb8..1606e1289ce68 100644 --- a/src/gui/editlocallymanager.h +++ b/src/gui/editlocallymanager.h @@ -26,7 +26,7 @@ class EditLocallyManager : public QObject Q_OBJECT public: - [[nodiscard]] static EditLocallyManager *instance(); + static EditLocallyManager *instance(); QHash folderSyncFinishedConnections; @@ -48,7 +48,7 @@ private slots: QString token; }; - [[nodiscard]] static EditLocallyInputData parseEditLocallyUrl(const QUrl &url); + static EditLocallyInputData parseEditLocallyUrl(const QUrl &url); QHash _jobs; };