From 1fcfe14a9d2deceb4c7465f95cccff6a872e74fa Mon Sep 17 00:00:00 2001
From: Eike Ziller <eike.ziller@qt.io>
Date: Wed, 11 Dec 2024 11:28:40 +0100
Subject: [PATCH] PluginInstall: Prevent selection of installed plugin

Prevent selection of plugin libraries that are already in any plugin
directory. Check if they can resolve their dependencies as well.
Also generally check if a plugin with the same ID is already installed.

Change-Id: I0cdc0b3e0eba8682f90db7a460974106a6d5e44c
Reviewed-by: Marcus Tillmanns <marcus.tillmanns@qt.io>
---
 .../coreplugin/plugininstallwizard.cpp        | 54 +++++++++++++++----
 1 file changed, 45 insertions(+), 9 deletions(-)

diff --git a/src/plugins/coreplugin/plugininstallwizard.cpp b/src/plugins/coreplugin/plugininstallwizard.cpp
index be49395d5b1..8f06015aec7 100644
--- a/src/plugins/coreplugin/plugininstallwizard.cpp
+++ b/src/plugins/coreplugin/plugininstallwizard.cpp
@@ -12,14 +12,16 @@
 
 #include <solutions/tasking/tasktreerunner.h>
 
+#include <utils/algorithm.h>
+#include <utils/appinfo.h>
 #include <utils/async.h>
 #include <utils/fileutils.h>
 #include <utils/hostosinfo.h>
 #include <utils/infolabel.h>
 #include <utils/layoutbuilder.h>
 #include <utils/pathchooser.h>
-#include <utils/qtcprocess.h>
 #include <utils/qtcassert.h>
+#include <utils/qtcprocess.h>
 #include <utils/temporarydirectory.h>
 #include <utils/unarchiver.h>
 #include <utils/wizard.h>
@@ -102,8 +104,15 @@ public:
             m_info->setText(Tr::tr("File does not exist."));
             return false;
         }
-        if (hasLibSuffix(path))
+        if (hasLibSuffix(path)) {
+            if (Utils::anyOf(PluginManager::pluginPaths(), [path](const FilePath &pluginPath) {
+                    return path.isChildOf(pluginPath);
+                })) {
+                m_info->setText(Tr::tr("Plugin is already installed."));
+                return false;
+            }
             return true;
+        }
 
         const auto sourceAndCommand = Unarchiver::sourceAndCommand(path);
         if (!sourceAndCommand)
@@ -118,6 +127,31 @@ public:
 
 using CheckResult = expected_str<PluginSpec *>;
 
+static Result checkPlugin(PluginSpec *spec)
+{
+    if (Utils::anyOf(PluginManager::plugins(), [spec](PluginSpec *other) {
+            return other->id() == spec->id();
+        }))
+        return Result::Error(
+            Tr::tr("A plugin with ID \"%1\" is already installed.").arg(spec->id()));
+    if (!spec->resolveDependencies(PluginManager::plugins())) {
+        return Result::Error(
+            Tr::tr("Plugin failed to resolve dependencies:") + " " + spec->errorString());
+    }
+    return Result::Ok;
+}
+
+static expected_str<std::unique_ptr<PluginSpec>> checkPlugin(
+    expected_str<std::unique_ptr<PluginSpec>> spec)
+{
+    if (!spec)
+        return spec;
+    const Result ok = checkPlugin(spec->get());
+    if (ok)
+        return spec;
+    return Utils::make_unexpected(ok.error());
+}
+
 // Async. Result is set if any issue was found.
 void checkContents(QPromise<CheckResult> &promise, const FilePath &tempDir)
 {
@@ -132,15 +166,15 @@ void checkContents(QPromise<CheckResult> &promise, const FilePath &tempDir)
         return;
     }
 
-    if (!plugins.front()->resolveDependencies(PluginManager::plugins())) {
-        promise.addResult(Utils::make_unexpected(
-            Tr::tr("Plugin failed to resolve dependencies:") + " "
-            + plugins.front()->errorString()));
-        qDeleteAll(plugins);
+    PluginSpec *plugin = plugins.front();
+    const Result ok = checkPlugin(plugin);
+    if (!ok) {
+        promise.addResult(Utils::make_unexpected(ok.error()));
+        delete plugin;
         return;
     }
 
-    promise.addResult(plugins.front());
+    promise.addResult(plugin);
 }
 
 class CheckArchivePage : public WizardPage
@@ -180,7 +214,8 @@ public:
         emit completeChanged();
         if (hasLibSuffix(m_data->sourcePath)) {
             m_cancelButton->setVisible(false);
-            expected_str<std::unique_ptr<PluginSpec>> spec = readCppPluginSpec(m_data->sourcePath);
+            expected_str<std::unique_ptr<PluginSpec>> spec = checkPlugin(
+                readCppPluginSpec(m_data->sourcePath));
             if (!spec) {
                 m_label->setType(InfoLabel::Error);
                 m_label->setText(spec.error());
@@ -434,6 +469,7 @@ static std::function<void(FilePath)> postCopyOperation()
 static bool copyPluginFile(const FilePath &src, const FilePath &dest)
 {
     const FilePath destFile = dest.pathAppended(src.fileName());
+    QTC_ASSERT(src != destFile, return true);
     if (destFile.exists()) {
         QMessageBox box(QMessageBox::Question,
                         Tr::tr("Overwrite File"),