Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
8234808: jdb quoted option parsing broken
Reviewed-by: cjplummer, sspitsyn
  • Loading branch information
Alex Menkov committed Sep 22, 2020
1 parent d8921ed commit d1f9b8a
Show file tree
Hide file tree
Showing 4 changed files with 201 additions and 15 deletions.
@@ -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
Expand Down Expand Up @@ -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) {
connection = new VMConnection(connectSpec, flags);
static void init(String connectSpec, boolean openNow, int flags, String extraOptions) {
connection = new VMConnection(connectSpec, flags, extraOptions);
if (!connection.isLaunch() || openNow) {
connection.open();
}
Expand Down
@@ -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
Expand Down Expand Up @@ -1069,8 +1069,8 @@ public static void main(String argv[]) throws MissingResourceException {
/*
* Here are examples of jdb command lines and how the options
* are interpreted as arguments to the program being debugged.
* arg1 arg2
* ---- ----
* arg1 arg2
* ---- ----
* jdb hello a b a b
* jdb hello "a b" a b
* jdb hello a,b a,b
Expand Down Expand Up @@ -1107,14 +1107,10 @@ public static void main(String argv[]) throws MissingResourceException {
connectSpec);
return;
}
connectSpec += "options=" + javaArgs + ",";
}

try {
if (! connectSpec.endsWith(",")) {
connectSpec += ","; // (Bug ID 4285874)
}
Env.init(connectSpec, launchImmediately, traceFlags);
Env.init(connectSpec, launchImmediately, traceFlags, javaArgs);
new TTY();
} catch(Exception e) {
MessageOutput.printException("Internal exception:", e);
Expand Down
@@ -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
Expand Down Expand Up @@ -78,7 +78,8 @@ private Connector findConnector(String name) {
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();

/*
Expand Down Expand Up @@ -121,10 +122,20 @@ private Map <String, com.sun.jdi.connect.Connector.Argument> parseConnectorArgs(
*/
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(" ");
}
Expand All @@ -150,9 +161,26 @@ private Map <String, com.sun.jdi.connect.Connector.Argument> parseConnectorArgs(
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);
Expand Down Expand Up @@ -299,7 +327,7 @@ static private boolean isLastChar(char[] arr, int pos) {
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(':');
Expand All @@ -317,7 +345,7 @@ static private boolean isLastChar(char[] arr, int pos) {
(MessageOutput.format("No connector named:", nameString));
}

connectorArgs = parseConnectorArgs(connector, argString);
connectorArgs = parseConnectorArgs(connector, argString, extraOptions);
this.traceFlags = traceFlags;
}

Expand Down
162 changes: 162 additions & 0 deletions 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);
}
}
}

1 comment on commit d1f9b8a

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented on d1f9b8a Sep 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.