bin/plugin removes itself

If you call `bin/plugin --remove es-plugin` the plugin got removed but the file `bin/plugin` itself was also deleted.

We now don't allow the following plugin names:

* elasticsearch
* plugin
* elasticsearch.bat
* plugin.bat
* elasticsearch.in.sh
* service.bat

Closes #6745
This commit is contained in:
David Pilato 2014-07-10 19:39:16 +02:00
parent db7f0d36af
commit 26bac39e0e
2 changed files with 45 additions and 5 deletions

View File

@ -20,6 +20,7 @@
package org.elasticsearch.plugins; package org.elasticsearch.plugins;
import com.google.common.base.Strings; import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet;
import org.elasticsearch.ElasticsearchIllegalArgumentException; import org.elasticsearch.ElasticsearchIllegalArgumentException;
import org.elasticsearch.ElasticsearchIllegalStateException; import org.elasticsearch.ElasticsearchIllegalStateException;
import org.elasticsearch.ElasticsearchTimeoutException; import org.elasticsearch.ElasticsearchTimeoutException;
@ -46,6 +47,7 @@ import java.util.*;
import java.util.zip.ZipEntry; import java.util.zip.ZipEntry;
import java.util.zip.ZipFile; import java.util.zip.ZipFile;
import static org.elasticsearch.common.Strings.hasLength;
import static org.elasticsearch.common.settings.ImmutableSettings.Builder.EMPTY_SETTINGS; import static org.elasticsearch.common.settings.ImmutableSettings.Builder.EMPTY_SETTINGS;
/** /**
@ -66,6 +68,14 @@ public class PluginManager {
// By default timeout is 0 which means no timeout // By default timeout is 0 which means no timeout
public static final TimeValue DEFAULT_TIMEOUT = TimeValue.timeValueMillis(0); public static final TimeValue DEFAULT_TIMEOUT = TimeValue.timeValueMillis(0);
private static final ImmutableSet<Object> BLACKLIST = ImmutableSet.builder()
.add("elasticsearch",
"elasticsearch.bat",
"elasticsearch.in.sh",
"plugin",
"plugin.bat",
"service.bat").build();
private final Environment environment; private final Environment environment;
private String url; private String url;
@ -123,6 +133,8 @@ public class PluginManager {
} }
PluginHandle pluginHandle = PluginHandle.parse(name); PluginHandle pluginHandle = PluginHandle.parse(name);
checkForForbiddenName(pluginHandle.name);
File pluginFile = pluginHandle.distroFile(environment); File pluginFile = pluginHandle.distroFile(environment);
// extract the plugin // extract the plugin
File extractLocation = pluginHandle.extractedDir(environment); File extractLocation = pluginHandle.extractedDir(environment);
@ -254,10 +266,7 @@ public class PluginManager {
PluginHandle pluginHandle = PluginHandle.parse(name); PluginHandle pluginHandle = PluginHandle.parse(name);
boolean removed = false; boolean removed = false;
if (Strings.isNullOrEmpty(pluginHandle.name)) { checkForForbiddenName(pluginHandle.name);
throw new ElasticsearchIllegalArgumentException("plugin name is incorrect");
}
File pluginToDelete = pluginHandle.extractedDir(environment); File pluginToDelete = pluginHandle.extractedDir(environment);
if (pluginToDelete.exists()) { if (pluginToDelete.exists()) {
debug("Removing: " + pluginToDelete.getPath()); debug("Removing: " + pluginToDelete.getPath());
@ -301,6 +310,12 @@ public class PluginManager {
} }
} }
private static void checkForForbiddenName(String name) {
if (!hasLength(name) || BLACKLIST.contains(name.toLowerCase(Locale.ROOT))) {
throw new ElasticsearchIllegalArgumentException("Illegal plugin name: " + name);
}
}
public File[] getListInstalledPlugins() { public File[] getListInstalledPlugins() {
File[] plugins = environment.pluginsFile().listFiles(); File[] plugins = environment.pluginsFile().listFiles();
return plugins; return plugins;

View File

@ -252,7 +252,7 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest {
public void testInstallPlugin() throws IOException { public void testInstallPlugin() throws IOException {
PluginManager pluginManager = pluginManager(getPluginUrlForResource("plugin_with_classfile.zip")); PluginManager pluginManager = pluginManager(getPluginUrlForResource("plugin_with_classfile.zip"));
pluginManager.downloadAndExtract("plugin"); pluginManager.downloadAndExtract("plugin-classfile");
File[] plugins = pluginManager.getListInstalledPlugins(); File[] plugins = pluginManager.getListInstalledPlugins();
assertThat(plugins, notNullValue()); assertThat(plugins, notNullValue());
assertThat(plugins.length, is(1)); assertThat(plugins.length, is(1));
@ -374,6 +374,31 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest {
pluginManager.removePlugin("file://whatever"); pluginManager.removePlugin("file://whatever");
} }
@Test
public void testForbiddenPluginName_ThrowsException() throws IOException {
runTestWithForbiddenName(null);
runTestWithForbiddenName("");
runTestWithForbiddenName("elasticsearch");
runTestWithForbiddenName("elasticsearch.bat");
runTestWithForbiddenName("elasticsearch.in.sh");
runTestWithForbiddenName("plugin");
runTestWithForbiddenName("plugin.bat");
runTestWithForbiddenName("service.bat");
runTestWithForbiddenName("ELASTICSEARCH");
runTestWithForbiddenName("ELASTICSEARCH.IN.SH");
}
private void runTestWithForbiddenName(String name) throws IOException {
try {
pluginManager(null).removePlugin(name);
fail("this plugin name [" + name +
"] should not be allowed");
} catch (ElasticsearchIllegalArgumentException e) {
// We expect that error
}
}
/** /**
* Retrieve a URL string that represents the resource with the given {@code resourceName}. * Retrieve a URL string that represents the resource with the given {@code resourceName}.
* @param resourceName The resource name relative to {@link PluginManagerTests}. * @param resourceName The resource name relative to {@link PluginManagerTests}.