commit bf3797fb89d2ecc189760615bb1b1324a686a987 Author: Andrew Sherman Date: Tue Nov 21 10:16:11 2017 -0800 HIVE-18127 Do not strip '--' comments from shell commands issued from CliDriver. CLiDriver has the ability to run shell commands by prefixing them with '!". This behavior is not widely used (there are only 3 examples in .q files). Since HIVE-16935 started stripping comments starting with '--', a shell command containing '--' will not work correctly. Fix this by using the unstripped command for shell commands. Note that it would be a security hole for HS2 to allow execution of arbitary shell commands from a client command. Add tests to nail down correct behavior with '--' comments: CliDriver should not strip strings starting with '--' in a shell command (FIXED in this change). HiveCli should strip '--' comments. A Jdbc program should allow commands starting with "!" but these will fail in the sql parser. diff --git beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java index 28b0cf9641c858dc438e78b3a3bf97479acf6411..068bb8dd91fe7324fb09d311f49d0ba86595dc63 100644 --- beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java +++ beeline/src/test/org/apache/hive/beeline/cli/TestHiveCli.java @@ -112,6 +112,13 @@ public void testCmd() { } @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); diff --git cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java index cf065821eef91fec3f5a7004f478a937d9209310..f142e10371258d4dd669d09446a3c12098334341 100644 --- cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java +++ cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java @@ -158,8 +158,8 @@ public int processCmd(String cmd) { } } } else if (cmd_trimmed.startsWith("!")) { - - String shell_cmd = cmd_trimmed.substring(1); + // for shell commands, use unstripped command + String shell_cmd = cmd.substring(1); shell_cmd = new VariableSubstitution(new HiveVariableSource() { @Override public Map getHiveVariable() { diff --git cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java index bf23ba39825803fee57490af28c08a6e7c0ca359..113ef30e6221ca14b88a35cc0c04ab08a606a90c 100644 --- cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java +++ cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java @@ -104,6 +104,41 @@ public void testThatCliDriverPrintsNoHeaderForCommandsWithNoSchema() 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; + String message; + String errors; + try { + CliSessionState.start(ss); + CliDriver cliDriver = new CliDriver(); + cliDriver.processCmd("!ls --abcdefghijklmnopqrstuvwxyz"); + } 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 we should have got " + + "an error in the output: '" + errors + "'.", errors.contains("illegal option")); + } + /** * Do the actual testing against a mocked CliDriver based on what type of schema * diff --git itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java index f5ed735c1ec14dfee338e56020fa2629b168389d..70bd29c5178456c683652cf2377206059b735514 100644 --- itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java +++ itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java @@ -68,6 +68,7 @@ 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 void testSelectThriftSerializeInTasks() throws Exception { 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();