DefaultCommandParser mis-tokenizes input containing escaped quotes (regression from 3.x)

[asgoth]

DefaultCommandParser mis-tokenizes input containing escaped quotes (regression from 3.x)

Spring Shell version: 4.0.3 (also present in 4.0.1)
Component: org.springframework.shell.core.command.DefaultCommandParser

Summary

DefaultCommandParser.parse(String) tokenizes the raw input line with a single regular
expression:

List<String> words = List.of(input.split("\\s+(?=(?:[^\"]*\"[^\"]*\")*[^\"]*$)"));

The look-ahead only splits on whitespace when the remainder of the line contains an even
number of " characters. It has no notion of escaping, so a \" inside a quoted value is
counted as a quote just like a real delimiter. As soon as a quoted argument contains an odd
number of escaped quotes, the line is split in the wrong place (or not at all), the first
token is no longer the command name, and the shell reports:

Command <garbage> not found.

This is a regression: in Spring Shell 3.x tokenization was done by a real, escape-aware
lexer (org.springframework.shell.jline.ExtendedDefaultParser for the interactive runner and
JLine's DefaultParser for the non-interactive runner), both configured with
escapeChars = { '\\' } and quoteChars = { '\'', '"' }. The same input worked fine there.

Affected versions

• Works: 3.x
• Broken: 4.0.1, 4.0.3 (regex-based splitting introduced with the new DefaultCommandParser)

Reproducer

A minimal fictitious app with one command taking a single string argument:

@Command
class GreetCommands {

    @Command(command = "greet")
    public String greet(String message) {
        return message;
    }
}

Now run:

shell:> greet "she said \" and left"
Command greet "she not found.

Expected: the command is invoked with message = she said " and left.

Reproducer without any Spring Shell dependency

The problem is entirely in the splitting expression, so it can be shown in isolation:

import java.util.List;

public class SplitDemo {

    private static final String SPLIT = "\\s+(?=(?:[^\"]*\"[^\"]*\")*[^\"]*$)";

    public static void main(String[] args) {
        show("greet \"plain text\"");             // ok
        show("greet \"say \\\"hi\\\" now\"");     // ok by accident (even number of escaped quotes)
        show("greet \"say \\\" now\"");           // BROKEN (odd number of escaped quotes)
    }

    private static void show(String input) {
        List<String> words = List.of(input.split(SPLIT));
        System.out.println("input        : " + input);
        System.out.println("tokens       : " + words);
        System.out.println("command name : " + words.get(0));
        System.out.println();
    }
}

Output:

input        : greet "plain text"
tokens       : [greet, "plain text"]
command name : greet

input        : greet "say \"hi\" now"
tokens       : [greet, "say \"hi\" now"]
command name : greet

input        : greet "say \" now"
tokens       : [greet "say, \" now"]
command name : greet "say

The third case shows the failure mode: the split happens inside the quoted value instead of
after the command name, so greet "say becomes the command name and lookup fails.

Note the inconsistency: an even number of escaped quotes inside the value happens to work,
an odd number does not. Users therefore see the command work for some values and fail for
others, with an error message that gives no hint about quoting.

Practical impact

Any command that accepts free-form text where a " may legitimately occur is affected, for
example a command that forwards a query/expression to another engine:

shell:> query "select replace(agg(code), '\"', '') from items"
Command query "select not found.

There is no way to express such a value: escaping is ignored by the splitter, and single
quotes are not treated as quoting characters in 4.x (they were in 3.x via JLine's
quoteChars = { '\'', '"' }), so they cannot be used as an alternative either.

Additional related shortcomings of the regex approach

Besides escaped quotes, the regex-based split differs from the 3.x lexer in a few other ways
that are probably worth addressing in the same fix:

1. No single-quote support. greet 'hello world' is split into two tokens; 3.x accepted
   ' as a quoting character.
2. Quotes only handled when they enclose a whole token. unquoteAndUnescapeQuoted only
   strips quotes when the token both starts and ends with ", so --message="a b" style
   values or partially quoted tokens such as pre"a b"post are not handled consistently.
3. Unbalanced quotes silently change tokenization for the rest of the line rather than
   producing a clear error such as "unbalanced quotes in input".

Suggested fix

Replace the regex split with a proper single-pass tokenizer that walks the input character by
character and tracks quoting/escaping state, i.e. restore the semantics of the 3.x lexer:

• \ escapes the next character (in particular \" yields a literal " and never opens or
  closes a quoted section);
• both " and ' open/close a quoted section, and whitespace inside a quoted section does not
  split;
• quotes are removed from the resulting token value, and escapes are resolved once, during
  tokenization (so unquoteAndUnescapeQuoted is no longer needed downstream);
• unbalanced quotes result in an explicit, actionable error.

Reusing JLine's parser is not desirable in spring-shell-core (it must stay JLine-free), but
the ~40 lines of state-machine tokenization can live in DefaultCommandParser itself.

Alternative fix: make the 3.x tokenization available as an option

If changing the default tokenization is considered too risky for the 4.x line, an acceptable
alternative would be to keep the current behaviour as the default but let applications opt in
to the 3.x (JLine ExtendedDefaultParser-style, escape- and quote-aware) tokenization through
configuration, instead of forcing every affected application to replace the CommandParser
bean altogether.

Concretely:

• Extract the tokenization step out of DefaultCommandParser.parse into a small strategy, for
  example an InputTokenizer (or simply Function<String, List<String>>) with two provided
  implementations: the current RegexInputTokenizer and an escape/quote-aware
  QuotedInputTokenizer reproducing the 3.x semantics. DefaultCommandParser would gain a
  constructor taking the tokenizer, defaulting to the current one for backwards compatibility.

• Expose it as a property under the existing spring.shell namespace
  (SpringShellProperties, @ConfigurationProperties(prefix = "spring.shell")), e.g.:

  # regex (default, current 4.x behaviour) | quoted (3.x behaviour)
  spring.shell.command.tokenizer=quoted

  and have ShellRunnerAutoConfiguration#commandParser select the implementation accordingly.

• Optionally also allow a custom InputTokenizer bean, which is a much smaller and safer
  extension point than replacing the whole CommandParser: applications keep all option/
  argument/sub-command parsing logic of the framework and only influence how the raw line is
  split.

This has the advantage that applications migrating from 3.x can restore the behaviour they
relied on with a single property, and that the fix does not change behaviour for existing 4.x
applications. The disadvantage, of course, is that the default remains a tokenizer that
produces Command ... not found for perfectly reasonable input, so making quoted the
default in the next minor/major would still be desirable.

Workaround

Until this is fixed, applications have to supply their own CommandParser bean (the
auto-configured DefaultCommandParser is @ConditionalOnMissingBean) that special-cases the
affected commands and takes the remainder of the line verbatim, which defeats the purpose of
the framework providing the parsing.

[czpilar]

I can confirm the regression - I went with the primary approach suggested here (character-by-character state machine), see PR #1375.

Before (main) / after (the PR branch):

A few notes from digging into the history:

The 2.x reference docs had an explicit "Quotes Handling" section: both single and double quotes were supported and stripped from the value, with backslash escaping (echo "He said "Hi!"", echo 'I'm here!'). In 3.x that section was lost in the docs migration, but the behavior remained because tokenization was done by JLine's DefaultParser. The 4.x rewrite replaced it with the regex-based splitting, which is where all four problems described here come from. I found no indication anywhere (docs, migration guide, commits) that dropping single quote support was intentional.
The completion layer (ExtendedDefaultParser in spring-shell-jline, a copy of JLine's DefaultParser) still understands both quote types and backslash escaping, so until now completion and execution tokenized the same line differently. The fix makes the two consistent again.

While verifying the fix end to end I found a second, independent part of this bug: JLine's LineReaderImpl.finish() strips all escape characters from the line returned by readLine() unless Option.DISABLE_EVENT_EXPANSION is set. So in interactive mode the backslash in greet "she said " and left" never even reached the command parser - the parser received greet "she said " and left", which is exactly why the reported error message is Command greet "she not found. In 3.x this did not matter because the shell consumed the words of JLine's ParsedLine instead of the returned raw line. The PR disables event expansion on the LineReader so that escape resolution is done in one place, by the command parser. The trade-off is that JLine history expansion (!!, !n) is disabled - it was never documented as a Spring Shell feature, and its expansion pass is what eats the backslashes.

What the fix does:

Replaces the splitting regex in DefaultCommandParser with a state machine that tracks the currently open quote and treats a backslash-escaped character as ordinary content, so greet "she said " and left" tokenizes correctly regardless of the number of escaped quotes.
Restores single quote support (greet 'a b', --option='a b'), matching the documented 2.x/3.x behavior.
Partially-quoted tokens like --message="a b" keep working, and quote characters in the middle of a word are still kept verbatim (existing 4.x behavior, covered by the current test suite - all existing tests pass unchanged).
Unbalanced quotes are rejected with a clear IllegalArgumentException ("Unbalanced quote (...) in input: ...") instead of producing a mis-parsed command. In interactive mode an unclosed quote keeps the shell-style continuation prompt (dquote>), same as 3.x.

One behavior change worth calling out: a bare apostrophe in an unquoted word (e.g. greet don't) now opens a quote, same as shells and the 3.x JLine parser - users can write don't or quote the value. I did not restore escape resolution outside quotes from 2.x (This\ is\ a\ value), since the current 4.x test suite explicitly pins the opposite behavior and resolving backslashes outside quotes would hurt Windows paths.

The PR also adds a "Quoting and escaping" section to the command syntax docs, since the behavior is currently undocumented.

I didn't go with the alternative InputTokenizer strategy property - the old regex behavior is strictly a subset of bugs rather than a mode someone would opt into, and custom parsing is already possible by providing a custom CommandParser bean.