honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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
This commit is contained in:
Jason Tedor 2017-06-26 21:42:53 -04:00 committed by GitHub
parent 2765ea41ca
commit cca18a2c35
3 changed files with 128 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
buildSrc/src/main/resources/checkstyle_suppressions.xml
View File

@ -13,6 +13,9 @@
<!-- JNA requires the no-argument constructor on JNAKernel32Library.SizeT to be public-->
<suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]bootstrap[/\\]JNAKernel32Library.java" checks="RedundantModifier" />
<!-- the constructors on some local classes in these tests must be public-->
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]plugins[/\\]PluginsServiceTests.java" checks="RedundantModifier" />
<!-- Hopefully temporary suppression of LineLength on files that don't pass it. We should remove these when we the
files start to pass. -->
<suppress files="client[/\\]rest[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]client[/\\]HeapBufferedAsyncResponseConsumerTests.java" checks="LineLength" />

56
core/src/main/java/org/elasticsearch/plugins/PluginsService.java
View File

@ -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,25 +419,44 @@ public class PluginsService extends AbstractComponent {
}
private Plugin loadPlugin(Class<? extends Plugin> pluginClass, Settings settings, Path configPath) {
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");
}
}
}
} catch (Exception e) {
throw new ElasticsearchException("Failed to load plugin class [" + pluginClass.getName() + "]", e);
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 {
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 (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) {

87
core/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java
View File

@ -30,6 +30,7 @@ import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.Locale;
@ -151,4 +152,90 @@ public class PluginsServiceTests extends ESTestCase {
assertThat(e, hasToString(containsString(expected)));
}
public void testLoadPluginWithNoPublicConstructor() {
class NoPublicConstructorPlugin extends Plugin {
private NoPublicConstructorPlugin() {
}
}
final Path home = createTempDir();
final Settings settings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), home).build();
final IllegalStateException e =
expectThrows(IllegalStateException.class, () -> newPluginsService(settings, NoPublicConstructorPlugin.class));
assertThat(e, hasToString(containsString("no public constructor")));
}
public void testLoadPluginWithMultiplePublicConstructors() {
class MultiplePublicConstructorsPlugin extends Plugin {
@SuppressWarnings("unused")
public MultiplePublicConstructorsPlugin() {
}
@SuppressWarnings("unused")
public MultiplePublicConstructorsPlugin(final Settings settings) {
}
}
final Path home = createTempDir();
final Settings settings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), home).build();
final IllegalStateException e =
expectThrows(IllegalStateException.class, () -> newPluginsService(settings, MultiplePublicConstructorsPlugin.class));
assertThat(e, hasToString(containsString("no unique public constructor")));
}
public void testLoadPluginWithNoPublicConstructorOfCorrectSignature() {
class TooManyParametersPlugin extends Plugin {
@SuppressWarnings("unused")
public TooManyParametersPlugin(Settings settings, Path configPath, Object object) {
}
}
class TwoParametersFirstIncorrectType extends Plugin {
@SuppressWarnings("unused")
public TwoParametersFirstIncorrectType(Object object, Path configPath) {
}
}
class TwoParametersSecondIncorrectType extends Plugin {
@SuppressWarnings("unused")
public TwoParametersSecondIncorrectType(Settings settings, Object object) {
}
}
class OneParameterIncorrectType extends Plugin {
@SuppressWarnings("unused")
public OneParameterIncorrectType(Object object) {
}
}
final Collection<Class<? extends Plugin>> classes = Arrays.asList(
TooManyParametersPlugin.class,
TwoParametersFirstIncorrectType.class,
TwoParametersSecondIncorrectType.class,
OneParameterIncorrectType.class);
for (Class<? extends Plugin> pluginClass : classes) {
final Path home = createTempDir();
final Settings settings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), home).build();
final IllegalStateException e = expectThrows(IllegalStateException.class, () -> newPluginsService(settings, pluginClass));
assertThat(e, hasToString(containsString("no public constructor of correct signature")));
}
}
}