Issue #5681 - clearer warning on JVM Arg / System Property use (#7769)

- Improved warning on unrecognized Command Line Options Signed-off-by: Joakim Erdfelt <[email protected]>
jetty · Mar 25, 2022 · 96af5d1 · 96af5d1
1 parent 5117a58
commit 96af5d1
Show file tree

Hide file tree

Showing 5 changed files with 98 additions and 22 deletions.
diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java b/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java
@@ -501,7 +501,11 @@ public void run()
 
         if (args.hasJvmArgs() || args.hasSystemProperties())
         {
-            StartLog.warn("System properties and/or JVM args set.  Consider using --dry-run or --exec");
+            StartLog.warn("Unknown Arguments detected.  Consider using --dry-run or --exec");
+            if (args.hasSystemProperties())
+                args.getSystemProperties().forEach((k, v) -> StartLog.warn("  Argument: -D%s=%s (interpreted as a System property, from %s)", k, System.getProperty(k), v));
+            if (args.hasJvmArgs())
+                args.getJvmArgSources().forEach((jvmArg, source) -> StartLog.warn("  Argument: %s (interpreted as a JVM argument, from %s)", jvmArg, source));
         }
 
         ClassLoader cl = classpath.getClassLoader();

diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java b/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java
@@ -149,7 +149,7 @@ public class StartArgs
     /**
      * JVM arguments, found via command line and in all active [exec] sections from enabled modules
      */
-    private final List<String> jvmArgs = new ArrayList<>();
+    private final Map<String, String> jvmArgSources = new LinkedHashMap<>();
 
     /**
      * List of all xml references found directly on command line or start.ini
@@ -331,21 +331,22 @@ public void dumpEnvironment(PrintStream out)
 
     public void dumpJvmArgs(PrintStream out)
     {
-        if (jvmArgs.isEmpty())
+        if (jvmArgSources.isEmpty())
             return;
 
         out.println();
         out.println("Forked JVM Arguments:");
         out.println("---------------------");
 
-        for (String jvmArgKey : jvmArgs)
+        jvmArgSources.forEach((key, sourceRef) ->
         {
-            String value = System.getProperty(jvmArgKey);
+            String value = System.getProperty(key);
+            String source = StartLog.isDebugEnabled() ? '(' + sourceRef + ')' : "";
             if (value != null)
-                out.printf(" %s = %s%n", jvmArgKey, value);
+                out.printf(" %s = %s %s%n", key, value, source);
             else
-                out.printf(" %s%n", jvmArgKey);
-        }
+                out.printf(" %s %s%n", key, source);
+        });
     }
 
     public void dumpProperties(PrintStream out)
@@ -544,7 +545,7 @@ public void expandModules(List<Module> activeModules) throws IOException
             for (String jvmArg : module.getJvmArgs())
             {
                 exec = true;
-                jvmArgs.add(jvmArg);
+                jvmArgSources.put(jvmArg, String.format("module[%s|jvm]", module.getName()));
             }
 
             // Find and Expand XML files
@@ -655,9 +656,25 @@ public List<FileArg> getFiles()
         return files;
     }
 
+    /**
+     * Gets the List of JVM arguments detected.
+     *
+     * @deprecated use {@link #getJvmArgSources()} instead, as it will return source references with each arg.
+     */
+    @Deprecated
     public List<String> getJvmArgs()
     {
-        return jvmArgs;
+        return new ArrayList<>(jvmArgSources.keySet());
+    }
+
+    /**
+     * Return ordered Map of JVM arguments to Source (locations)
+     *
+     * @return the ordered map of JVM Argument to Source (locations)
+     */
+    public Map<String, String> getJvmArgSources()
+    {
+        return jvmArgSources;
     }
 
     public CommandLineBuilder getMainArgs(Set<String> parts) throws IOException
@@ -681,7 +698,7 @@ public CommandLineBuilder getMainArgs(Set<String> parts) throws IOException
             cmd.addRawArg("-Djetty.home=" + baseHome.getHome());
             cmd.addRawArg("-Djetty.base=" + baseHome.getBase());
 
-            for (String x : getJvmArgs())
+            for (String x : getJvmArgSources().keySet())
             {
                 if (x.startsWith("-D"))
                 {
@@ -816,7 +833,10 @@ else if (properties.size() > 0)
     public String getMainClassname()
     {
         String mainClass = System.getProperty("jetty.server", isJPMS() ? MODULE_MAIN_CLASS : MAIN_CLASS);
-        return System.getProperty("main.class", mainClass);
+        Prop mainClassProp = properties.getProp("main.class", true);
+        if (mainClassProp != null)
+            return mainClassProp.value;
+        return mainClass;
     }
 
     public String getMavenLocalRepoDir()
@@ -878,6 +898,11 @@ public Props getProperties()
         return properties;
     }
 
+    public Map<String, String> getSystemProperties()
+    {
+        return systemPropertySource;
+    }
+
     public Set<String> getSkipFileValidationModules()
     {
         return skipFileValidationModules;
@@ -895,7 +920,7 @@ public List<Path> getXmlFiles()
 
     public boolean hasJvmArgs()
     {
-        return !jvmArgs.isEmpty();
+        return !jvmArgSources.isEmpty();
     }
 
     public boolean hasSystemProperties()
@@ -1331,11 +1356,9 @@ public void parse(final String rawarg, String source)
         // Anything else with a "-" is considered a JVM argument
         if (arg.startsWith("-"))
         {
-            // Only add non-duplicates
-            if (!jvmArgs.contains(arg))
-            {
-                jvmArgs.add(arg);
-            }
+            StartLog.debug("Unrecognized Arg (possible JVM Arg): %s (from %s)", arg, source);
+            // always use the latest source (overriding any past tracked source)
+            jvmArgSources.put(arg, source);
             return;
         }
 
@@ -1555,6 +1578,6 @@ public void setRun(boolean run)
     public String toString()
     {
         return String.format("%s[enabledModules=%s, xmlRefs=%s, properties=%s, jvmArgs=%s]",
-            getClass().getSimpleName(), modules, xmlRefs, properties, jvmArgs);
+            getClass().getSimpleName(), modules, xmlRefs, properties, jvmArgSources.keySet());
     }
 }
