From fcf4114adcad731726eb6fa10da5eb14deb3b2f9 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Wed, 10 Jan 2018 14:59:37 +0100 Subject: [PATCH] Make sure that we don't detect files as maven coordinate when installing a plugin (#28163) * This change makes sure that we don't detect a file path containing a ':' as a maven coordinate (e.g.: `file:C:\path\to\zip`) * restore test muted on master --- .../elasticsearch/plugins/InstallPluginCommand.java | 4 ++-- .../plugins/InstallPluginCommandTests.java | 12 ++++++++++++ plugins/examples/meta-plugin/build.gradle | 7 ++----- .../test/smoke_test_plugins/10_basic.yml | 5 ++--- 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java b/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java index 6fdc7e62c15..a8b7db48a7c 100644 --- a/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java +++ b/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java @@ -232,14 +232,14 @@ class InstallPluginCommand extends EnvironmentAwareCommand { // now try as maven coordinates, a valid URL would only have a colon and slash String[] coordinates = pluginId.split(":"); - if (coordinates.length == 3 && pluginId.contains("/") == false) { + if (coordinates.length == 3 && pluginId.contains("/") == false && pluginId.startsWith("file:") == false) { String mavenUrl = getMavenUrl(terminal, coordinates, Platforms.PLATFORM_NAME); terminal.println("-> Downloading " + pluginId + " from maven central"); return downloadZipAndChecksum(terminal, mavenUrl, tmpDir, true); } // fall back to plain old URL - if (pluginId.contains(":/") == false) { + if (pluginId.contains(":") == false) { // definitely not a valid url, so assume it is a plugin name List plugins = checkMisspelledPlugin(pluginId); String msg = "Unknown plugin " + pluginId; diff --git a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java index 993a231423e..db5a6a16a9d 100644 --- a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java +++ b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java @@ -22,6 +22,7 @@ package org.elasticsearch.plugins; import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; import com.google.common.jimfs.Configuration; import com.google.common.jimfs.Jimfs; +import org.apache.lucene.util.IOUtils; import org.apache.lucene.util.LuceneTestCase; import org.elasticsearch.Version; import org.elasticsearch.cli.ExitCodes; @@ -44,6 +45,7 @@ import org.junit.After; import org.junit.Before; import java.io.BufferedReader; +import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.io.StringReader; @@ -430,6 +432,16 @@ public class InstallPluginCommandTests extends ESTestCase { assertTrue(e.getMessage(), e.getMessage().contains("no protocol")); } + public void testFileNotMaven() throws Exception { + Tuple env = createEnv(fs, temp); + String dir = randomAlphaOfLength(10) + ":" + randomAlphaOfLength(5) + "\\" + randomAlphaOfLength(5); + Exception e = expectThrows(Exception.class, + // has two colons, so it appears similar to maven coordinates + () -> installPlugin("file:" + dir, env.v1())); + assertFalse(e.getMessage(), e.getMessage().contains("maven.org")); + assertTrue(e.getMessage(), e.getMessage().contains(dir)); + } + public void testUnknownPlugin() throws Exception { Tuple env = createEnv(fs, temp); UserException e = expectThrows(UserException.class, () -> installPlugin("foo", env.v1())); diff --git a/plugins/examples/meta-plugin/build.gradle b/plugins/examples/meta-plugin/build.gradle index a920c877642..3674837b0b2 100644 --- a/plugins/examples/meta-plugin/build.gradle +++ b/plugins/examples/meta-plugin/build.gradle @@ -50,10 +50,7 @@ integTestCluster { distribution = 'zip' // Install the meta plugin before start. - /** - * NORELEASE Tests fail on windows, see https://github.com/elastic/elasticsearch/pull/28163 - */ - //setupCommand 'installMetaPlugin', - // 'bin/elasticsearch-plugin', 'install', 'file:' + buildZip.archivePath + setupCommand 'installMetaPlugin', + 'bin/elasticsearch-plugin', 'install', 'file:' + buildZip.archivePath } check.dependsOn integTest diff --git a/plugins/examples/meta-plugin/src/test/resources/rest-api-spec/test/smoke_test_plugins/10_basic.yml b/plugins/examples/meta-plugin/src/test/resources/rest-api-spec/test/smoke_test_plugins/10_basic.yml index d3744aa5234..011a278ed89 100644 --- a/plugins/examples/meta-plugin/src/test/resources/rest-api-spec/test/smoke_test_plugins/10_basic.yml +++ b/plugins/examples/meta-plugin/src/test/resources/rest-api-spec/test/smoke_test_plugins/10_basic.yml @@ -10,6 +10,5 @@ - do: nodes.info: {} -# NORELEASE Tests fail on windows, see https://github.com/elastic/elasticsearch/pull/28163 -# - match: { nodes.$master.plugins.0.name: dummy-plugin1 } -# - match: { nodes.$master.plugins.1.name: dummy-plugin2 } + - match: { nodes.$master.plugins.0.name: dummy-plugin1 } + - match: { nodes.$master.plugins.1.name: dummy-plugin2 }