From 0375d887f26223963a0cd83b43ccb7022491c32e Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 22 Dec 2017 10:02:11 -0800 Subject: [PATCH] Plugins: Add validation to plugin descriptor parsing (#27951) This commit checks there are no leftover unparsed elements when parsing a plugin descriptor. --- .../org/elasticsearch/plugins/PluginInfo.java | 38 +++++++++++++------ .../plugins/PluginInfoTests.java | 18 ++++++++- 2 files changed, 43 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/plugins/PluginInfo.java b/core/src/main/java/org/elasticsearch/plugins/PluginInfo.java index c301b65738b..4418a0a68c4 100644 --- a/core/src/main/java/org/elasticsearch/plugins/PluginInfo.java +++ b/core/src/main/java/org/elasticsearch/plugins/PluginInfo.java @@ -34,7 +34,10 @@ import java.io.InputStream; import java.nio.file.Files; import java.nio.file.Path; import java.util.Locale; +import java.util.Map; import java.util.Properties; +import java.util.function.Function; +import java.util.stream.Collectors; /** * An in-memory representation of the plugin descriptor. @@ -119,27 +122,33 @@ public class PluginInfo implements Writeable, ToXContentObject { */ public static PluginInfo readFromProperties(final Path path) throws IOException { final Path descriptor = path.resolve(ES_PLUGIN_PROPERTIES); - final Properties props = new Properties(); - try (InputStream stream = Files.newInputStream(descriptor)) { - props.load(stream); + + final Map propsMap; + { + final Properties props = new Properties(); + try (InputStream stream = Files.newInputStream(descriptor)) { + props.load(stream); + } + propsMap = props.stringPropertyNames().stream().collect(Collectors.toMap(Function.identity(), props::getProperty)); } - final String name = props.getProperty("name"); + + final String name = propsMap.remove("name"); if (name == null || name.isEmpty()) { throw new IllegalArgumentException( "property [name] is missing in [" + descriptor + "]"); } - final String description = props.getProperty("description"); + final String description = propsMap.remove("description"); if (description == null) { throw new IllegalArgumentException( "property [description] is missing for plugin [" + name + "]"); } - final String version = props.getProperty("version"); + final String version = propsMap.remove("version"); if (version == null) { throw new IllegalArgumentException( "property [version] is missing for plugin [" + name + "]"); } - final String esVersionString = props.getProperty("elasticsearch.version"); + final String esVersionString = propsMap.remove("elasticsearch.version"); if (esVersionString == null) { throw new IllegalArgumentException( "property [elasticsearch.version] is missing for plugin [" + name + "]"); @@ -154,20 +163,20 @@ public class PluginInfo implements Writeable, ToXContentObject { esVersionString); throw new IllegalArgumentException(message); } - final String javaVersionString = props.getProperty("java.version"); + final String javaVersionString = propsMap.remove("java.version"); if (javaVersionString == null) { throw new IllegalArgumentException( "property [java.version] is missing for plugin [" + name + "]"); } JarHell.checkVersionFormat(javaVersionString); JarHell.checkJavaVersion(name, javaVersionString); - final String classname = props.getProperty("classname"); + final String classname = propsMap.remove("classname"); if (classname == null) { throw new IllegalArgumentException( "property [classname] is missing for plugin [" + name + "]"); } - final String hasNativeControllerValue = props.getProperty("has.native.controller"); + final String hasNativeControllerValue = propsMap.remove("has.native.controller"); final boolean hasNativeController; if (hasNativeControllerValue == null) { hasNativeController = false; @@ -191,7 +200,10 @@ public class PluginInfo implements Writeable, ToXContentObject { } } - final String requiresKeystoreValue = props.getProperty("requires.keystore", "false"); + String requiresKeystoreValue = propsMap.remove("requires.keystore"); + if (requiresKeystoreValue == null) { + requiresKeystoreValue = "false"; + } final boolean requiresKeystore; try { requiresKeystore = Booleans.parseBoolean(requiresKeystoreValue); @@ -200,6 +212,10 @@ public class PluginInfo implements Writeable, ToXContentObject { " but was [" + requiresKeystoreValue + "]", e); } + if (propsMap.isEmpty() == false) { + throw new IllegalArgumentException("Unknown properties in plugin descriptor: " + propsMap.keySet()); + } + return new PluginInfo(name, description, version, classname, hasNativeController, requiresKeystore); } diff --git a/core/src/test/java/org/elasticsearch/plugins/PluginInfoTests.java b/core/src/test/java/org/elasticsearch/plugins/PluginInfoTests.java index ceb70ec9e55..7e15e7b1bad 100644 --- a/core/src/test/java/org/elasticsearch/plugins/PluginInfoTests.java +++ b/core/src/test/java/org/elasticsearch/plugins/PluginInfoTests.java @@ -31,7 +31,7 @@ import java.util.stream.Collectors; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.core.IsEqual.equalTo; +import static org.hamcrest.Matchers.equalTo; public class PluginInfoTests extends ESTestCase { @@ -170,10 +170,24 @@ public class PluginInfoTests extends ESTestCase { plugins.add(new PluginInfo("d", "foo", "dummy", "dummyclass", randomBoolean(), randomBoolean())); PluginsAndModules pluginsInfo = new PluginsAndModules(plugins, Collections.emptyList()); - final List infos = pluginsInfo.getPluginInfos(); List names = infos.stream().map(PluginInfo::getName).collect(Collectors.toList()); assertThat(names, contains("a", "b", "c", "d", "e")); } + public void testUnknownProperties() throws Exception { + Path pluginDir = createTempDir().resolve("fake-plugin"); + PluginTestUtil.writeProperties(pluginDir, + "extra", "property", + "unknown", "property", + "description", "fake desc", + "classname", "Foo", + "name", "my_plugin", + "version", "1.0", + "elasticsearch.version", Version.CURRENT.toString(), + "java.version", System.getProperty("java.specification.version")); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> PluginInfo.readFromProperties(pluginDir)); + assertThat(e.getMessage(), containsString("Unknown properties in plugin descriptor")); + } + }