Repository: metron
Updated Branches:
  refs/heads/master a9b01d1e3 -> bfc4786df


METRON-1013 StellarShell does not validate parameters (ottobackwards) closes 
apache/metron#639


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

Branch: refs/heads/master
Commit: bfc4786df55d522597d87c01872982bc9ec7923f
Parents: a9b01d1
Author: ottobackwards <[email protected]>
Authored: Wed Jul 12 10:04:20 2017 -0400
Committer: otto <[email protected]>
Committed: Wed Jul 12 10:04:20 2017 -0400

----------------------------------------------------------------------
 dependencies_with_url.csv                       |   1 +
 metron-stellar/stellar-common/README.md         |   2 +-
 metron-stellar/stellar-common/pom.xml           |   5 +
 .../stellar/common/shell/StellarShell.java      |   9 +-
 .../shell/StellarShellOptionsValidator.java     | 113 +++++++++++
 .../shell/StellarShellOptionsValidatorTest.java | 194 +++++++++++++++++++
 6 files changed, 322 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/metron/blob/bfc4786d/dependencies_with_url.csv
----------------------------------------------------------------------
diff --git a/dependencies_with_url.csv b/dependencies_with_url.csv
index 7c3bf1f..a8d78e5 100644
--- a/dependencies_with_url.csv
+++ b/dependencies_with_url.csv
@@ -175,6 +175,7 @@ 
commons-net:commons-net:jar:3.1:compile,ASLv2,http://commons.apache.org/net/
 commons-net:commons-net:jar:3.1:provided,ASLv2,http://commons.apache.org/net/
 
commons-validator:commons-validator:jar:1.4.0:compile,ASLv2,http://commons.apache.org/validator/
 
commons-validator:commons-validator:jar:1.5.1:compile,ASLv2,http://commons.apache.org/proper/commons-validator/
+commons-validator:commons-validator:jar:1.6:compile,ASLv2,http://commons.apache.org/proper/commons-validator/
 
io.confluent:kafka-avro-serializer:jar:1.0:compile,ASLv2,https://github.com/confluentinc/schema-registry/
 
io.confluent:kafka-schema-registry-client:jar:1.0:compile,ASLv2,https://github.com/confluentinc/schema-registry/
 io.netty:netty-all:jar:4.0.23.Final:compile,ASLv2,

http://git-wip-us.apache.org/repos/asf/metron/blob/bfc4786d/metron-stellar/stellar-common/README.md
----------------------------------------------------------------------
diff --git a/metron-stellar/stellar-common/README.md 
b/metron-stellar/stellar-common/README.md
index 8a2699d..7a4343a 100644
--- a/metron-stellar/stellar-common/README.md
+++ b/metron-stellar/stellar-common/README.md
@@ -843,7 +843,7 @@ usage: stellar
  -irc,--inputrc <arg>   File containing the inputrc if not the default
                         ~/.inputrc
  -v,--variables <arg>   File containing a JSON Map of variables
