[#738] Improve unquoting to handle nested escaped quotes

docs/index.adoc

@@ -1691,7 +1691,19 @@ From picocli 3.7, quotes around command line parameters are preserved by default
 If `CommandLine::setTrimQuotes` is set to `true`, picocli will remove quotes from the command line arguments, as follows:
 
 * if the command line argument contains just the leading and trailing quote, these quotes are removed
-* if the command line argument contains more quotes than just the leading and trailing quote, the parser first tries to process the parameter with the quotes intact. For example, the `split` regular expression inside a quoted region should be ignored, so arguments like `"a,b","x,y"` are handled correctly. For arguments with nested quotes, quotes are removed later in the processing pipeline, after `split` operations are applied.
+* if the command line argument contains unescaped quotes, other than the leading and trailing quote, the argument is unchanged (the leading and trailing quotes remain)
+* if a quoted command line argument contains backslash-escaped quotes, the leading and trailing quotes are removed, and the backslash-escaped quotes are converted to unescaped quotes.
+
+For example:
+
+[cols=3*,options="header"]
+|===
+|Command Line Argument|After Trimming Quotes|Note
+|`"-x=abc"`|`-x=abc`| quotes removed
+|`"a,b","x,y"` |`"a,b","x,y"`|left unchanged
+|`"-x=a,b,\"c,d,e\",f"`|`-x=a,b,"c,d,e",f`|Splitting will find 4 values
+|`"-x=\"a,b,\\"c,d,e\\",f\""`|`-x="a,b,\"c,d,e\",f"`|Splitting will find 4 values
+|===
 
 ==== Splitting Quoted Parameters
 Also, the logic for <<Split Regex,splitting>> parameters is smart enough to avoid matching the `split` regular expression inside a quoted region.
@@ -1727,8 +1739,9 @@ f
 "xxx,yyy"
 ```
 
-This can be configured with `CommandLine::setSplitQuotedStrings`:
-when the `splitQuotedStrings` parser attribute is set to `true` the `split` regex is applied to the parameter value regardless of quotes.
+This "smart splitting" can be switched off with `CommandLine::setSplitQuotedStrings`:
+setting the `splitQuotedStrings` parser attribute to `true` switches off smart splitting,
+and the `split` regex is applied to the parameter value regardless of quotes.
 
 [WARNING]
 ====

src/main/java/picocli/CommandLine.java

@@ -7355,10 +7355,10 @@ private String[] debug(String[] result, String msg, String value) {
             }
             // @since 3.7
             private static String[] splitRespectingQuotedStrings(String value, int limit, ParserSpec parser, ArgSpec argSpec, String splitRegex) {
+                Queue<String> quotedValues = new LinkedList<String>();
                 StringBuilder splittable = new StringBuilder();
                 StringBuilder temp = new StringBuilder();
                 StringBuilder current = splittable;
-                Queue<String> quotedValues = new LinkedList<String>();
                 boolean escaping = false, inQuote = false;
                 for (int ch = 0, i = 0; i < value.length(); i += Character.charCount(ch)) {
                     ch = value.codePointAt(i);
@@ -11925,19 +11925,6 @@ private boolean assertNoMissingParameters(ArgSpec argSpec, Range arity, Stack<St
             return true;
         }
 
-        /** Return the unquoted value if the value contains no nested quotes, otherwise, return the value as is. */
-        private String smartUnquote(String value) {
-            String unquoted = unquote(value);
-            if (unquoted == null || unquoted.contains("\"")) { return value; }
-            return unquoted;
-        }
-        private String unquote(String value) {
-            if (value == null || !commandSpec.parser().trimQuotes()) { return value; }
-            return (value.length() > 1 && value.startsWith("\"") && value.endsWith("\""))
-                    ? value.substring(1, value.length() - 1)
-                    : value;
-        }
-
         char[] readPassword(ArgSpec argSpec) {
             String name = argSpec.isOption() ? ((OptionSpec) argSpec).longestName() : "position " + position;
             String prompt = String.format("Enter value for %s (%s): ", name, str(argSpec.description(), 0));
@@ -11974,6 +11961,40 @@ String positionDesc(ArgSpec arg) {
             return (arg.group() == null) ? pos + " (command-local)" : pos + " (in group " + arg.group().synopsis() + ")";
         }
     }
+
+    /** Return the unquoted value if the value contains no nested quotes, otherwise, return the value as is. */
+    String smartUnquote(String value) {
+        if (value == null || !commandSpec.parser().trimQuotes()) { return value; }
+        String unquoted = unquote(value);
+        if (unquoted == value) { return value; }
+        StringBuilder result = new StringBuilder();
+        boolean requote = false;
+        int slashCount = 0;
+        for (int ch = 0, i = 0; i < unquoted.length(); i += Character.charCount(ch)) {
+            ch = unquoted.codePointAt(i);
+            switch (ch) {
+                case '\\':
+                    slashCount++;
+                    break;
+                case '\"':
+                    if (slashCount == 0) { requote = true; }
+                    slashCount = 0;
+                    break;
+                default: slashCount = 0; break;
+            }
+            if ((slashCount & 1) == 0) { result.appendCodePoint(ch); }
+        }
+        if (requote) {
+            result.append('"').insert(0, '"');
+        }
+        return result.toString();
+    }
+    private String unquote(String value) {
+        if (value == null || !commandSpec.parser().trimQuotes()) { return value; }
+        return (value.length() > 1 && value.startsWith("\"") && value.endsWith("\""))
+                ? value.substring(1, value.length() - 1)
+                : value;
+    }
     private static class PositionalParametersSorter implements Comparator<ArgSpec> {
         private static final Range OPTION_INDEX = new Range(0, 0, false, true, "0");
         public int compare(ArgSpec p1, ArgSpec p2) {

src/test/java/picocli/ArgSplitTest.java

@@ -2,6 +2,7 @@
 
 import org.junit.Ignore;
 import org.junit.Test;
+import picocli.CommandLine.Command;
 import picocli.CommandLine.MissingParameterException;
 import picocli.CommandLine.Model.ArgSpec;
 import picocli.CommandLine.Model.ParserSpec;
@@ -414,6 +415,42 @@ class Example {
         }
     }
 
+    @Test
+    public void testParseQuotedOptionsWithEscapedNestedQuotedValues() {
+        class Example {
+            @Option(names = "-x", split = ",")
+            List<String> parts;
+        }
+        String[] args = {"\"-x=a,b,\\\"c,d,e\\\",f\""};
+        Example example = new Example();
+        new CommandLine(example).setTrimQuotes(true).parseArgs(args);
+        assertEquals(Arrays.asList("a", "b", "c,d,e", "f"), example.parts);
+    }
+
+    @Test
+    public void testParseQuotedOptionsWithEscapedDoublyNestedQuotedValues() {
+        class Example {
+            @Option(names = "-x", split = ",")
+            List<String> parts;
+        }
+        String[] args = {"\"-x=\\\"a,b,\\\\\"c,d,e\\\\\",f\\\"\""};
+        Example example = new Example();
+        new CommandLine(example).setTrimQuotes(true).parseArgs(args);
+        assertEquals(Arrays.asList("a","b","c,d,e","f"), example.parts);
+    }
+
+    @Test
+    public void testParseQuotedOptionsWithEscapedNestedQuotedValues2() {
+        class Example {
+            @Option(names = "-x", split = ",")
+            List<String> parts;
+        }
+        String[] args = {"\"-x\"", "\"a,b,\\\"c,d,e\\\",f\""};
+        Example example = new Example();
+        new CommandLine(example).setTrimQuotes(true).parseArgs(args);
+        assertEquals(Arrays.asList("a", "b", "c,d,e", "f"), example.parts);
+    }
+
 
     // test https://github.com/remkop/picocli/issues/379
     @Test
@@ -449,6 +486,18 @@ class App {
         assertEquals("", app.parameters.get("OtherOptions"));
     }
 
+    @Test
+    public void testSmartUnquote() {
+//        assertEquals("\"-Dvalues=a,b,c\",\"-Dother=1,2\"", smartUnquote("\"-Dvalues=a,b,c\",\"-Dother=1,2\""));
+//        assertEquals("-x=a,b,\"c,d,e\",f", smartUnquote("\"-x=a,b,\\\"c,d,e\\\",f\""));
+        assertEquals("-x=\"a,b,\\\"c,d,e\\\",f\"", smartUnquote("\"-x=\\\"a,b,\\\\\"c,d,e\\\\\",f\\\"\""));
+    }
+
+    private String smartUnquote(String value) {
+        @Command class App {}
+        return new CommandLine(new App()).setTrimQuotes(true).smartUnquote(value);
+    }
+
     @Test
     public void testArgSpecSplitValueDebug() {
         PositionalParamSpec positional = PositionalParamSpec.builder().splitRegex("b").build();

src/test/java/picocli/CommandLineTest.java

@@ -3865,23 +3865,18 @@ class App {
 
     @SuppressWarnings("unchecked")
     @Test
-    public void testInterpreterUnquote() throws Exception {
-        Class c = Class.forName("picocli.CommandLine$Interpreter");
-        Method unquote = c.getDeclaredMethod("unquote", String.class);
-        unquote.setAccessible(true);
-
+    public void testSmartUnquote() {
         CommandSpec spec = CommandSpec.create();
         spec.parser().trimQuotes(true);
         CommandLine cmd = new CommandLine(spec);
-        Object interpreter = TestUtil.interpreter(cmd);
 
-        assertNull(unquote.invoke(interpreter, new Object[]{null}));
-        assertEquals("abc", unquote.invoke(interpreter, "\"abc\""));
-        assertEquals("", unquote.invoke(interpreter, "\"\""));
-        assertEquals("only balanced quotes 1", "\"abc", unquote.invoke(interpreter, "\"abc"));
-        assertEquals("only balanced quotes 2", "abc\"", unquote.invoke(interpreter, "abc\""));
-        assertEquals("only balanced quotes 3", "\"", unquote.invoke(interpreter, "\""));
-        assertEquals("no quotes", "X", unquote.invoke(interpreter, "X"));
+        assertNull(cmd.smartUnquote(null));
+        assertEquals("abc", cmd.smartUnquote("\"abc\""));
+        assertEquals("", cmd.smartUnquote("\"\""));
+        assertEquals("only balanced quotes 1", "\"abc", cmd.smartUnquote("\"abc"));
+        assertEquals("only balanced quotes 2", "abc\"", cmd.smartUnquote("abc\""));
+        assertEquals("only balanced quotes 3", "\"", cmd.smartUnquote("\""));
+        assertEquals("no quotes", "X", cmd.smartUnquote("X"));
     }
 
     @SuppressWarnings("unchecked")