8234808: jdb quoted option parsing broken

Reviewed-by: cjplummer, sspitsyn

src/jdk.jdi/share/classes/com/sun/tools/example/debug/tty/Env.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -57,8 +57,8 @@ class Env {
     private static HashMap<String, Value> savedValues = new HashMap<String, Value>();
     private static Method atExitMethod;
 
-    static void init(String connectSpec, boolean openNow, int flags) {
+    static void init(String connectSpec, boolean openNow, int flags, String extraOptions) {
-        connection = new VMConnection(connectSpec, flags);
+        connection = new VMConnection(connectSpec, flags, extraOptions);
         if (!connection.isLaunch() || openNow) {
             connection.open();
         }

src/jdk.jdi/share/classes/com/sun/tools/example/debug/tty/TTY.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -1107,14 +1107,10 @@ public class TTY implements EventNotifier {
                            connectSpec);
                 return;
             }
-            connectSpec += "options=" + javaArgs + ",";
         }
 
         try {
-            if (! connectSpec.endsWith(",")) {
+            Env.init(connectSpec, launchImmediately, traceFlags, javaArgs);
-                connectSpec += ","; // (Bug ID 4285874)
-            }
-            Env.init(connectSpec, launchImmediately, traceFlags);
             new TTY();
         } catch(Exception e) {
             MessageOutput.printException("Internal exception:", e);

src/jdk.jdi/share/classes/com/sun/tools/example/debug/tty/VMConnection.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -78,7 +78,8 @@ class VMConnection {
         return null;
     }
 
-    private Map <String, com.sun.jdi.connect.Connector.Argument> parseConnectorArgs(Connector connector, String argString) {
+    private Map <String, com.sun.jdi.connect.Connector.Argument>
+            parseConnectorArgs(Connector connector, String argString, String extraOptions) {
         Map<String, com.sun.jdi.connect.Connector.Argument> arguments = connector.defaultArguments();
 
         /*
@@ -121,10 +122,20 @@ class VMConnection {
              */
             if (name.equals("options")) {
                 StringBuilder sb = new StringBuilder();
+                if (extraOptions != null) {
+                    sb.append(extraOptions).append(" ");
+                    // set extraOptions to null to avoid appending it again
+                    extraOptions = null;
+                }
                 for (String s : splitStringAtNonEnclosedWhiteSpace(value)) {
+                    boolean wasEnclosed = false;
                     while (isEnclosed(s, "\"") || isEnclosed(s, "'")) {
+                        wasEnclosed = true;
                         s = s.substring(1, s.length() - 1);
                     }
+                    if (wasEnclosed && hasWhitespace(s)) {
+                        s = "\"" + s + "\"";
+                    }
                     sb.append(s);
                     sb.append(" ");
                 }
@@ -150,9 +161,26 @@ class VMConnection {
             throw new IllegalArgumentException
                 (MessageOutput.format("Illegal connector argument", argString));
         }
+        if (extraOptions != null) {
+            // there was no "options" specified in argString
+            Connector.Argument argument = arguments.get("options");
+            if (argument != null) {
+                argument.setValue(extraOptions);
+            }
+        }
         return arguments;
     }
 
+    private static boolean hasWhitespace(String string) {
+        int length = string.length();
+        for (int i = 0; i < length; i++) {
+            if (Character.isWhitespace(string.charAt(i))) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     private static boolean isEnclosed(String value, String enclosingChar) {
         if (value.indexOf(enclosingChar) == 0) {
             int lastIndex = value.lastIndexOf(enclosingChar);
@@ -299,7 +327,7 @@ class VMConnection {
         return (pos + 1 == arr.length);
     }
 
-    VMConnection(String connectSpec, int traceFlags) {
+    VMConnection(String connectSpec, int traceFlags, String extraOptions) {
         String nameString;
         String argString;
         int index = connectSpec.indexOf(':');
@@ -317,7 +345,7 @@ class VMConnection {
                 (MessageOutput.format("No connector named:", nameString));
         }
 
-        connectorArgs = parseConnectorArgs(connector, argString);
+        connectorArgs = parseConnectorArgs(connector, argString, extraOptions);
         this.traceFlags = traceFlags;
     }
 

test/jdk/com/sun/jdi/JdbOptions.java

@@ -0,0 +1,162 @@
+/*
+ * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8234808
+ *
+ * @library /test/lib
+ * @run main/othervm JdbOptions
+ */
+
+import jdk.test.lib.Platform;
+import lib.jdb.Jdb;
+import lib.jdb.JdbCommand;
+import jdk.test.lib.process.OutputAnalyzer;
+
+import java.lang.management.ManagementFactory;
+import java.util.Arrays;
+import java.util.List;
+
+class JbdOptionsTarg {
+    static final String OK_MSG = "JbdOptionsTarg: OK";
+
+    static String argString(String s) {
+        return "arg >" + s + "<";
+    }
+
+    static String propString(String name, String value) {
+        return "prop[" + name + "] = >" + value + "<";
+    }
+
+    public static void main(String[] args) {
+        System.out.println(OK_MSG);
+        // print all args
+        List<String> vmArgs = ManagementFactory.getRuntimeMXBean().getInputArguments();
+        for (String s: vmArgs) {
+            System.out.println(argString(s));
+        }
+        // print requested sys.props
+        for (String p: args) {
+            System.out.println(propString(p, System.getProperty(p)));
+        }
+    }
+}
+
+public class JdbOptions {
+    private static final String targ = JbdOptionsTarg.class.getName();
+
+    public static void main(String[] args) throws Exception {
+        // the simplest case
+        test("-connect",
+                "com.sun.jdi.CommandLineLaunch:vmexec=java,options=-client -XX:+PrintVMOptions,main=" + targ)
+            .expectedArg("-XX:+PrintVMOptions");
+
+        // pass property through 'options'
+        test("-connect",
+                "com.sun.jdi.CommandLineLaunch:vmexec=java,options='-Dboo=foo',main=" + targ + " boo")
+            .expectedProp("boo", "foo");
+
+        // property with spaces
+        test("-connect",
+                "com.sun.jdi.CommandLineLaunch:vmexec=java,options=\"-Dboo=foo 2\",main=" + targ + " boo")
+            .expectedProp("boo", "foo 2");
+
+        // property with spaces (with single quotes)
+        test("-connect",
+                "com.sun.jdi.CommandLineLaunch:vmexec=java,options='-Dboo=foo 2',main=" + targ + " boo")
+                .expectedProp("boo", "foo 2");
+
+        // properties with spaces (with single quotes)
+        test("-connect",
+                "com.sun.jdi.CommandLineLaunch:vmexec=java,options=-Dboo=foo '-Dboo2=foo 2',main=" + targ + " boo boo2")
+                .expectedProp("boo", "foo")
+                .expectedProp("boo2", "foo 2");
+
+        // 'options' contains commas - values are quoted (double quotes)
+        test("-connect",
+                "com.sun.jdi.CommandLineLaunch:vmexec=java,options=\"-client\" \"-XX:+PrintVMOptions\""
+                + " \"-XX:StartFlightRecording=dumponexit=true,maxsize=500M\" \"-XX:FlightRecorderOptions=repository=jfrrep\""
+                + ",main=" + targ)
+            .expectedArg("-XX:StartFlightRecording=dumponexit=true,maxsize=500M")
+            .expectedArg("-XX:FlightRecorderOptions=repository=jfrrep");
+
+        // 'options' contains commas - values are quoted (single quotes)
+        test("-connect",
+                "com.sun.jdi.CommandLineLaunch:vmexec=java,options='-client' '-XX:+PrintVMOptions'"
+                        + " '-XX:StartFlightRecording=dumponexit=true,maxsize=500M' '-XX:FlightRecorderOptions=repository=jfrrep'"
+                        + ",main=" + targ)
+            .expectedArg("-XX:StartFlightRecording=dumponexit=true,maxsize=500M")
+            .expectedArg("-XX:FlightRecorderOptions=repository=jfrrep");
+
+        // java options are specified in 2 ways, with and without spaces
+        // options are quoted by using single and double quotes.
+        test("-Dprop1=val1",
+                "-Dprop2=val 2",
+                "-connect",
+                "com.sun.jdi.CommandLineLaunch:vmexec=java,options=-Dprop3=val3 '-Dprop4=val 4'"
+                        + " \"-XX:StartFlightRecording=dumponexit=true,maxsize=500M\""
+                        + " '-XX:FlightRecorderOptions=repository=jfrrep'"
+                        + ",main=" + targ + " prop1 prop2 prop3 prop4")
+                .expectedProp("prop1", "val1")
+                .expectedProp("prop2", "val 2")
+                .expectedProp("prop3", "val3")
+                .expectedProp("prop4", "val 4")
+                .expectedArg("-XX:StartFlightRecording=dumponexit=true,maxsize=500M")
+                .expectedArg("-XX:FlightRecorderOptions=repository=jfrrep");
+
+    }
+
+    private static class TestResult {
+        OutputAnalyzer out;
+        TestResult(OutputAnalyzer output) {
+            out = output;
+        }
+        TestResult expectedArg(String s) {
+            out.shouldContain(JbdOptionsTarg.argString(s));
+            return this;
+        }
+        TestResult expectedProp(String name, String value) {
+            out.shouldContain(JbdOptionsTarg.propString(name, value));
+            return this;
+        }
+    }
+
+    private static TestResult test(String... args) throws Exception {
+        System.out.println();
+        System.out.println("...testcase...");
+        if (Platform.isWindows()) {
+            // on Windows we need to escape quotes
+            args = Arrays.stream(args)
+                    .map(s -> s.replace("\"", "\\\""))
+                    .toArray(String[]::new);
+        }
+        try (Jdb jdb = new Jdb(args)) {
+            jdb.waitForSimplePrompt(1024, true); // 1024 lines should be enough
+            jdb.command(JdbCommand.run().allowExit());
+            OutputAnalyzer out = new OutputAnalyzer(jdb.getJdbOutput());
+            out.shouldContain(JbdOptionsTarg.OK_MSG);
+            return new TestResult(out);
+        }
+    }
+}