- -z,--zookeeper <arg>   Zookeeper URL
+ -z,--zookeeper <arg>   Zookeeper URL fragment in the form 
[HOSTNAME|IPADDRESS]:PORT
  -na,--no_ansi          Make the input prompt not use ANSI colors.
 ```
 

http://git-wip-us.apache.org/repos/asf/metron/blob/bfc4786d/metron-stellar/stellar-common/pom.xml
----------------------------------------------------------------------
diff --git a/metron-stellar/stellar-common/pom.xml 
b/metron-stellar/stellar-common/pom.xml
index 130ba75..11008b9 100644
--- a/metron-stellar/stellar-common/pom.xml
+++ b/metron-stellar/stellar-common/pom.xml
@@ -92,6 +92,11 @@
             <version>4.1</version>
         </dependency>
         <dependency>
+            <groupId>commons-validator</groupId>
+            <artifactId>commons-validator</artifactId>
+            <version>1.6</version>
+        </dependency>
+        <dependency>
             <groupId>org.apache.curator</groupId>
             <artifactId>curator-client</artifactId>
             <version>${global_curator_version}</version>

http://git-wip-us.apache.org/repos/asf/metron/blob/bfc4786d/metron-stellar/stellar-common/src/main/java/org/apache/metron/stellar/common/shell/StellarShell.java
----------------------------------------------------------------------
diff --git 
a/metron-stellar/stellar-common/src/main/java/org/apache/metron/stellar/common/shell/StellarShell.java
 
b/metron-stellar/stellar-common/src/main/java/org/apache/metron/stellar/common/shell/StellarShell.java
index 1b51415..b8b31a3 100644
--- 
a/metron-stellar/stellar-common/src/main/java/org/apache/metron/stellar/common/shell/StellarShell.java
+++ 
b/metron-stellar/stellar-common/src/main/java/org/apache/metron/stellar/common/shell/StellarShell.java
@@ -110,7 +110,7 @@ public class StellarShell extends AeshConsoleCallback 
implements Completion {
 
     // define valid command-line options
     Options options = new Options();
-    options.addOption("z", "zookeeper", true, "Zookeeper URL");
+    options.addOption("z", "zookeeper", true, "Zookeeper URL fragment in the 
form [HOSTNAME|IPADDRESS]:PORT");
     options.addOption("v", "variables", true, "File containing a JSON Map of 
variables");
     options.addOption("irc", "inputrc", true, "File containing the inputrc if 
not the default ~/.inputrc");
     options.addOption("na", "no_ansi", false, "Make the input prompt not use 
ANSI colors.");
@@ -127,6 +127,13 @@ public class StellarShell extends AeshConsoleCallback 
implements Completion {
       System.exit(0);
     }
 
+    try {
+      StellarShellOptionsValidator.validateOptions(commandLine);
+    }catch(IllegalArgumentException e){
+      System.out.println(e.getMessage());
+      System.exit(1);
+    }
+    
     console = createConsole(commandLine);
     executor = createExecutor(commandLine, console, 
getStellarProperties(commandLine));
     loadVariables(commandLine, executor);

http://git-wip-us.apache.org/repos/asf/metron/blob/bfc4786d/metron-stellar/stellar-common/src/main/java/org/apache/metron/stellar/common/shell/StellarShellOptionsValidator.java
----------------------------------------------------------------------
diff --git 
a/metron-stellar/stellar-common/src/main/java/org/apache/metron/stellar/common/shell/StellarShellOptionsValidator.java
 
b/metron-stellar/stellar-common/src/main/java/org/apache/metron/stellar/common/shell/StellarShellOptionsValidator.java
new file mode 100644
index 0000000..411c288
--- /dev/null
+++ 
b/metron-stellar/stellar-common/src/main/java/org/apache/metron/stellar/common/shell/StellarShellOptionsValidator.java
@@ -0,0 +1,113 @@
+/*
+ *
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ */
+
+package org.apache.metron.stellar.common.shell;
+
+import java.io.File;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.validator.routines.InetAddressValidator;
+
+public class StellarShellOptionsValidator {
+
+  private static final Pattern validPortPattern = 
Pattern.compile("(^.*)[:](\\d+)$");
+  private static final Pattern validHostNamePattern;
+
+  static {
+    validHostNamePattern = Pattern.compile(
+        "^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\\\\-]*[a-zA-Z0-9])\\\\.)"
+            + "*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\\\\-]*[A-Za-z0-9])$");
+  }
+
+  private static final InetAddressValidator inetAddressValidator = 
InetAddressValidator
+      .getInstance();
+
+  /**
+   * Validates Stellar CLI Options.
+   */
+  public static void validateOptions(CommandLine commandLine) throws 
IllegalArgumentException {
+    if (commandLine.hasOption('z')) {
+      validateZookeeperOption(commandLine.getOptionValue('z'));
+    }
+    // v, irc, p are files
+    if (commandLine.hasOption('v')) {
+      validateFileOption("v", commandLine.getOptionValue('v'));
+    }
+    if (commandLine.hasOption("irc")) {
+      validateFileOption("irc", commandLine.getOptionValue("irc"));
+    }
+    if (commandLine.hasOption('p')) {
+      validateFileOption("p", commandLine.getOptionValue('p'));
+    }
+
+  }
+
+  /**
+   * Zookeeper argument should be in the form [HOST|IP]:PORT.
+   *
+   * @param z the zookeeper url fragment
+   */
+  private static void validateZookeeperOption(String z) throws 
IllegalArgumentException {
+
+    Matcher matcher = validPortPattern.matcher(z);
+    if (!matcher.matches()) {
+      throw new IllegalArgumentException(String.format("Zookeeper option must 
have port: %s", z));
+    }
+
+    if (matcher.groupCount() != 2) {
+      throw new IllegalArgumentException(
+          String.format("Zookeeper Option must be in the form of 
[HOST|IP]:PORT  %s", z));
+    }
+    String name = matcher.group(1);
+    Integer port = Integer.parseInt(matcher.group(2));
+
+    if (!validHostNamePattern.matcher(name).matches() && 
!inetAddressValidator.isValid(name)) {
+      throw new IllegalArgumentException(
+          String.format("Zookeeper Option %s is not a valid host name or ip 
address  %s", name, z));
+    }
+
+    if(port == 0 || port > 65535){
+      throw new IllegalArgumentException(
+          String.format("Zookeeper Option %s port is not valid",z));
+    }
+  }
+
+  /**
+   * File options must exist and be readable.
+   *
+   * @param option name of the option
+   * @param fileName the file name
+   */
+  private static void validateFileOption(String option, String fileName)
+      throws IllegalArgumentException {
+    File file = new File(fileName);
+    if (!file.exists()) {
+      throw new IllegalArgumentException(
+          String.format("%s: File %s doesn't exist", option, fileName));
+    }
+    if (!file.canRead()) {
+      throw new IllegalArgumentException(
+          String.format("%s: File %s is not readable", option, fileName));
+    }
+  }
+}
+
+

http://git-wip-us.apache.org/repos/asf/metron/blob/bfc4786d/metron-stellar/stellar-common/src/test/java/org/apache/metron/stellar/common/shell/StellarShellOptionsValidatorTest.java
----------------------------------------------------------------------
diff --git 
a/metron-stellar/stellar-common/src/test/java/org/apache/metron/stellar/common/shell/StellarShellOptionsValidatorTest.java
 
b/metron-stellar/stellar-common/src/test/java/org/apache/metron/stellar/common/shell/StellarShellOptionsValidatorTest.java
new file mode 100644
index 0000000..8fb1873
--- /dev/null
+++ 
b/metron-stellar/stellar-common/src/test/java/org/apache/metron/stellar/common/shell/StellarShellOptionsValidatorTest.java
@@ -0,0 +1,194 @@
+/*
+ *
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ */
+
+package org.apache.metron.stellar.common.shell;
+
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.CommandLineParser;
+import org.apache.commons.cli.Options;
+import org.apache.commons.cli.PosixParser;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.io.File;
+
+import static org.junit.Assert.*;
+
+public class StellarShellOptionsValidatorTest {
+
+  @Test
+  public void validateOptions() throws Exception {
+    String[] validZHostArg = new String[]{"-z", "host1:8888"};
+    String[] validZIPArg = new String[]{"-z", "10.10.10.3:9999"};
+    String[] invalidZNoPortArg = new String[]{"-z", "host1"};
+    String[] invalidZIPNoPortArg = new String[]{"-z", "10.10.10.3"};
+    String[] invalidZNameArg = new String[]{"-z", "!!!@!!@!:8882"};
+    String[] invalidZIPArg = new String[]{"-z", "11111.22222.10.3:3332"};
+    String[] invalidZMissingNameArg = new String[]{"-z", ":8882"};
+    String[] invalidZZeroPortArg = new String[]{"-z", "host1:0"};
+    String[] invalidZHugePortArg = new String[]{"-z", "host1:75565"};
+
+
+    String existingFileName = "./target/existsFile";
+    String nonExistentFile = "./target/doesNotExist";
+
+    String[] validVFileArg = new String[]{"-v", existingFileName};
+    String[] validIrcFileArg = new String[]{"-irc", existingFileName};
+    String[] validPFileArg = new String[]{"-p", existingFileName};
+    String[] invalidVFileArg = new String[]{"-v", nonExistentFile};
+    String[] invalidIrcFileArg = new String[]{"-irc", nonExistentFile};
+    String[] invalidPFileArg = new String[]{"-p", nonExistentFile};
+
+    File existingFile = new File(existingFileName);
+    if (!existingFile.exists()) {
+      existingFile.createNewFile();
+    }
+    Options options = new Options();
+    options.addOption("z", "zookeeper", true, "Zookeeper URL");
+    options.addOption("v", "variables", true, "File containing a JSON Map of 
variables");
+    options.addOption("irc", "inputrc", true,
+        "File containing the inputrc if not the default ~/.inputrc");
+    options.addOption("na", "no_ansi", false, "Make the input prompt not use 
ANSI colors.");
+    options.addOption("h", "help", false, "Print help");
+    options.addOption("p", "properties", true, "File containing Stellar 
properties");
+
+    CommandLineParser parser = new PosixParser();
+
+    // these should work
+    CommandLine commandLine = parser.parse(options, validZHostArg);
+    StellarShellOptionsValidator.validateOptions(commandLine);
+
+    commandLine = parser.parse(options, validZIPArg);
+    StellarShellOptionsValidator.validateOptions(commandLine);
+
+    commandLine = parser.parse(options, validVFileArg);
+    StellarShellOptionsValidator.validateOptions(commandLine);
+
+    commandLine = parser.parse(options, validIrcFileArg);
+    StellarShellOptionsValidator.validateOptions(commandLine);
+
+    commandLine = parser.parse(options, validPFileArg);
+    StellarShellOptionsValidator.validateOptions(commandLine);
+
+    // these should not
+
+    boolean caught = false;
+
+    try {
+      commandLine = parser.parse(options, invalidZNoPortArg);
+      StellarShellOptionsValidator.validateOptions(commandLine);
+    } catch (IllegalArgumentException e) {
+      caught = true;
+    }
+    Assert.assertTrue("Did not catch failure for not providing port with host 
name ", caught);
+    caught = false;
+
+    try {
+      commandLine = parser.parse(options, invalidZIPNoPortArg);
+      StellarShellOptionsValidator.validateOptions(commandLine);
+    } catch (IllegalArgumentException e) {
+      caught = true;
+    }
+    Assert.assertTrue("Did not catch failure for not providing port with ip 
address ", caught);
+    caught = false;
+
+    try {
+      commandLine = parser.parse(options, invalidZNameArg);
+      StellarShellOptionsValidator.validateOptions(commandLine);
+    } catch (IllegalArgumentException e) {
+      caught = true;
+    }
+    Assert.assertTrue("Did not catch failure for providing invalid host name 
", caught);
+    caught = false;
+
+    try {
+      commandLine = parser.parse(options, invalidZIPArg);
+      StellarShellOptionsValidator.validateOptions(commandLine);
+    } catch (IllegalArgumentException e) {
+      caught = true;
+    }
+    Assert.assertTrue("Did not catch failure for providing invalid ip address 
", caught);
+    caught = false;
+
+    try {
+      commandLine = parser.parse(options, invalidZMissingNameArg);
+      StellarShellOptionsValidator.validateOptions(commandLine);
+    } catch (IllegalArgumentException e) {
+      caught = true;
+    }
+    Assert.assertTrue("Did not catch failure for only providing port ", 
caught);
+    caught = false;
+
+    try {
+      commandLine = parser.parse(options, invalidZZeroPortArg);
+      StellarShellOptionsValidator.validateOptions(commandLine);
+    } catch (IllegalArgumentException e) {
+      caught = true;
+    }
+    Assert.assertTrue("Did not catch failure for 0 port ", caught);
+    caught = false;
+
+    try {
+      commandLine = parser.parse(options, invalidZHugePortArg);
+      StellarShellOptionsValidator.validateOptions(commandLine);
+    } catch (IllegalArgumentException e) {
+      caught = true;
+    }
+    Assert.assertTrue("Did not catch failure for port out of range ", caught);
+    caught = false;
+
+    try {
+      commandLine = parser.parse(options, invalidVFileArg);
+      StellarShellOptionsValidator.validateOptions(commandLine);
+    } catch (IllegalArgumentException e) {
+      caught = true;
+    }
+    Assert.assertTrue("Did not catch failure for passing non-existant file to 
-v ", caught);
+    caught = false;
+
+    try {
+      commandLine = parser.parse(options, invalidVFileArg);
+      StellarShellOptionsValidator.validateOptions(commandLine);
+    } catch (IllegalArgumentException e) {
+      caught = true;
+    }
+    Assert.assertTrue("Did not catch failure for passing non-existant file to 
-v ", caught);
+    caught = false;
+
+    try {
+      commandLine = parser.parse(options, invalidIrcFileArg);
+      StellarShellOptionsValidator.validateOptions(commandLine);
+    } catch (IllegalArgumentException e) {
+      caught = true;
+    }
+    Assert.assertTrue("Did not catch failure for passing non-existant file to 
-irc ", caught);
+    caught = false;
+
+    try {
+      commandLine = parser.parse(options, invalidPFileArg);
+      StellarShellOptionsValidator.validateOptions(commandLine);
+    } catch (IllegalArgumentException e) {
+      caught = true;
+    }
+    Assert.assertTrue("Did not catch failure for passing non-existant file to 
-p ", caught);
+    caught = false;
+  }
+
+}
\ No newline at end of file

Reply via email to