From 5154f029108e3d3ba804b9d4555903347f2376c2 Mon Sep 17 00:00:00 2001
From: Andrew Steinborn <git@steinborn.me>
Date: Fri, 5 Jul 2024 22:07:15 -0400
Subject: [PATCH] Improved fix for #1372

Realized that the precondition that gets violated happens way too early, in `Maps.uniqueIndex()`.
---
 .../proxy/plugin/VelocityPluginManager.java   | 32 ++++++++++---------
 1 file changed, 17 insertions(+), 15 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 367219c63..0af477c42 100644
--- a/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityPluginManager.java
+++ b/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityPluginManager.java
@@ -86,43 +86,45 @@ public class VelocityPluginManager implements PluginManager {
     checkNotNull(directory, "directory");
     checkArgument(directory.toFile().isDirectory(), "provided path isn't a directory");
 
-    List<PluginDescription> found = new ArrayList<>();
+    Map<String, PluginDescription> foundCandidates = new LinkedHashMap<>();
     JavaPluginLoader loader = new JavaPluginLoader(server, directory);
 
     try (DirectoryStream<Path> stream = Files.newDirectoryStream(directory,
         p -> p.toFile().isFile() && p.toString().endsWith(".jar"))) {
       for (Path path : stream) {
         try {
-          found.add(loader.loadCandidate(path));
+          PluginDescription candidate = loader.loadCandidate(path);
+
+          // If we found a duplicate candidate (with the same ID), don't load it.
+          PluginDescription maybeExistingCandidate = foundCandidates.putIfAbsent(
+              candidate.getId(), candidate);
+
+          if (maybeExistingCandidate != 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(),
+                maybeExistingCandidate.getSource().map(Objects::toString).orElse("<UNKNOWN>"));
+          }
         } catch (Throwable e) {
           logger.error("Unable to load plugin {}", path, e);
         }
       }
     }
 
-    if (found.isEmpty()) {
+    if (foundCandidates.isEmpty()) {
       // No plugins found
       return;
     }
 
-    List<PluginDescription> sortedPlugins = PluginDependencyUtils.sortCandidates(found);
+    List<PluginDescription> sortedPlugins = PluginDependencyUtils.sortCandidates(
+        new ArrayList<>(foundCandidates.values()));
 
     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() && !loadedCandidates.containsKey(dependency.getId())) {