From 6224adf70a2f029b3d54e99cdb62eb6fc7b6d4e2 Mon Sep 17 00:00:00 2001
From: Andrew Steinborn <git@steinborn.me>
Date: Fri, 5 Jul 2024 22:01:31 -0400
Subject: [PATCH] Don't try to load a candidate plugin with the same ID more
 than once

Fixes #1372. Honestly, this was an oversight that I'm somewhat surprised that nobody caught until recently.
---
 .../proxy/plugin/VelocityPluginManager.java   | 21 ++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityPluginManager.java b/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityPluginManager.java
index 2c6f752f7..367219c63 100644
--- a/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityPluginManager.java
+++ b/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityPluginManager.java
@@ -43,13 +43,13 @@ import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashSet;
+import java.util.HashMap;
 import java.util.IdentityHashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
-import java.util.Set;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
@@ -107,14 +107,25 @@ public class VelocityPluginManager implements PluginManager {
 
     List<PluginDescription> sortedPlugins = PluginDependencyUtils.sortCandidates(found);
 
-    Set<String> loadedPluginsById = new HashSet<>();
+    Map<String, PluginDescription> loadedCandidates = new HashMap<>();
     Map<PluginContainer, Module> pluginContainers = new LinkedHashMap<>();
     // Now load the plugins
     pluginLoad:
     for (PluginDescription candidate : sortedPlugins) {
+      // If we found a duplicate candidate (with the same ID), don't load it.
+      PluginDescription existingCandidate = loadedCandidates.get(candidate.getId());
+      if (existingCandidate != null) {
+        logger.error("Refusing to load plugin at path {} since we already "
+            + "loaded a plugin with the same ID {} from {}",
+            candidate.getSource().map(Objects::toString).orElse("<UNKNOWN>"),
+            candidate.getId(),
+            existingCandidate.getSource().map(Objects::toString).orElse("<UNKNOWN>"));
+        continue;
+      }
+
       // Verify dependencies
       for (PluginDependency dependency : candidate.getDependencies()) {
-        if (!dependency.isOptional() && !loadedPluginsById.contains(dependency.getId())) {
+        if (!dependency.isOptional() && !loadedCandidates.containsKey(dependency.getId())) {
           logger.error("Can't load plugin {} due to missing dependency {}", candidate.getId(),
               dependency.getId());
           continue pluginLoad;
@@ -125,7 +136,7 @@ public class VelocityPluginManager implements PluginManager {
         PluginDescription realPlugin = loader.createPluginFromCandidate(candidate);
         VelocityPluginContainer container = new VelocityPluginContainer(realPlugin);
         pluginContainers.put(container, loader.createModule(container));
-        loadedPluginsById.add(realPlugin.getId());
+        loadedCandidates.put(realPlugin.getId(), realPlugin);
       } catch (Throwable e) {
         logger.error("Can't create module for plugin {}", candidate.getId(), e);
       }