From 52b6586594cba31e6729c3f254dc55e864798738 Mon Sep 17 00:00:00 2001
From: Stefan Forstenlechner <stefan@forstenlechner.dev>
Date: Wed, 19 Jun 2024 17:57:41 +0200
Subject: [PATCH] Add error handling

in case directory could not be created or permissions could not be set
---
 .../remote/DatabaseSettingsWidgetRemote.cpp   |  6 ++--
 src/gui/remote/RemoteHandler.cpp              | 35 ++++++++++++++-----
 src/gui/remote/RemoteHandler.h                |  2 +-
 3 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/src/gui/remote/DatabaseSettingsWidgetRemote.cpp b/src/gui/remote/DatabaseSettingsWidgetRemote.cpp
index 87d6dbab3..5266c5947 100644
--- a/src/gui/remote/DatabaseSettingsWidgetRemote.cpp
+++ b/src/gui/remote/DatabaseSettingsWidgetRemote.cpp
@@ -177,25 +177,27 @@ void DatabaseSettingsWidgetRemote::testDownload()
     params->downloadCommand = m_ui->downloadCommand->text();
     params->downloadInput = m_ui->inputForDownload->toPlainText();
 
-    QScopedPointer<RemoteHandler> remoteHandler(new RemoteHandler(this));
     if (params->downloadCommand.isEmpty()) {
         m_ui->messageWidget->showMessage(tr("Download command cannot be empty."), MessageWidget::Warning);
         return;
     }
 
+    QScopedPointer<RemoteHandler> remoteHandler(new RemoteHandler(this));
     RemoteHandler::RemoteResult result = remoteHandler->download(params);
     if (!result.success) {
         m_ui->messageWidget->showMessage(tr("Download failed with error: %1").arg(result.errorMessage),
                                          MessageWidget::Error);
+        remoteHandler->cleanup(result.filePath);
         return;
     }
 
     if (!QFile::exists(result.filePath)) {
         m_ui->messageWidget->showMessage(tr("Download finished, but file %1 could not be found.").arg(result.filePath),
                                          MessageWidget::Error);
+        remoteHandler->cleanup(result.filePath);
         return;
     }
 
     remoteHandler->cleanup(result.filePath);
     m_ui->messageWidget->showMessage(tr("Download successful."), MessageWidget::Positive);
-}
\ No newline at end of file
+}
diff --git a/src/gui/remote/RemoteHandler.cpp b/src/gui/remote/RemoteHandler.cpp
index 9fb23aa68..61753eb78 100644
--- a/src/gui/remote/RemoteHandler.cpp
+++ b/src/gui/remote/RemoteHandler.cpp
@@ -47,7 +47,15 @@ RemoteHandler::RemoteResult RemoteHandler::download(const RemoteParams* params)
             return result;
         }
 
-        auto filePath = getTempFileLocation();
+        QString error;
+        auto filePath = getTempFileLocation(&error);
+        result.filePath = filePath;
+        if (!error.isEmpty()) {
+            result.success = false;
+            result.errorMessage = error;
+            return result;
+        }
+
         auto remoteProcess = m_createRemoteProcess(nullptr); // use nullptr parent, otherwise there is a warning
         remoteProcess->setTempFileLocation(filePath);
         remoteProcess->start(params->downloadCommand);
@@ -72,7 +80,6 @@ RemoteHandler::RemoteResult RemoteHandler::download(const RemoteParams* params)
                 result.errorMessage = tr("Command `%1` failed to download database.").arg(params->downloadCommand);
             } else {
                 result.success = true;
-                result.filePath = filePath;
             }
         } else if (finished) {
             result.success = false;
@@ -135,7 +142,7 @@ RemoteHandler::RemoteResult RemoteHandler::upload(const QString& filePath, const
     });
 }
 
-QString RemoteHandler::getTempFileLocation()
+QString RemoteHandler::getTempFileLocation(QString* error)
 {
     QString uuid = QUuid::createUuid().toString().remove(0, 1);
     uuid.chop(1);
@@ -143,12 +150,24 @@ QString RemoteHandler::getTempFileLocation()
     if (writableLocation.isEmpty()) {
         writableLocation = QStandardPaths::writableLocation(QStandardPaths::TempLocation);
     }
-    QString tempLocation = QDir(writableLocation).absoluteFilePath(PREFIX + uuid);
-    QDir().mkdir(tempLocation);
-    QDir uuidPath(tempLocation);
-    QFile(uuidPath.path()).setPermissions(QFileDevice::ReadOwner | QFileDevice::WriteOwner | QFileDevice::ExeOwner);
 
-    return QDir::toNativeSeparators(uuidPath.absoluteFilePath("RemoteDatabase-" + uuid + ".kdbx"));
+    QString tempDirLocation = QDir(writableLocation).absoluteFilePath(PREFIX + uuid);
+    QString tempFileLocation =
+        QDir::toNativeSeparators(QDir(tempDirLocation).absoluteFilePath("RemoteDatabase-" + uuid + ".kdbx"));
+
+    if (!QDir().mkdir(tempDirLocation)) {
+        *error = tr("Could not create temporary directory '%1'").arg(tempDirLocation);
+        return "";
+    }
+
+    if (!QFile::setPermissions(tempDirLocation,
+                               QFileDevice::ReadOwner | QFileDevice::WriteOwner | QFileDevice::ExeOwner)) {
+        QDir(tempDirLocation).removeRecursively();
+        *error = tr("Could not change permissions of temporary directory '%1' to owner").arg(tempDirLocation);
+        return "";
+    }
+
+    return tempFileLocation;
 }
 
 void RemoteHandler::cleanup(QString& tempFileLocation)
diff --git a/src/gui/remote/RemoteHandler.h b/src/gui/remote/RemoteHandler.h
index 497dc4a62..ca31a2883 100644
--- a/src/gui/remote/RemoteHandler.h
+++ b/src/gui/remote/RemoteHandler.h
@@ -50,7 +50,7 @@ public:
     static void setRemoteProcessFunc(std::function<QScopedPointer<RemoteProcess>(QObject*)> func);
 
 private:
-    static QString getTempFileLocation();
+    static QString getTempFileLocation(QString* error);
 
     static std::function<QScopedPointer<RemoteProcess>(QObject*)> m_createRemoteProcess;
     inline static const QString PREFIX = "KPXC-Sync-";