You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by st...@apache.org on 2017/12/01 17:41:04 UTC

hive git commit: HIVE-18127: Do not strip '--' comments from shell commands issued from CliDriver (Andrew Sherman, reviewed by Sahil Takiar)

Repository: hive
Updated Branches:
  refs/heads/master f68ebdce7 -> 33d527f25


HIVE-18127: Do not strip '--' comments from shell commands issued from CliDriver (Andrew Sherman, reviewed by Sahil Takiar)


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

Branch: refs/heads/master
Commit: 33d527f257137c421e8d362c5cf15d8e8fe26599
Parents: f68ebdc
Author: Andrew Sherman <as...@cloudera.com>
Authored: Fri Dec 1 09:39:51 2017 -0800
Committer: Sahil Takiar <st...@cloudera.com>
Committed: Fri Dec 1 09:40:16 2017 -0800

----------------------------------------------------------------------
 .../apache/hive/beeline/cli/TestHiveCli.java    |  7 ++++
 .../org/apache/hadoop/hive/cli/CliDriver.java   |  4 +-
 .../hadoop/hive/cli/TestCliDriverMethods.java   | 43 ++++++++++++++++++++
 .../apache/hive/jdbc/TestJdbcWithMiniHS2.java   | 13 ++++++
 4 files changed, 65 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hive/blob/33d527f2/beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java
----------------------------------------------------------------------
diff --git a/beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java b/beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java
index 28b0cf9..068bb8d 100644
--- a/beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java
+++ b/beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java
@@ -112,6 +112,13 @@ public class TestHiveCli {
   }
 
   @Test
+  public void testCommentStripping() {
+    // this should work as comments are stripped by HiveCli
+    verifyCMD("!ls --abcdefghijklmnopqrstuvwxyz\n", "src", os, null, ERRNO_OK, true);
+  }
+
+
+  @Test
   public void testSetPromptValue() {
     verifyCMD("set hive.cli.prompt=MYCLI;SHOW\nTABLES;", "MYCLI> ", errS, null,
         ERRNO_OK, true);

http://git-wip-us.apache.org/repos/asf/hive/blob/33d527f2/cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java
----------------------------------------------------------------------
diff --git a/cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java b/cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java
index cf06582..bd0b422 100644
--- a/cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java
+++ b/cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java
@@ -158,8 +158,8 @@ public class CliDriver {
         }
       }
     } else if (cmd_trimmed.startsWith("!")) {
-
-      String shell_cmd = cmd_trimmed.substring(1);
+      // for shell commands, use unstripped command
+      String shell_cmd = cmd.trim().substring(1);
       shell_cmd = new VariableSubstitution(new HiveVariableSource() {
         @Override
         public Map<String, String> getHiveVariable() {

http://git-wip-us.apache.org/repos/asf/hive/blob/33d527f2/cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java
----------------------------------------------------------------------
diff --git a/cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java b/cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java
index bf23ba3..8f1c15e 100644
--- a/cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java
+++ b/cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java
@@ -104,6 +104,49 @@ public class TestCliDriverMethods extends TestCase {
     verify(mockOut, never()).print(anyString());
   }
 
+  // Test that CliDriver does not strip comments starting with '--'
+  public void testThatCliDriverDoesNotStripComments() throws Exception {
+    // We need to overwrite System.out and System.err as that is what is used in ShellCmdExecutor
+    // So save old values...
+    PrintStream oldOut = System.out;
+    PrintStream oldErr = System.err;
+
+    // Capture stdout and stderr
+    ByteArrayOutputStream dataOut = new ByteArrayOutputStream();
+    PrintStream out = new PrintStream(dataOut);
+    System.setOut(out);
+    ByteArrayOutputStream dataErr = new ByteArrayOutputStream();
+    PrintStream err = new PrintStream(dataErr);
+    System.setErr(err);
+
+    CliSessionState ss = new CliSessionState(new HiveConf());
+    ss.out = out;
+    ss.err = err;
+
+    // Save output as yo cannot print it while System.out and System.err are weird
+    String message;
+    String errors;
+    int ret;
+    try {
+      CliSessionState.start(ss);
+      CliDriver cliDriver = new CliDriver();
+      // issue a command with bad options
+      ret = cliDriver.processCmd("!ls --abcdefghijklmnopqrstuvwxyz123456789");
+    } finally {
+      // restore System.out and System.err
+      System.setOut(oldOut);
+      System.setErr(oldErr);
+    }
+    message = dataOut.toString("UTF-8");
+    errors = dataErr.toString("UTF-8");
+    assertTrue("Comments with '--; should not have been stripped,"
+        + " so command should fail", ret != 0);
+    assertTrue("Comments with '--; should not have been stripped,"
+        + " so we should have got an error in the output: '" + errors + "'.",
+        errors.contains("option"));
+    assertNotNull(message); // message kept around in for debugging
+  }
+
   /**
    * Do the actual testing against a mocked CliDriver based on what type of schema
    *

http://git-wip-us.apache.org/repos/asf/hive/blob/33d527f2/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java
----------------------------------------------------------------------
diff --git a/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java b/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java
index f5ed735..70bd29c 100644
--- a/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java
+++ b/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java
@@ -68,6 +68,7 @@ import org.apache.hadoop.hive.ql.exec.FunctionRegistry;
 import org.apache.hadoop.hive.ql.exec.UDF;
 import org.apache.hive.common.util.ReflectionUtil;
 import org.apache.hive.jdbc.miniHS2.MiniHS2;
+import org.apache.hive.service.cli.HiveSQLException;
 import org.datanucleus.ClassLoaderResolver;
 import org.datanucleus.NucleusContext;
 import org.datanucleus.api.jdo.JDOPersistenceManagerFactory;
@@ -587,6 +588,18 @@ public class TestJdbcWithMiniHS2 {
     stmt.close();
   }
 
+
+  // Test that jdbc does not allow shell commands starting with "!".
+  @Test
+  public void testBangCommand() throws Exception {
+    try (Statement stmt = conTestDb.createStatement()) {
+      stmt.execute("!ls --l");
+      fail("statement should fail, allowing this would be bad security");
+    } catch (HiveSQLException e) {
+      assertTrue(e.getMessage().contains("cannot recognize input near '!'"));
+    }
+  }
+
   @Test
   public void testJoinThriftSerializeInTasks() throws Exception {
     Statement stmt = conTestDb.createStatement();