geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jinmeil...@apache.org
Subject [2/2] geode git commit: GEODE-2983: correctly handling --J option value that has ", " inside.
Date Thu, 01 Jun 2017 15:11:57 GMT
GEODE-2983: correctly handling --J option value that has "," inside.

(cherry picked from commit 72b9351)


Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/a2243f4f
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/a2243f4f
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/a2243f4f

Branch: refs/heads/release/1.2.0
Commit: a2243f4f451c2c527a916ba0f7a0ba8a4541d0e7
Parents: e62668a
Author: Jinmei Liao <jiliao@pivotal.io>
Authored: Wed May 31 10:39:22 2017 -0700
Committer: Jinmei Liao <jiliao@pivotal.io>
Committed: Thu Jun 1 08:11:07 2017 -0700

----------------------------------------------------------------------
 .../management/internal/cli/GfshParser.java     | 13 +++-
 .../cli/commands/LauncherLifecycleCommands.java | 18 +++--
 .../internal/cli/GfshParserJUnitTest.java       |  6 +-
 .../internal/cli/GfshParserParsingTest.java     | 80 +++++++++++++++++++-
 4 files changed, 101 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/a2243f4f/geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
index 288ea05..df16e9b 100755
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java
@@ -45,6 +45,10 @@ public class GfshParser extends SimpleParser {
   public static final String COMMAND_DELIMITER = ";";
   public static final String CONTINUATION_CHARACTER = "\\";
 
+  private static final char ASCII_UNIT_SEPARATOR = '\u001F';
+  public static final String J_ARGUMENT_DELIMITER = "" + ASCII_UNIT_SEPARATOR;
+  public static final String J_OPTION_CONTEXT = "splittingRegex=" + J_ARGUMENT_DELIMITER;
+
   // pattern used to split the user input with whitespaces except those in quotes (single
or double)
   private static Pattern PATTERN =
       Pattern.compile("\\s*([^\\s']*)'([^']*)'\\s+|\\s*([^\\s\"]*)\"([^\"]*)\"\\s+|\\S+");
