Fix the JvmOptions parsing of procfs cmdline

Author: jbachorik

components/environment/src/main/java/datadog/environment/JvmOptions.java

@@ -55,20 +55,29 @@ List<String> findVmOptions(String[] procfsCmdline) {
       List<String> vmOptions = new ArrayList<>();
       // Start at 1 to skip "java" command itself
       int index = 1;
-      // Look for main class or "-jar", end of VM options
+      // Look for first self-standing argument that is not prefixed with "-" or end of VM options
+      // Skip "-jar" and the jar file
       // Simultaneously, collect all arguments in the VM options
       for (; index < procfsCmdline.length; index++) {
         String argument = procfsCmdline[index];
         if (argument.startsWith("@")) {
           vmOptions.addAll(getArgumentsFromFile(argument));
         } else {
-          vmOptions.add(argument);
-        }
-        if (!argument.startsWith("-") || "-jar".equals(argument)) {
-          if (index + 1 < procfsCmdline.length) {
-            vmOptions.add(procfsCmdline[index + 1]); // jar file or the main class
+          if ("-jar".equals(argument)) {
+            // skip "-jar" and the jar file
+            index++;
+            continue;
+          } else if ("-cp".equals(argument)) {
+            // slurp '-cp' and the classpath
+            vmOptions.add(argument);
+            if (index + 1 < procfsCmdline.length) {
+              argument = procfsCmdline[++index];
+            }
+          } else if (!argument.startsWith("-")) {
+            // end of VM options
+            break;
           }
-          break;
+          vmOptions.add(argument);
         }
       }
       // Insert JDK_JAVA_OPTIONS at the start if present and supported

components/environment/src/test/java/datadog/environment/JvmOptionsTest.java

@@ -8,15 +8,23 @@
 import static java.util.Collections.emptyList;
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonList;
+import static java.util.Objects.requireNonNull;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assumptions.assumeTrue;
 import static org.junit.jupiter.params.provider.Arguments.arguments;
 
 import datadog.environment.CommandLineHelper.Result;
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Stream;
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
@@ -122,12 +130,37 @@ private static Stream<Arguments> procFsCmdLine() {
         arguments(
             "Java with JAR and options",
             new String[]{"java", "-Xmx512m", "-Xms256m", "-jar", "app.jar"},
-            asList("-Xmx512m", "-Xms256m", "-jar", "app.jar")
+            asList("-Xmx512m", "-Xms256m")
         ),
         arguments(
             "Java from class and options",
             new String[]{"java", "-Xmx512m", "-Xms256m", "-cp", "app.jar", "Main"},
-            asList("-Xmx512m", "-Xms256m", "-cp", "app.jar", "Main")
+            asList("-Xmx512m", "-Xms256m", "-cp", "app.jar")
+        ),
+        arguments(
+            "Java from class and options, mixed",
+            new String[]{"java", "-Xms256m", "-cp", "app.jar", "-Xmx512m", "Main"},
+            asList("-Xms256m", "-cp", "app.jar", "-Xmx512m")
+        ),
+        arguments(
+            "Args from file",
+            new String[]{"java", "-Dargfile.prop=test", "-verbose:class", argFile("carriage-return-separated"), "-jar", "app.jar"},
+            flatten("-Dargfile.prop=test", "-verbose:class", expectedArsFromArgFile("carriage-return-separated"))
+        ),
+        arguments(
+            "Args from file",
+            new String[]{"java", "-Dargfile.prop=test", "-verbose:class", argFile("new-line-separated"), "-jar", "app.jar"},
+            flatten("-Dargfile.prop=test", "-verbose:class", expectedArsFromArgFile("new-line-separated"))
+        ),
+        arguments(
+            "Args from file",
+            new String[]{"java", "-Dargfile.prop=test", "-verbose:class", argFile("space-separated"), "-jar", "app.jar"},
+            flatten("-Dargfile.prop=test", "-verbose:class", expectedArsFromArgFile("space-separated"))
+        ),
+        arguments(
+            "Args from file",
+            new String[]{"java", "-Dargfile.prop=test", "-verbose:class", argFile("tab-separated"), "-jar", "app.jar"},
+            flatten("-Dargfile.prop=test", "-verbose:class", expectedArsFromArgFile("tab-separated"))
         ));
     // spotless:on
   }
@@ -157,4 +190,36 @@ private static Map<String, String> env(String... keysAndValues) {
     }
     return env;
   }
+
+  private static String argFile(String name) {
+    return "@src/test/resources/argfiles/" + name + ".txt";
+  }
+
+  private static List<String> expectedArsFromArgFile(String name) {
+    List<String> arguments = new ArrayList<>();
+    try (InputStream stream =
+            requireNonNull(
+                CommandLineTest.class.getResourceAsStream("/argfiles/" + name + "-expected.txt"));
+        BufferedReader reader = new BufferedReader(new InputStreamReader(stream))) {
+      String line;
+      while ((line = reader.readLine()) != null) {
+        arguments.add(line);
+      }
+    } catch (IOException e) {
+      Assertions.fail("Failed to read expected args from " + name + "argfile", e);
+    }
+    return arguments;
+  }
+
+  private static List<String> flatten(Object... values) {
+    List<String> result = new ArrayList<>();
+    for (Object value : values) {
+      if (value instanceof Collection) {
+        result.addAll((Collection<? extends String>) value);
+      } else {
+        result.add(value.toString());
+      }
+    }
+    return result;
+  }
 }