This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-cli.git
The following commit(s) were added to refs/heads/master by this push:
new 4417394 CLI-254: "test" gets parsed as test, quotes die :-( (#58)
4417394 is described below
commit 44173949e8165537b201d4efd024e31e6e8b06eb
Author: Istvan Toth <[email protected]>
AuthorDate: Sun Oct 17 16:37:00 2021 +0200
CLI-254: "test" gets parsed as test, quotes die :-( (#58)
* CLI-254 "test" gets parsed as test, quotes die :-(
* address review comments, improve backwards compatibility
---
.../java/org/apache/commons/cli/DefaultParser.java | 161 ++++++++++++++++++++-
.../org/apache/commons/cli/BasicParserTest.java | 12 ++
.../org/apache/commons/cli/DefaultParserTest.java | 93 ++++++++++++
.../org/apache/commons/cli/ParserTestCase.java | 37 +++++
4 files changed, 297 insertions(+), 6 deletions(-)
diff --git a/src/main/java/org/apache/commons/cli/DefaultParser.java
b/src/main/java/org/apache/commons/cli/DefaultParser.java
index 0c3d069..4a3a65b 100644
--- a/src/main/java/org/apache/commons/cli/DefaultParser.java
+++ b/src/main/java/org/apache/commons/cli/DefaultParser.java
@@ -55,6 +55,10 @@ public class DefaultParser implements CommandLineParser {
/** Flag indicating if partial matching of long options is supported. */
private final boolean allowPartialMatching;
+ /** Flag indicating if balanced leading and trailing double quotes should
be stripped from option arguments.
+ * null represents the historic arbitrary behaviour */
+ private final Boolean stripLeadingAndTrailingQuotes;
+
/**
* Creates a new DefaultParser instance with partial matching enabled.
*
@@ -76,6 +80,7 @@ public class DefaultParser implements CommandLineParser {
*/
public DefaultParser() {
this.allowPartialMatching = true;
+ this.stripLeadingAndTrailingQuotes = null;
}
/**
@@ -101,6 +106,20 @@ public class DefaultParser implements CommandLineParser {
*/
public DefaultParser(final boolean allowPartialMatching) {
this.allowPartialMatching = allowPartialMatching;
+ this.stripLeadingAndTrailingQuotes = null;
+ }
+
+ /**
+ * Creates a new DefaultParser instance with the specified partial
matching and quote
+ * stripping policy.
+ *
+ * @param allowPartialMatching if partial matching of long options shall
be enabled
+ * @param stripLeadingAndTrailingQuotes if balanced outer double quoutes
should be stripped
+ */
+ private DefaultParser(final boolean allowPartialMatching,
+ final Boolean stripLeadingAndTrailingQuotes) {
+ this.allowPartialMatching = allowPartialMatching;
+ this.stripLeadingAndTrailingQuotes = stripLeadingAndTrailingQuotes;
}
/**
@@ -196,7 +215,7 @@ public class DefaultParser implements CommandLineParser {
if (currentOption != null && token.length() != i + 1) {
// add the trail as an argument of the option
- currentOption.addValueForProcessing(token.substring(i + 1));
+
currentOption.addValueForProcessing(stripLeadingAndTrailingQuotesDefaultOff(token.substring(i
+ 1)));
break;
}
}
@@ -242,7 +261,7 @@ public class DefaultParser implements CommandLineParser {
if (option.acceptsArg()) {
handleOption(option);
- currentOption.addValueForProcessing(value);
+
currentOption.addValueForProcessing(stripLeadingAndTrailingQuotesDefaultOff(value));
currentOption = null;
} else {
handleUnknownToken(currentToken);
@@ -314,7 +333,7 @@ public class DefaultParser implements CommandLineParser {
if (opt.hasArg()) {
if (opt.getValues() == null || opt.getValues().length ==
0) {
- opt.addValueForProcessing(value);
+
opt.addValueForProcessing(stripLeadingAndTrailingQuotesDefaultOff(value));
}
} else if (!("yes".equalsIgnoreCase(value) ||
"true".equalsIgnoreCase(value) || "1".equalsIgnoreCase(value))) {
// if the value is not yes, true or 1 then don't add the
option to the CommandLine
@@ -361,12 +380,12 @@ public class DefaultParser implements CommandLineParser {
if (opt != null && options.getOption(opt).acceptsArg()) {
handleOption(options.getOption(opt));
-
currentOption.addValueForProcessing(t.substring(opt.length()));
+
currentOption.addValueForProcessing(stripLeadingAndTrailingQuotesDefaultOff(t.substring(opt.length())));
currentOption = null;
} else if (isJavaProperty(t)) {
// -SV1 (-Dflag)
handleOption(options.getOption(t.substring(0, 1)));
- currentOption.addValueForProcessing(t.substring(1));
+
currentOption.addValueForProcessing(stripLeadingAndTrailingQuotesDefaultOff(t.substring(1)));
currentOption = null;
} else {
// -S1S2S3 or -S1S2V
@@ -415,7 +434,7 @@ public class DefaultParser implements CommandLineParser {
} else if ("--".equals(token)) {
skipParsing = true;
} else if (currentOption != null && currentOption.acceptsArg() &&
isArgument(token)) {
-
currentOption.addValueForProcessing(Util.stripLeadingAndTrailingQuotes(token));
+
currentOption.addValueForProcessing(stripLeadingAndTrailingQuotesDefaultOn(token));
} else if (token.startsWith("--")) {
handleLongOption(token);
} else if (token.startsWith("-") && !"-".equals(token)) {
@@ -625,4 +644,134 @@ public class DefaultParser implements CommandLineParser {
group.setSelected(option);
}
}
+
+ /**
+ * Strip balanced leading and trailing quotes if the
stripLeadingAndTrailingQuotes is set
+ * If stripLeadingAndTrailingQuotes is null, then do not strip
+ *
+ * @param token a string
+ * @return token with the quotes stripped (if set)
+ */
+ private String stripLeadingAndTrailingQuotesDefaultOff(final String token)
{
+ if (stripLeadingAndTrailingQuotes != null &&
stripLeadingAndTrailingQuotes) {
+ return Util.stripLeadingAndTrailingQuotes(token);
+ } else {
+ return token;
+ }
+ }
+
+ /**
+ * Strip balanced leading and trailing quotes if the
stripLeadingAndTrailingQuotes is set
+ * If stripLeadingAndTrailingQuotes is null, then do not strip
+ *
+ * @param token a string
+ * @return token with the quotes stripped (if set)
+ */
+ private String stripLeadingAndTrailingQuotesDefaultOn(final String token) {
+ if (stripLeadingAndTrailingQuotes == null ||
stripLeadingAndTrailingQuotes) {
+ return Util.stripLeadingAndTrailingQuotes(token);
+ } else {
+ return token;
+ }
+ }
+
+ /**
+ * Returns a {@link Builder} to create an {@link DefaultParser} using
descriptive
+ * methods.
+ *
+ * @return a new {@link Builder} instance
+ * @since 1.5
+ */
+ public static Builder builder() {
+ return new Builder();
+ }
+
+ /**
+ * A nested builder class to create <code>DefaultParser</code> instances
+ * using descriptive methods.
+ *
+ * Example usage:
+ * <pre>
+ * DefaultParser parser = Option.builder()
+ * .setAllowPartialMatching(false)
+ * .setStripLeadingAndTrailingQuotes(false)
+ * .build();
+ * </pre>
+ *
+ * @since 1.5
+ */
+ public static final class Builder {
+
+ /** Flag indicating if partial matching of long options is supported.
*/
+ private boolean allowPartialMatching = true;
+
+ /** Flag indicating if balanced leading and trailing double quotes
should be stripped from option arguments. */
+ private Boolean stripLeadingAndTrailingQuotes;
+
+ /**
+ * Constructs a new <code>Builder</code> for a
<code>DefaultParser</code> instance.
+ *
+ * Both allowPartialMatching and stripLeadingAndTrailingQuotes are
true by default,
+ * mimicking the argument-less constructor.
+ */
+ private Builder() {
+ }
+
+ /**
+ * Sets if partial matching of long options is supported.
+ *
+ * By "partial matching" we mean that given the following code:
+ *
+ * <pre>
+ * {
+ * @code
+ * final Options options = new Options();
+ * options.addOption(new Option("d", "debug", false, "Turn on
debug."));
+ * options.addOption(new Option("e", "extract", false, "Turn on
extract."));
+ * options.addOption(new Option("o", "option", true, "Turn on
option with argument."));
+ * }
+ * </pre>
+ *
+ * If "partial matching" is turned on, {@code -de} only matches the
{@code "debug"} option. However, with
+ * "partial matching" disabled, {@code -de} would enable both {@code
debug} as well as {@code extract}
+ *
+ * @param allowPartialMatching whether to allow partial matching of
long options
+ * @return this builder, to allow method chaining
+ * @since 1.5
+ */
+ public Builder setAllowPartialMatching(final boolean
allowPartialMatching) {
+ this.allowPartialMatching = allowPartialMatching;
+ return this;
+ }
+
+ /**
+ * Sets if balanced leading and trailing double quotes should be
stripped from option arguments.
+ *
+ * If "stripping of balanced leading and trailing double quotes from
option arguments" is true,
+ * the outermost balanced double quotes of option arguments values
will be removed.
+ * For example, <code>-o '"x"'</code> getValue() will return
<code>x</code>, instead of <code>"x"</code>
+ *
+ * If "stripping of balanced leading and trailing double quotes from
option arguments" is null,
+ * then quotes will be stripped from option values separated by space
from the option, but
+ * kept in other cases, which is the historic behaviour.
+ *
+ * @param stripLeadingAndTrailingQuotes whether balanced leading and
trailing double quotes should be stripped from option arguments.
+ * @return this builder, to allow method chaining
+ * @since 1.5
+ */
+ public Builder setStripLeadingAndTrailingQuotes(final Boolean
stripLeadingAndTrailingQuotes) {
+ this.stripLeadingAndTrailingQuotes = stripLeadingAndTrailingQuotes;
+ return this;
+ }
+
+ /**
+ * Builds an DefaultParser with the values declared by this {@link
Builder}.
+ *
+ * @return the new {@link DefaultParser}
+ * @since 1.5
+ */
+ public DefaultParser build() {
+ return new DefaultParser(allowPartialMatching,
stripLeadingAndTrailingQuotes);
+ }
+ }
}
diff --git a/src/test/java/org/apache/commons/cli/BasicParserTest.java
b/src/test/java/org/apache/commons/cli/BasicParserTest.java
index 5d5fcd8..2526e3f 100644
--- a/src/test/java/org/apache/commons/cli/BasicParserTest.java
+++ b/src/test/java/org/apache/commons/cli/BasicParserTest.java
@@ -173,4 +173,16 @@ public class BasicParserTest extends ParserTestCase {
@Ignore("not supported by the BasicParser")
public void testUnrecognizedOptionWithBursting() throws Exception {
}
+
+ @Override
+ @Test
+ @Ignore("not supported by the BasicParser")
+ public void testShortOptionConcatenatedQuoteHandling() throws Exception {
+ }
+
+ @Override
+ @Test
+ @Ignore("not supported by the BasicParser")
+ public void testLongOptionWithEqualsQuoteHandling() throws Exception {
+ }
}
diff --git a/src/test/java/org/apache/commons/cli/DefaultParserTest.java
b/src/test/java/org/apache/commons/cli/DefaultParserTest.java
index 3cde2fe..d0ebe7b 100644
--- a/src/test/java/org/apache/commons/cli/DefaultParserTest.java
+++ b/src/test/java/org/apache/commons/cli/DefaultParserTest.java
@@ -17,7 +17,10 @@
package org.apache.commons.cli;
+import static org.junit.Assert.assertEquals;
+
import org.junit.Before;
+import org.junit.Test;
public class DefaultParserTest extends ParserTestCase {
@@ -27,4 +30,94 @@ public class DefaultParserTest extends ParserTestCase {
super.setUp();
parser = new DefaultParser();
}
+
+ @Override
+ @Test
+ public void testShortOptionConcatenatedQuoteHandling() throws Exception {
+ final String[] args = new String[] {"-b\"quoted string\""};
+
+ final CommandLine cl = parser.parse(options, args);
+
+ //This is behaviour is not consistent with the other parsers, but is
required for backwards compatibility
+ assertEquals("Confirm -b\"arg\" keeps quotes", "\"quoted string\"",
cl.getOptionValue("b"));
+ }
+
+ @Override
+ @Test
+ public void testLongOptionWithEqualsQuoteHandling() throws Exception {
+ final String[] args = new String[] {"--bfile=\"quoted string\""};
+
+ final CommandLine cl = parser.parse(options, args);
+
+ assertEquals("Confirm --bfile=\"arg\" strips quotes", "\"quoted
string\"", cl.getOptionValue("b"));
+ }
+
+ @Test
+ public void testShortOptionQuoteHandlingWithStrip() throws Exception {
+ parser =
DefaultParser.builder().setStripLeadingAndTrailingQuotes(true).build();
+ final String[] args = new String[] {"-b", "\"quoted string\""};
+
+ final CommandLine cl = parser.parse(options, args);
+
+ assertEquals("Confirm -b \"arg\" strips quotes", "quoted string",
cl.getOptionValue("b"));
+ }
+
+ @Test
+ public void testShortOptionQuoteHandlingWithoutStrip() throws Exception {
+ parser =
DefaultParser.builder().setStripLeadingAndTrailingQuotes(false).build();
+ final String[] args = new String[] {"-b", "\"quoted string\""};
+
+ final CommandLine cl = parser.parse(options, args);
+
+ assertEquals("Confirm -b \"arg\" keeps quotes", "\"quoted string\"",
cl.getOptionValue("b"));
+ }
+
+ @Test
+ public void testLongOptionQuoteHandlingWithStrip() throws Exception {
+ parser =
DefaultParser.builder().setStripLeadingAndTrailingQuotes(true).build();
+ final String[] args = new String[] {"--bfile", "\"quoted string\""};
+
+ final CommandLine cl = parser.parse(options, args);
+
+ assertEquals("Confirm --bfile \"arg\" strips quotes", "quoted
string", cl.getOptionValue("b"));
+ }
+
+ @Test
+ public void testLongOptionQuoteHandlingWithoutStrip() throws Exception {
+ parser =
DefaultParser.builder().setStripLeadingAndTrailingQuotes(false).build();
+ final String[] args = new String[] {"--bfile", "\"quoted string\""};
+
+ final CommandLine cl = parser.parse(options, args);
+
+ assertEquals("Confirm --bfile \"arg\" keeps quotes", "\"quoted
string\"", cl.getOptionValue("b"));
+ }
+
+ @Test
+ public void testLongOptionWithEqualsQuoteHandlingWithStrip() throws
Exception {
+ parser =
DefaultParser.builder().setStripLeadingAndTrailingQuotes(true).build();
+ final String[] args = new String[] {"--bfile=\"quoted string\""};
+
+ final CommandLine cl = parser.parse(options, args);
+
+ assertEquals("Confirm --bfile=\"arg\" strips quotes", "quoted
string", cl.getOptionValue("b"));
+ }
+
+ @Test
+ public void testLongOptionWithEqualsQuoteHandlingWithoutStrip() throws
Exception {
+ parser =
DefaultParser.builder().setStripLeadingAndTrailingQuotes(false).build();
+ final String[] args = new String[] {"--bfile=\"quoted string\""};
+
+ final CommandLine cl = parser.parse(options, args);
+
+ assertEquals("Confirm --bfile=\"arg\" keeps quotes", "\"quoted
string\"", cl.getOptionValue("b"));
+ }
+
+ @Test
+ public void testBuilder() throws Exception {
+ parser = DefaultParser.builder()
+ .setStripLeadingAndTrailingQuotes(false)
+ .setAllowPartialMatching(false)
+ .build();
+ assertEquals(DefaultParser.class, parser.getClass());
+ }
}
diff --git a/src/test/java/org/apache/commons/cli/ParserTestCase.java
b/src/test/java/org/apache/commons/cli/ParserTestCase.java
index f74b651..cae48a6 100644
--- a/src/test/java/org/apache/commons/cli/ParserTestCase.java
+++ b/src/test/java/org/apache/commons/cli/ParserTestCase.java
@@ -956,6 +956,42 @@ public abstract class ParserTestCase {
}
@Test
+ public void testShortOptionQuoteHandling() throws Exception {
+ final String[] args = new String[] {"-b", "\"quoted string\""};
+
+ final CommandLine cl = parser.parse(options, args);
+
+ assertEquals("Confirm -b \"arg\" strips quotes", "quoted string",
cl.getOptionValue("b"));
+ }
+
+ @Test
+ public void testLongOptionQuoteHandling() throws Exception {
+ final String[] args = new String[] {"--bfile", "\"quoted string\""};
+
+ final CommandLine cl = parser.parse(options, args);
+
+ assertEquals("Confirm --bfile \"arg\" strips quotes", "quoted
string", cl.getOptionValue("b"));
+ }
+
+ @Test
+ public void testLongOptionWithEqualsQuoteHandling() throws Exception {
+ final String[] args = new String[] {"--bfile=\"quoted string\""};
+
+ final CommandLine cl = parser.parse(options, args);
+
+ assertEquals("Confirm --bfile=\"arg\" strips quotes", "quoted
string", cl.getOptionValue("b"));
+ }
+
+ @Test
+ public void testShortOptionConcatenatedQuoteHandling() throws Exception {
+ final String[] args = new String[] {"-b\"quoted string\""};
+
+ final CommandLine cl = parser.parse(options, args);
+
+ assertEquals("Confirm -b\"arg\" strips quotes", "quoted string",
cl.getOptionValue("b"));
+ }
+
+ @Test
public void testWithRequiredOption() throws Exception {
final String[] args = {"-b", "file"};
@@ -970,4 +1006,5 @@ public abstract class ParserTestCase {
assertEquals("Confirm arg of -b", "file", cl.getOptionValue("b"));
assertTrue("Confirm NO of extra args", cl.getArgList().isEmpty());
}
+
}