@@ -131,6 +135,7 @@ public class GfshParser extends SimpleParser {
 
         if (i < tokens.size()) {
           String jArg = tokens.get(i);
+          // remove the quotes around each --J arugments
           if (jArg.charAt(0) == '"' || jArg.charAt(0) == '\'') {
             jArg = jArg.substring(1, jArg.length() - 1);
           }
@@ -151,7 +156,12 @@ public class GfshParser extends SimpleParser {
       if (i == firstJIndex) {
         rawInput.append("--J ");
         if (jArguments.size() > 0) {
-          rawInput.append("\"").append(StringUtils.join(jArguments, ",")).append("\" ");
+          // quote the entire J argument with double quotes, and delimited with a special
delimiter,
+          // and we
+          // need to tell the gfsh parser to use this delimiter when splitting the --J argument
in
+          // each command
+          rawInput.append("\"").append(StringUtils.join(jArguments, J_ARGUMENT_DELIMITER))
+              .append("\" ");
         }
       }
       // then add the next inputToken
@@ -176,7 +186,6 @@ public class GfshParser extends SimpleParser {
 
     return new GfshParseResult(result.getMethod(), result.getInstance(), result.getArguments(),
         userInput);
-
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/geode/blob/a2243f4f/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
index 74acfd6..a2ace7d 100755
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
@@ -81,6 +81,7 @@ import org.apache.geode.management.cli.ConverterHint;
 import org.apache.geode.management.cli.Result;
 import org.apache.geode.management.internal.ManagementConstants;
 import org.apache.geode.management.internal.cli.CliUtil;
+import org.apache.geode.management.internal.cli.GfshParser;
 import org.apache.geode.management.internal.cli.LogWrapper;
 import org.apache.geode.management.internal.cli.converters.ConnectionEndpointConverter;
 import org.apache.geode.management.internal.cli.domain.ConnectToLocatorResult;
@@ -228,7 +229,7 @@ public class LauncherLifecycleCommands extends AbstractCommandsSupport
{
           help = CliStrings.START_LOCATOR__INITIALHEAP__HELP) final String initialHeap,
       @CliOption(key = CliStrings.START_LOCATOR__MAXHEAP,
           help = CliStrings.START_LOCATOR__MAXHEAP__HELP) final String maxHeap,
-      @CliOption(key = CliStrings.START_LOCATOR__J,
+      @CliOption(key = CliStrings.START_LOCATOR__J, optionContext = GfshParser.J_OPTION_CONTEXT,
           help = CliStrings.START_LOCATOR__J__HELP) final String[] jvmArgsOpts,
       @CliOption(key = CliStrings.START_LOCATOR__CONNECT, unspecifiedDefaultValue = "true",
           specifiedDefaultValue = "true",
@@ -1300,7 +1301,7 @@ public class LauncherLifecycleCommands extends AbstractCommandsSupport
{
           help = CliStrings.START_SERVER__INCLUDE_SYSTEM_CLASSPATH__HELP) final Boolean includeSystemClasspath,
       @CliOption(key = CliStrings.START_SERVER__INITIAL_HEAP,
           help = CliStrings.START_SERVER__INITIAL_HEAP__HELP) final String initialHeap,
-      @CliOption(key = CliStrings.START_SERVER__J,
+      @CliOption(key = CliStrings.START_SERVER__J, optionContext = GfshParser.J_OPTION_CONTEXT,
           help = CliStrings.START_SERVER__J__HELP) final String[] jvmArgsOpts,
       @CliOption(key = CliStrings.START_SERVER__LOCATORS,
           optionContext = ConverterHint.LOCATOR_DISCOVERY_CONFIG,
@@ -1938,8 +1939,8 @@ public class LauncherLifecycleCommands extends AbstractCommandsSupport
{
       @CliOption(key = CliStrings.START_JCONSOLE__VERSION, specifiedDefaultValue = "true",
           unspecifiedDefaultValue = "false",
           help = CliStrings.START_JCONSOLE__VERSION__HELP) final boolean version,
-      @CliOption(key = CliStrings.START_JCONSOLE__J,
-          help = CliStrings.START_JCONSOLE__J__HELP) final List<String> jvmArgs) {
+      @CliOption(key = CliStrings.START_JCONSOLE__J, optionContext = GfshParser.J_OPTION_CONTEXT,
+          help = CliStrings.START_JCONSOLE__J__HELP) final String[] jvmArgs) {
     try {
       String[] jconsoleCommandLine =
           createJConsoleCommandLine(null, interval, notile, pluginpath, version, jvmArgs);
@@ -1994,7 +1995,7 @@ public class LauncherLifecycleCommands extends AbstractCommandsSupport
{
 
   protected String[] createJConsoleCommandLine(final String member, final int interval,
       final boolean notile, final String pluginpath, final boolean version,
-      final List<String> jvmArgs) {
+      final String[] jvmArgs) {
     List<String> commandLine = new ArrayList<>();
 
     commandLine.add(getJConsolePathname());
@@ -2100,8 +2101,9 @@ public class LauncherLifecycleCommands extends AbstractCommandsSupport
{
   @CliCommand(value = CliStrings.START_JVISUALVM, help = CliStrings.START_JVISUALVM__HELP)
   @CliMetaData(shellOnly = true, relatedTopic = {CliStrings.TOPIC_GEODE_MANAGER,
       CliStrings.TOPIC_GEODE_JMX, CliStrings.TOPIC_GEODE_M_AND_M})
-  public Result startJVisualVM(@CliOption(key = CliStrings.START_JCONSOLE__J,
-      help = CliStrings.START_JCONSOLE__J__HELP) final List<String> jvmArgs) {
+  public Result startJVisualVM(
+      @CliOption(key = CliStrings.START_JCONSOLE__J, optionContext = GfshParser.J_OPTION_CONTEXT,
+          help = CliStrings.START_JCONSOLE__J__HELP) final String[] jvmArgs) {
     try {
       String[] jvisualvmCommandLine = createJVisualVMCommandLine(jvmArgs);
 
@@ -2136,7 +2138,7 @@ public class LauncherLifecycleCommands extends AbstractCommandsSupport
{
     }
   }
 
-  protected String[] createJVisualVMCommandLine(final List<String> jvmArgs) {
+  protected String[] createJVisualVMCommandLine(final String[] jvmArgs) {
     List<String> commandLine = new ArrayList<>();
 
     commandLine.add(getJVisualVMPathname());

http://git-wip-us.apache.org/repos/asf/geode/blob/a2243f4f/geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserJUnitTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserJUnitTest.java
index 95d5292..a96c30e 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserJUnitTest.java
@@ -115,7 +115,8 @@ public class GfshParserJUnitTest {
         {"command", "--J", "-Dkey=value", "--option", "'test value'", "--J", "-Dkey2=value2"};
     Arrays.stream(strings).forEach(tokens::add);
     assertThat(GfshParser.getSimpleParserInputFromTokens(tokens))
-        .isEqualTo("command --J \"-Dkey=value,-Dkey2=value2\" --option 'test value'");
+        .isEqualTo("command --J \"-Dkey=value" + GfshParser.J_ARGUMENT_DELIMITER
+            + "-Dkey2=value2\" --option 'test value'");
   }
 
   @Test
@@ -140,6 +141,7 @@ public class GfshParserJUnitTest {
         {"command", "--option", "'test value'", "--J", "-Dkey=value", "--J", "-Dkey2=value2"};
     Arrays.stream(strings).forEach(tokens::add);
     assertThat(GfshParser.getSimpleParserInputFromTokens(tokens))
-        .isEqualTo("command --option 'test value' --J \"-Dkey=value,-Dkey2=value2\"");
+        .isEqualTo("command --option 'test value' --J \"-Dkey=value"
+            + GfshParser.J_ARGUMENT_DELIMITER + "-Dkey2=value2\"");
   }
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/a2243f4f/geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
index 4467792..ab6dc3d 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java
@@ -58,8 +58,8 @@ public class GfshParserParsingTest {
         GfshParser.convertToSimpleParserInput(buffer));
 
     buffer = "start locator --J=-Dgemfire.http-service-port=8080 --name=loc1 --J=-Ddummythinghere";
-    assertEquals(
-        "start locator --J \"-Dgemfire.http-service-port=8080,-Ddummythinghere\" --name loc1",
+    assertEquals("start locator --J \"-Dgemfire.http-service-port=8080"
+        + GfshParser.J_ARGUMENT_DELIMITER + "-Ddummythinghere\" --name loc1",
         GfshParser.convertToSimpleParserInput(buffer));
 
     buffer = "start locator --";
@@ -67,8 +67,8 @@ public class GfshParserParsingTest {
 
     buffer =
         "start locator --J=-Dgemfire.http-service-port=8080 --name=loc1 --J=-Ddummythinghere
--";
-    assertEquals(
-        "start locator --J \"-Dgemfire.http-service-port=8080,-Ddummythinghere\" --name loc1
--",
+    assertEquals("start locator --J \"-Dgemfire.http-service-port=8080"
+        + GfshParser.J_ARGUMENT_DELIMITER + "-Ddummythinghere\" --name loc1 --",
         GfshParser.convertToSimpleParserInput(buffer));
 
     buffer = "start server --name=name1 --locators=localhost --J=-Dfoo=bar";
@@ -77,6 +77,78 @@ public class GfshParserParsingTest {
   }
 
   @Test
+  public void testStartLocatorJOptionWithComma() throws Exception {
+    buffer =
+        "start locator --name=test --J='-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=30000'
--J=-Dfoo=bar";
+    GfshParseResult result = parser.parse(buffer);
+    assertThat(result).isNotNull();
+    Object[] arguments = result.getArguments();
+    // the 17th argument is the jvmarguments;
+    String[] jvmArgs = (String[]) arguments[17];
+    assertThat(jvmArgs).hasSize(2);
+
+    // make sure the resulting jvm arguments do not have quotes (either single or double)
around
+    // them.
+    assertThat(jvmArgs[0])
+        .isEqualTo("-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=30000");
+    assertThat(jvmArgs[1]).isEqualTo("-Dfoo=bar");
+  }
+
+  @Test
+  public void testStartServerJOptionWithComma() throws Exception {
+    buffer =
+        "start server --name=test --J='-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=30000'
--J='-Dfoo=bar'";
+    GfshParseResult result = parser.parse(buffer);
+    assertThat(result).isNotNull();
+    Object[] arguments = result.getArguments();
+    // the 18th argument is the jvmarguments;
+    String[] jvmArgs = (String[]) arguments[18];
+    assertThat(jvmArgs).hasSize(2);
+
+    // make sure the resulting jvm arguments do not have quotes (either single or double)
around
+    // them.
+    assertThat(jvmArgs[0])
+        .isEqualTo("-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=30000");
+    assertThat(jvmArgs[1]).isEqualTo("-Dfoo=bar");
+  }
+
+  @Test
+  public void testStartJConsoleJOptionWithComma() throws Exception {
+    buffer =
+        "start jconsole --J='-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=30000'
--J=-Dfoo=bar";
+    GfshParseResult result = parser.parse(buffer);
+    assertThat(result).isNotNull();
+    Object[] arguments = result.getArguments();
+    // the 4th argument is the jvmarguments;
+    String[] jvmArgs = (String[]) arguments[4];
+    assertThat(jvmArgs).hasSize(2);
+
+    // make sure the resulting jvm arguments do not have quotes (either single or double)
around
+    // them.
+    assertThat(jvmArgs[0])
+        .isEqualTo("-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=30000");
+    assertThat(jvmArgs[1]).isEqualTo("-Dfoo=bar");
+  }
+
+  @Test
+  public void testStartJvisulvmOptionWithComma() throws Exception {
+    buffer =
+        "start jvisualvm --J=\"-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=30000\"
--J=-Dfoo=bar";
+    GfshParseResult result = parser.parse(buffer);
+    assertThat(result).isNotNull();
+    Object[] arguments = result.getArguments();
+    // the 1st argument is the jvmarguments;
+    String[] jvmArgs = (String[]) arguments[0];
+    assertThat(jvmArgs).hasSize(2);
+
+    // make sure the resulting jvm arguments do not have quotes (either single or double)
around
+    // them.
+    assertThat(jvmArgs[0])
+        .isEqualTo("-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=30000");
+    assertThat(jvmArgs[1]).isEqualTo("-Dfoo=bar");
+  }
+
+  @Test
   public void testParseOptionStartsWithHyphenWithoutQuotes() throws Exception {
     String input =
         "rebalance --exclude-region=/GemfireDataCommandsDUnitTestRegion2 --simulate=true
--time-out=-1";


Mime
View raw message