GEODE-6364: Deploy of invalid jar file does not write file contents t…

…o config on locator (apache#3164)
jiarong4 · Feb 7, 2019 · 6820a3f · 6820a3f
1 parent 2d7a830
commit 6820a3f
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 0 deletions.
diff --git a/...a/org/apache/geode/management/internal/configuration/ClusterConfigDeployJarDUnitTest.java b/...a/org/apache/geode/management/internal/configuration/ClusterConfigDeployJarDUnitTest.java
@@ -18,10 +18,14 @@
 import static org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL;
 import static org.assertj.core.api.Assertions.assertThat;
 
+import java.io.File;
+import java.io.FileWriter;
+
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 
+import org.apache.geode.test.dunit.IgnoredException;
 import org.apache.geode.test.dunit.rules.ClusterStartupRule;
 import org.apache.geode.test.dunit.rules.MemberVM;
 import org.apache.geode.test.junit.rules.GfshCommandRule;
@@ -132,6 +136,34 @@ public void testDeploy() throws Exception {
     expectedGroup1and2Config.verify(server3);
   }
 
+  @Test
+  public void testInvalidJarDeploy() throws Exception {
+    IgnoredException.addIgnoredException(IllegalArgumentException.class);
+
+    // set up the locator/servers
+    MemberVM locator = lsRule.startLocatorVM(0, locatorProps);
+    // server1 in no group
+    MemberVM server1 = lsRule.startServerVM(1, serverProps, locator.getPort());
+
+    gfshConnector.connect(locator);
+    assertThat(gfshConnector.isConnected()).isTrue();
+
+    File junkFile = temporaryFolder.newFile("junk");
+    FileWriter writer = new FileWriter(junkFile);
+    writer.write("this is not a real jar");
+    writer.close();
+
+    // We want to ensure that a mix of good and bad jars does not produce a 'partial' deploy.
+    gfshConnector.executeAndAssertThat("deploy --jar=" + clusterJar + ","
+        + junkFile.getAbsolutePath()).statusIsError();
+    gfshConnector.executeAndAssertThat("list deployed").statusIsSuccess()
+        .containsOutput("No JAR Files Found");
+
+    ConfigGroup cluster = new ConfigGroup("cluster").jars();
+    ClusterConfig expectedClusterConfig = new ClusterConfig(cluster);
+    expectedClusterConfig.verify(locator);
+  }
+
   @Test
   public void testUndeploy() throws Exception {
     // set up the locator/servers

diff --git a/...e-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommand.java b/...e-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommand.java
@@ -37,6 +37,7 @@
 import org.apache.geode.cache.execute.ResultCollector;
 import org.apache.geode.distributed.DistributedMember;
 import org.apache.geode.distributed.internal.InternalConfigurationPersistenceService;
+import org.apache.geode.internal.DeployedJar;
 import org.apache.geode.management.cli.CliMetaData;
 import org.apache.geode.management.cli.ConverterHint;
 import org.apache.geode.management.cli.Result;
@@ -84,6 +85,8 @@ public Result deploy(
 
     List<String> jarFullPaths = CommandExecutionContext.getFilePathFromShell();
 
+    verifyJarContent(jarFullPaths);
+
     Set<DistributedMember> targetMembers;
     targetMembers = findMembers(groups, null);
 
@@ -148,6 +151,16 @@ public Result deploy(
     return result;
   }
 
+  private void verifyJarContent(List<String> jarNames) {
+    for (String jarName : jarNames) {
+      File jar = new File(jarName);
+      if (!DeployedJar.hasValidJarContent(jar)) {
+        throw new IllegalArgumentException(
+            "File does not contain valid JAR content: " + jar.getName());
+      }
+    }
+  }
+
   /**
    * Interceptor used by gfsh to intercept execution of deploy command at "shell".
    */