CLI-254 "test" gets parsed as test, quotes die :-(

stoty · stoty · commit 4a7dc42706e7 · 2021-10-08T07:51:11.000+02:00
diff --git a/src/main/java/org/apache/commons/cli/DefaultParser.java b/src/main/java/org/apache/commons/cli/DefaultParser.java
@@ -55,6 +55,9 @@ 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. */
+    private final boolean stripLeadingAndTrailingQuotes;
+
     /**
      * Creates a new DefaultParser instance with partial matching enabled.
      *
@@ -76,6 +79,7 @@ public class DefaultParser implements CommandLineParser {
      */
     public DefaultParser() {
         this.allowPartialMatching = true;
+        this.stripLeadingAndTrailingQuotes = true;
     }
 
     /**
@@ -101,6 +105,20 @@ public DefaultParser() {
      */
     public DefaultParser(final boolean allowPartialMatching) {
         this.allowPartialMatching = allowPartialMatching;
+        this.stripLeadingAndTrailingQuotes = true;
+    }
+
+    /**
+     * Create 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;
     }
 
     /**
@@ -415,7 +433,7 @@ private void handleToken(final String token) throws ParseException {
         } else if ("--".equals(token)) {
             skipParsing = true;
         } else if (currentOption != null && currentOption.acceptsArg() && isArgument(token)) {
-            currentOption.addValueForProcessing(Util.stripLeadingAndTrailingQuotes(token));
+            currentOption.addValueForProcessing(conditionallyStripLeadingAndTrailingQuotes(token));
         } else if (token.startsWith("--")) {
             handleLongOption(token);
         } else if (token.startsWith("-") && !"-".equals(token)) {
@@ -625,4 +643,117 @@ private void updateRequiredOptions(final Option option) throws AlreadySelectedEx
             group.setSelected(option);
         }
     }
+
+    /**
+     * Strip balanced leading and trailing quotes if the stripLeadingAndTrailingQuotes is set
+     *
+     * @param token
+     * @return token with the quotes stripped (if set)
+     */
+    protected String conditionallyStripLeadingAndTrailingQuotes(final String token) {
+        if(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.
+     * <p>
+     * Example usage:
+     * <pre>
+     * DefaultParser parser = Option.builder()
+     *     .setAllowPartialMatching(false)
+     *     .setStripLeadingAndTrailingQuotes(false)
+     *     .build();
+     * </pre>
+     *
+     * @since 1.5
+     */
+    public static final class Builder
+    {
+
+        private boolean allowPartialMatching = true;
+
+        private boolean stripLeadingAndTrailingQuotes = true ;
+
+        /**
+         * 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>
+         * {
+         *     &#64;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>
+         *
+         * with "partial matching" 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
+         */
+        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.
+         *
+         * with "stripping of balanced leading and trailing double quotes from option arguments" turned
+         * on, the outermost balanced double quotes of option arguments values will be removed.
+         * ie.
+         * for <code>-o '"x"'</code> getValue() will return <code>x</code>, instead of <code>"x"</code>
+         *
+         * @param stripLeadingAndTrailingQuotes whether balanced leading and trailing double quotes should be stripped from option arguments.
+         * @return this builder, to allow method chaining
+         */
+        public Builder setStripLeadingAndTrailingQuotes(final boolean stripLeadingAndTrailingQuotes)
+        {
+            this.stripLeadingAndTrailingQuotes = stripLeadingAndTrailingQuotes;
+            return this;
+        }
+
+        /**
+         * Constructs an DefaultParser with the values declared by this {@link Builder}.
+         *
+         * @return the new {@link DefaultParser}
+         */
+        public DefaultParser build()
+        {
+            return new DefaultParser(allowPartialMatching, stripLeadingAndTrailingQuotes);
+
+        }
+    }
 }
diff --git a/src/test/java/org/apache/commons/cli/ParserTestCase.java b/src/test/java/org/apache/commons/cli/ParserTestCase.java
@@ -23,6 +23,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.junit.Assume.assumeTrue;
 
 import java.util.Arrays;
 import java.util.List;
@@ -970,4 +971,72 @@ public void testWithRequiredOption() throws Exception {
         assertEquals("Confirm arg of -b", "file", cl.getOptionValue("b"));
         assertTrue("Confirm NO of extra args", cl.getArgList().isEmpty());
     }
+
+
+    @Test
+    public void testLongOptionWithEqualsQuoteHandling() throws Exception
+    {
+        assumeTrue(parser instanceof DefaultParser );
+        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("bfile"));
+    }
+
+    @Test
+    public void testShortOptionQuoteHandlingWithDefault() throws Exception
+    {
+        assumeTrue(parser instanceof DefaultParser );
+        final String[] args = new String[] { "-b", "\"quoted string\""};
+
+        final CommandLine cl = parser.parse(options, args);
+
+        assertEquals("Confirm -b strips quotes",  "quoted string", cl.getOptionValue("b"));
+    }
+
+    @Test
+    public void testShortOptionQuoteHandlingWithNoStrip() throws Exception
+    {
+        assumeTrue(parser instanceof DefaultParser );
+        parser = DefaultParser.builder().setStripLeadingAndTrailingQuotes(false).build();
+        final String[] args = new String[] { "-b", "\"quoted string\""};
+
+        final CommandLine cl = parser.parse(options, args);
+
+        assertEquals("Confirm -b keeps quotes",  "\"quoted string\"", cl.getOptionValue("b"));
+    }
+
+    @Test
+    public void testLongOptionQuoteHandlingWithDefault() throws Exception
+    {
+        assumeTrue(parser instanceof DefaultParser );
+        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 testLongOptionQuoteHandlingWithNoStrip() throws Exception
+    {
+        assumeTrue(parser instanceof DefaultParser );
+        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
+    {
+        assumeTrue(parser instanceof DefaultParser );
+        parser = DefaultParser.builder().setStripLeadingAndTrailingQuotes(false).build();
+        assumeTrue(parser instanceof DefaultParser );
+        parser = DefaultParser.builder().setAllowPartialMatching(false).build();
+        assumeTrue(parser instanceof DefaultParser );
+    }
 }