diff --git a/jetty-start/src/test/java/org/eclipse/jetty/start/MainTest.java b/jetty-start/src/test/java/org/eclipse/jetty/start/MainTest.java
@@ -17,9 +17,12 @@
 import java.io.File;
 import java.io.PrintStream;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
+import java.util.Iterator;
 import java.util.List;
+import java.util.stream.Collectors;
 
 import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
 import org.junit.jupiter.api.BeforeEach;
@@ -91,6 +94,54 @@ public void testListConfig() throws Exception
         assertThat("user.dir should indicate that it was specified on the command line", userDirLine, containsString("(<command-line>)"));
     }
 
+    @Test
+    public void testUnknownDistroCommand() throws Exception
+    {
+        List<String> cmdLineArgs = new ArrayList<>();
+        File testJettyHome = MavenTestingUtils.getTestResourceDir("dist-home");
+        Path testJettyBase = MavenTestingUtils.getTargetTestingPath("base-example-unknown");
+        FS.ensureDirectoryExists(testJettyBase);
+        Path zedIni = testJettyBase.resolve("start.d/zed.ini");
+        FS.ensureDirectoryExists(zedIni.getParent());
+        Files.writeString(zedIni, "--zed-0-zed");
+        cmdLineArgs.add("jetty.home=" + testJettyHome);
+        cmdLineArgs.add("jetty.base=" + testJettyBase);
+        cmdLineArgs.add("main.class=" + PropertyDump.class.getName());
+        cmdLineArgs.add("--module=base");
+        cmdLineArgs.add("--foople");
+        cmdLineArgs.add("-Dzed.key=0.value");
+
+        List<String> output;
+
+        try (ByteArrayOutputStream baos = new ByteArrayOutputStream();
+             PrintStream out = new PrintStream(baos, true, StandardCharsets.UTF_8))
+        {
+            PrintStream originalStream = StartLog.setStream(new PrintStream(out));
+            try
+            {
+                Main main = new Main();
+                StartArgs args = main.processCommandLine(cmdLineArgs.toArray(new String[0]));
+                main.start(args);
+                out.flush();
+                output = List.of(baos.toString(StandardCharsets.UTF_8).split(System.lineSeparator()));
+            }
+            finally
+            {
+                StartLog.setStream(originalStream);
+            }
+        }
+
+        // Test a System Property that comes from JVM
+        List<String> warnings = output.stream().filter((line) -> line.startsWith("WARN")).collect(Collectors.toList());
+        // warnings.forEach(System.out::println);
+        Iterator<String> warningIter = warnings.iterator();
+
+        assertThat("Announcement", warningIter.next(), containsString("Unknown Arguments detected."));
+        assertThat("System Prop on command line detail", warningIter.next(), containsString("Argument: -Dzed.key=0.value (interpreted as a System property, from <command-line>"));
+        assertThat("JVM Arg in ini detail", warningIter.next(), containsString("Argument: --zed-0-zed (interpreted as a JVM argument, from " + zedIni));
+        assertThat("JVM Arg on command line detail", warningIter.next(), containsString("Argument: --foople (interpreted as a JVM argument, from <command-line>"));
+    }
+
     @Test
     @Disabled("Just a bit noisy for general testing")
     public void testHelp() throws Exception

diff --git a/jetty-start/src/test/java/org/eclipse/jetty/start/PropertyDump.java b/jetty-start/src/test/java/org/eclipse/jetty/start/PropertyDump.java
@@ -63,7 +63,5 @@ public static void main(String[] args)
                 }
             }
         }
-
-        System.exit(0);
     }
 }
diff --git a/jetty-start/src/test/java/org/eclipse/jetty/start/usecases/BasicTest.java b/jetty-start/src/test/java/org/eclipse/jetty/start/usecases/BasicTest.java
@@ -256,7 +256,7 @@ public void testWithCommandLine() throws Exception
             "-Xms1g",
             "-Xmx1g"
         );
-        List<String> actualJvmArgs = results.startArgs.getJvmArgs();
+        List<String> actualJvmArgs = new ArrayList<>(results.startArgs.getJvmArgSources().keySet());
         assertThat("JVM Args", actualJvmArgs, contains(expectedJvmArgs.toArray()));
     }
-Original file line number
+Diff line change
@@ Expand Up / @@ -63,7 +63,5 @@ public static void main(String[] args) @@
                     }
                 }
             }
-            System.exit(0);
         }
     }