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

add a new constructor parameter to DefaultParser that disables quote
processing on option arguments

Author: stoty

src/main/java/org/apache/commons/cli/DefaultParser.java

@@ -57,8 +57,12 @@ 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.
+     * Creates a new DefaultParser instance with partial matching and
+     * stripping of balanced leading and trailing double quotes from option arguments enabled.
      *
      * By "partial matching" we mean that given the following code:
      * <pre>
@@ -73,13 +77,19 @@ public class DefaultParser implements CommandLineParser
      * <code>"debug"</code> option. However, with "partial matching" disabled,
      * <code>-de</code> would enable both <code>debug</code> as well as
      * <code>extract</code> options.
+     * 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>
      */
     public DefaultParser() {
         this.allowPartialMatching = true;
+        this.stripLeadingAndTrailingQuotes = true;
     }
 
     /**
-     * Create a new DefaultParser instance with the specified partial matching policy.
+     * Create a new DefaultParser instance with the specified partial matching policy and
+     * stripping of balanced leading and trailing double quotes from option arguments enabled.
      *
      * By "partial matching" we mean that given the following code:
      * <pre>
@@ -94,13 +104,48 @@ public DefaultParser() {
      * <code>"debug"</code> option. However, with "partial matching" disabled,
      * <code>-de</code> would enable both <code>debug</code> as well as
      * <code>extract</code> options.
-     *
+     * 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 allowPartialMatching if partial matching of long options shall be enabled
      */
     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.
+     *
+     * 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>
+     * with "partial matching" turned on, <code>-de</code> only matches the
+     * <code>"debug"</code> option. However, with "partial matching" disabled,
+     * <code>-de</code> would enable both <code>debug</code> as well as
+     * <code>extract</code> options.
+     * 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 allowPartialMatching if partial matching of long options shall be enabled
+     * @param stripLeadingAndTrailingQuotes if balanced outer double quoutes should be stripped
+     */
+    public DefaultParser(final boolean allowPartialMatching,
+            final boolean stripLeadingAndTrailingQuotes) {
+        this.allowPartialMatching = allowPartialMatching;
+        this.stripLeadingAndTrailingQuotes = stripLeadingAndTrailingQuotes;
     }
 
+
     @Override
     public CommandLine parse(final Options options, final String[] arguments) throws ParseException
     {
@@ -280,7 +325,7 @@ else if ("--".equals(token))
         }
         else if (currentOption != null && currentOption.acceptsArg() && isArgument(token))
         {
-            currentOption.addValueForProcessing(Util.stripLeadingAndTrailingQuotes(token));
+            currentOption.addValueForProcessing(conditionallyStripLeadingAndTrailingQuotes(token));
         }
         else if (token.startsWith("--"))
         {
@@ -777,4 +822,12 @@ protected void handleConcatenatedOptions(final String token) throws ParseExcepti
             }
         }
     }
+
+    protected String conditionallyStripLeadingAndTrailingQuotes(final String token) {
+        if(stripLeadingAndTrailingQuotes) {
+            return Util.stripLeadingAndTrailingQuotes(token);
+        } else {
+            return token;
+        }
+    }
 }

src/test/java/org/apache/commons/cli/ParserTestCase.java

@@ -22,6 +22,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;
@@ -1105,4 +1106,61 @@ public void testPropertyOptionGroup() throws Exception
         assertTrue(cmd.hasOption("x"));
         assertFalse(cmd.hasOption("y"));
     }
+
+    @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 = new DefaultParser(true, false);
+        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 = new DefaultParser(true, false);
+        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"));
+    }
 }