Make plugin loading stricter

Today we load plugins reflectively, looking for constructors that
conform to specific signatures. This commit tightens the reflective
operations here, not allowing plugins to have ambiguous constructors.

Relates #25405

1 files changed, 36 insertions, 16 deletions

diff --git a/core/src/main/java/org/elasticsearch/plugins/PluginsService.java b/core/src/main/java/org/elasticsearch/plugins/PluginsService.java
index 5219fabf24..2e0ec0f242 100644
--- a/core/src/main/java/org/elasticsearch/plugins/PluginsService.java
+++ b/core/src/main/java/org/elasticsearch/plugins/PluginsService.java
@@ -42,6 +42,7 @@ import org.elasticsearch.index.IndexModule;
 import org.elasticsearch.threadpool.ExecutorBuilder;
 
 import java.io.IOException;
+import java.lang.reflect.Constructor;
 import java.net.URL;
 import java.net.URLClassLoader;
 import java.nio.file.DirectoryStream;
@@ -418,27 +419,46 @@ public class PluginsService extends AbstractComponent {
     }
 
     private Plugin loadPlugin(Class<? extends Plugin> pluginClass, Settings settings, Path configPath) {
+        final Constructor<?>[] constructors = pluginClass.getConstructors();
+        if (constructors.length == 0) {
+            throw new IllegalStateException("no public constructor for [" + pluginClass.getName() + "]");
+        }
+
+        if (constructors.length > 1) {
+            throw new IllegalStateException("no unique public constructor for [" + pluginClass.getName() + "]");
+        }
+
+        final Constructor<?> constructor = constructors[0];
+        if (constructor.getParameterCount() > 2) {
+            throw new IllegalStateException(signatureMessage(pluginClass));
+        }
+
+        final Class[] parameterTypes = constructor.getParameterTypes();
         try {
-            try {
-                return pluginClass.getConstructor(Settings.class, Path.class).newInstance(settings, configPath);
-            } catch (NoSuchMethodException e) {
-                try {
-                    return pluginClass.getConstructor(Settings.class).newInstance(settings);
-                } catch (NoSuchMethodException e1) {
-                    try {
-                        return pluginClass.getConstructor().newInstance();
-                    } catch (NoSuchMethodException e2) {
-                        throw new ElasticsearchException("No constructor for [" + pluginClass + "]. A plugin class must " +
-                                "have either an empty default constructor, a single argument constructor accepting a " +
-                                "Settings instance, or a single argument constructor accepting a pair of Settings, Path instances");
-                    }
-                }
+            if (constructor.getParameterCount() == 2 && parameterTypes[0] == Settings.class && parameterTypes[1] == Path.class) {
+                return (Plugin)constructor.newInstance(settings, configPath);
+            } else if (constructor.getParameterCount() == 1 && parameterTypes[0] == Settings.class) {
+                return (Plugin)constructor.newInstance(settings);
+            } else if (constructor.getParameterCount() == 0) {
+                return (Plugin)constructor.newInstance();
+            } else {
+                throw new IllegalStateException(signatureMessage(pluginClass));
             }
-        } catch (Exception e) {
-            throw new ElasticsearchException("Failed to load plugin class [" + pluginClass.getName() + "]", e);
+        } catch (final ReflectiveOperationException e) {
+            throw new IllegalStateException("failed to load plugin class [" + pluginClass.getName() + "]", e);
         }
     }
 
+    private String signatureMessage(final Class<? extends Plugin> clazz) {
+        return String.format(
+                Locale.ROOT,
+                "no public constructor of correct signature for [%s]; must be [%s], [%s], or [%s]",
+                clazz.getName(),
+                "(org.elasticsearch.common.settings.Settings,java.nio.file.Path)",
+                "(org.elasticsearch.common.settings.Settings)",
+                "()");
+    }
+
     public <T> List<T> filterPlugins(Class<T> type) {
         return plugins.stream().filter(x -> type.isAssignableFrom(x.v2().getClass()))
             .map(p -> ((T)p.v2())).collect(Collectors.toList());