OOZIE-3390 [Shell action] STDERR contains a bogus error message (kmarton, andras.piros)
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 @@
}
@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 @@
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 @@
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();
diff --git a/release-log.txt b/release-log.txt
index 7d0dfe7..d9ba02c 100644
--- a/release-log.txt
+++ b/release-log.txt
@@ -1,5 +1,6 @@
-- 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)
diff --git a/sharelib/oozie/pom.xml b/sharelib/oozie/pom.xml
index 70f488b..6856d70 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>
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.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 @@
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 @@
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 {
