Repository: oozie Updated Branches: refs/heads/master f09b11b99 -> 471659fa0
OOZIE-3390 [Shell action] STDERR contains a bogus error message (kmarton, andras.piros) Project: http://git-wip-us.apache.org/repos/asf/oozie/repo Commit: http://git-wip-us.apache.org/repos/asf/oozie/commit/471659fa Tree: http://git-wip-us.apache.org/repos/asf/oozie/tree/471659fa Diff: http://git-wip-us.apache.org/repos/asf/oozie/diff/471659fa Branch: refs/heads/master Commit: 471659fa0ddded743bdd84a80c543f05da77c1ee Parents: f09b11b Author: Andras Piros <[email protected]> Authored: Mon Dec 3 14:04:16 2018 +0100 Committer: Andras Piros <[email protected]> Committed: Mon Dec 3 14:04:16 2018 +0100 ---------------------------------------------------------------------- .../action/hadoop/TestShellContentWriter.java | 28 +++++++++++++++-- release-log.txt | 3 +- sharelib/oozie/pom.xml | 5 +++ .../oozie/action/hadoop/ShellContentWriter.java | 32 ++++++++++++++++++-- 4 files changed, 62 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/oozie/blob/471659fa/core/src/test/java/org/apache/oozie/action/hadoop/TestShellContentWriter.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/oozie/action/hadoop/TestShellContentWriter.java b/core/src/test/java/org/apache/oozie/action/hadoop/TestShellContentWriter.java index 75a98cb..de2141a 100644 --- a/core/src/test/java/org/apache/oozie/action/hadoop/TestShellContentWriter.java +++ b/core/src/test/java/org/apache/oozie/action/hadoop/TestShellContentWriter.java @@ -65,6 +65,25 @@ public class TestShellContentWriter { } @Test + public void testPrintShellValidShellCommand() { + callPrint(1, "echo"); + + Assert.assertTrue(String.format("output stream must be empty but is [%s]", outputStream.toString()), + outputStream.toString().isEmpty()); + Assert.assertTrue(String.format("error stream must be empty but is [%s]", errorStream.toString()), + errorStream.toString().isEmpty()); + } + + @Test + public void testPrintShellInvalidShellCommand() { + callPrint(1, "invalid command"); + + Assert.assertTrue(String.format("output stream must be empty but is [%s]", outputStream.toString()), + outputStream.toString().isEmpty()); + Assert.assertTrue(String.format("invalid error stream message [%s]", errorStream.toString()), + errorStream.toString().contains("doesn't appear to exist")); + } + @Test public void testPrintControlCharacter() throws Exception { writeScript("echo Hello World\011"); @@ -107,7 +126,8 @@ public class TestShellContentWriter { writeScript(""); Assert.assertTrue(outputStream.toString().isEmpty()); - Assert.assertTrue(errorStream.toString().contains("doesn't appear to exist")); + Assert.assertTrue(String.format("invalid error stream message [%s]", errorStream.toString()), + errorStream.toString().contains("doesn't appear to exist")); } private void writeScript(String content) throws IOException { @@ -120,11 +140,15 @@ public class TestShellContentWriter { Files.write(scriptFile.toPath(), content.getBytes()); } + callPrint(maxLen, scriptFile.getAbsolutePath()); + } + + private void callPrint(final int maxLen, final String filename) { ShellContentWriter writer = new ShellContentWriter( maxLen, outputStream, errorStream, - scriptFile.getAbsolutePath() + filename ); writer.print(); http://git-wip-us.apache.org/repos/asf/oozie/blob/471659fa/release-log.txt ---------------------------------------------------------------------- diff --git a/release-log.txt b/release-log.txt index 5b8f0fc..eb00eb4 100644 --- a/release-log.txt +++ b/release-log.txt @@ -1,9 +1,7 @@ -- Oozie 5.2.0 release (trunk - unreleased) -OOZIE-3389 Getting input dependency list on the UI throws NPE (andras.piros via asalamon74, kmarton) OOZIE-3382 [SSH action] [SSH action] Optimize process streams draining (asalamon74 via andras.piros) OOZIE-3384 [tests] TestWorkflowActionRetryInfoXCommand#testRetryConsoleUrlForked() is flaky (asalamon74 via kmarton) -OOZIE-3386 Misleading error message when workflow application does not exist (kmarton) OOZIE-3120 Upgrade maven-assembly plugin to v.3.1.0 (dionusos via kmarton) OOZIE-3381 [coordinator] Enhance logging of CoordElFunctions (andras.piros via kmarton) OOZIE-3380 TestCoordMaterializeTransitionXCommand failure after DST change date (asalamon74 via kmarton) @@ -18,6 +16,7 @@ OOZIE-3277 [build] Check for star imports (kmarton via andras.piros) -- Oozie 5.1.0 release +OOZIE-3390 [Shell action] STDERR contains a bogus error message (kmarton, andras.piros) OOZIE-3389 Getting input dependency list on the UI throws NPE (andras.piros via asalamon74, kmarton) OOZIE-3386 Misleading error message when workflow application does not exist (kmarton) OOZIE-3377 [docs] Remaining 5.1.0 documentation changes (andras.piros) http://git-wip-us.apache.org/repos/asf/oozie/blob/471659fa/sharelib/oozie/pom.xml ---------------------------------------------------------------------- diff --git a/sharelib/oozie/pom.xml b/sharelib/oozie/pom.xml index 6883729..e522e9a 100644 --- a/sharelib/oozie/pom.xml +++ b/sharelib/oozie/pom.xml @@ -72,6 +72,11 @@ <artifactId>jackson-databind</artifactId> <scope>test</scope> </dependency> + + <dependency> + <groupId>com.google.code.findbugs</groupId> + <artifactId>annotations</artifactId> + </dependency> </dependencies> <build> http://git-wip-us.apache.org/repos/asf/oozie/blob/471659fa/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/ShellContentWriter.java ---------------------------------------------------------------------- diff --git a/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/ShellContentWriter.java b/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/ShellContentWriter.java index fe833ac..0cef202 100644 --- a/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/ShellContentWriter.java +++ b/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/ShellContentWriter.java @@ -18,6 +18,7 @@ package org.apache.oozie.action.hadoop; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.apache.commons.io.Charsets; import java.io.BufferedInputStream; @@ -29,6 +30,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.attribute.BasicFileAttributes; +import java.util.concurrent.TimeUnit; /** * Dump the content of a shell script to output stream. @@ -67,7 +69,7 @@ public class ShellContentWriter { Path path = Paths.get(filename); try { - if (Files.exists(path)) { + if (isValidScript(path)) { BasicFileAttributes attributes = Files.readAttributes(path, BasicFileAttributes.class); long len = attributes.size(); if (len < maxLen) { @@ -88,11 +90,37 @@ public class ShellContentWriter { writeLine(errorStream, message); } } else { - writeLine(errorStream, "Path " + filename + " doesn't appear to exist"); + writeLine(errorStream, "Path " + filename + " doesn't appear to exist, not printing its content."); } } catch (IOException ignored) { } } + @SuppressFBWarnings(value = {"COMMAND_INJECTION", "PATH_TRAVERSAL_OUT"}, + justification = "pathToScript is specified in the WF action. It will surely be executed later on, no need to filter") + private boolean isValidScript(final Path pathToScript) { + if (Files.exists(pathToScript)) { + return true; + } + + try { + // command -v is not present on every tested platform, using which instead + final Process presentOnPathProcess = new ProcessBuilder() + .command("which", pathToScript.toString()) + .start(); + + final boolean hasFinished = presentOnPathProcess.waitFor(1, TimeUnit.SECONDS); + if (!hasFinished) { + return false; + } + + final int exitValue = presentOnPathProcess.exitValue(); + + return exitValue == 0; + } catch (final IOException | InterruptedException | IllegalThreadStateException e) { + return false; + } + } + // Read the file into the given buffer. // Return true if the file appears to be printable, false otherwise. private boolean readFile(Path path, byte[] buffer) throws IOException {
