From 0fd2b457400fc7a67dd5737393bfcfd10c68fab7 Mon Sep 17 00:00:00 2001
From: Mahmood Ali <mahmood@notnoop.com>
Date: Tue, 12 Feb 2019 13:41:26 -0500
Subject: [PATCH] drivers/java: restore 0.8.7 java version detection (#5317)

Restore 0.8.x behavior where java driver is marked as detected when
`java -version` exits with 0 but returns unexpected output.

Furthermore, we restore behavior when `java -version` where we parse the
first three lines of `java -version` but ignore rest.

If `java -version` returns less than 3 lines, Nomad 0.8.7 would panic.
In this implementation, we'd still mark java as detected but returns
empty version.

The 0.8.7 logic for detecting java version is found in
https://github.com/hashicorp/nomad/blob/v0.8.7/client/driver/java.go#L132-L172
.

I punt on revamping how we can be more resilient to java -version
syntax, and aimed for preserving existing behavior instead.
---
 drivers/java/utils.go      | 12 +++++---
 drivers/java/utils_test.go | 62 ++++++++++++++++++++++++++++++++++++--
 2 files changed, 67 insertions(+), 7 deletions(-)

diff --git a/drivers/java/utils.go b/drivers/java/utils.go
index 35bfa73c3..b41eec691 100644
--- a/drivers/java/utils.go
+++ b/drivers/java/utils.go
@@ -18,14 +18,15 @@ func javaVersionInfo() (version, runtime, vm string, err error) {
 	cmd.Stderr = &out
 	err = cmd.Run()
 	if err != nil {
+		err = fmt.Errorf("failed to check java version: %v", err)
 		return
 	}
 
-	version, runtime, vm, err = parseJavaVersionOutput(out.String())
+	version, runtime, vm = parseJavaVersionOutput(out.String())
 	return
 }
 
-func parseJavaVersionOutput(infoString string) (version, runtime, vm string, err error) {
+func parseJavaVersionOutput(infoString string) (version, runtime, vm string) {
 	infoString = strings.TrimSpace(infoString)
 
 	lines := strings.Split(infoString, "\n")
@@ -33,8 +34,9 @@ func parseJavaVersionOutput(infoString string) (version, runtime, vm string, err
 		lines = lines[1:]
 	}
 
-	if len(lines) != 3 {
-		return "", "", "", fmt.Errorf("unexpected java version info output, expected 3 lines but got: %v", infoString)
+	if len(lines) < 3 {
+		// unexpected output format, don't attempt to parse output for version
+		return "", "", ""
 	}
 
 	versionString := strings.TrimSpace(lines[0])
@@ -44,5 +46,5 @@ func parseJavaVersionOutput(infoString string) (version, runtime, vm string, err
 		versionString = match[1]
 	}
 
-	return versionString, strings.TrimSpace(lines[1]), strings.TrimSpace(lines[2]), nil
+	return versionString, strings.TrimSpace(lines[1]), strings.TrimSpace(lines[2])
 }
diff --git a/drivers/java/utils_test.go b/drivers/java/utils_test.go
index f29d07474..d09884ed4 100644
--- a/drivers/java/utils_test.go
+++ b/drivers/java/utils_test.go
@@ -56,12 +56,24 @@ func TestDriver_parseJavaVersionOutput(t *testing.T) {
 			"OpenJDK Runtime Environment (IcedTea6 1.13.8) (6b36-1.13.8-0ubuntu1~12.04)",
 			"OpenJDK 64-Bit Server VM (build 23.25-b01, mixed mode)",
 		},
+		{
+			"Eclipse OpenJ9",
+			`openjdk version "1.8.0_192"
+			OpenJDK Runtime Environment (build 1.8.0_192-b12_openj9)
+			Eclipse OpenJ9 VM (build openj9-0.11.0, JRE 1.8.0 Linux amd64-64-Bit Compressed References
+			20181107_95 (JIT enabled, AOT enabled)
+			OpenJ9 - 090ff9dcd
+			OMR - ea548a66
+			JCL - b5a3affe73 based on jdk8u192-b12)`,
+			"1.8.0_192",
+			"OpenJDK Runtime Environment (build 1.8.0_192-b12_openj9)",
+			"Eclipse OpenJ9 VM (build openj9-0.11.0, JRE 1.8.0 Linux amd64-64-Bit Compressed References",
+		},
 	}
 
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
-			version, runtime, vm, err := parseJavaVersionOutput(c.output)
-			require.NoError(t, err)
+			version, runtime, vm := parseJavaVersionOutput(c.output)
 
 			require.Equal(t, c.version, version)
 			require.Equal(t, c.runtime, runtime)
@@ -92,3 +104,49 @@ func TestDriver_javaVersionInfo(t *testing.T) {
 	require.Equal(t, "Java HotSpot(TM) 64-Bit Server VM (build 24.80-b11, mixed mode)", vm)
 
 }
+
+func TestDriver_javaVersionInfo_UnexpectedOutput(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("test requires bash to run")
+	}
+
+	initCmd := javaVersionCommand
+	defer func() {
+		javaVersionCommand = initCmd
+	}()
+
+	javaVersionCommand = []string{
+		"/bin/sh", "-c",
+		fmt.Sprintf("printf '%%s\n' '%s' >/dev/stderr", "unexpected java -version output"),
+	}
+
+	version, runtime, vm, err := javaVersionInfo()
+	require.NoError(t, err)
+	require.Equal(t, "", version)
+	require.Equal(t, "", runtime)
+	require.Equal(t, "", vm)
+}
+
+func TestDriver_javaVersionInfo_JavaVersionFails(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("test requires bash to run")
+	}
+
+	initCmd := javaVersionCommand
+	defer func() {
+		javaVersionCommand = initCmd
+	}()
+
+	javaVersionCommand = []string{
+		"/bin/sh", "-c",
+		"exit 127",
+	}
+
+	version, runtime, vm, err := javaVersionInfo()
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "failed to check java version")
+
+	require.Equal(t, "", version)
+	require.Equal(t, "", runtime)
+	require.Equal(t, "", vm)
+}