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 <asher...@cloudera.com> Authored: Fri Dec 1 09:39:51 2017 -0800 Committer: Sahil Takiar <stak...@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();