Flexible query parser: toQueryString prints user@example.com, && and || unescaped, so the printed query does not parse

QueryNode#toQueryString(EscapeQuerySyntax) is documented to "convert to a query string understood by the query parser", and EscapeQuerySyntaxImpl is the escaper for the standard syntax. Since 9.1 (LUCENE-10223) the standard grammar rejects an unescaped @ anywhere in a term, so an e-mail address has to be written user\@example.com (or quoted); && and || have been the AND and OR operators since the flexible parser's first release, 2.9.0. The escaper was never updated for either: it escapes @ only as a term's first character, and its reserved words are AND, OR, NOT but not && or ||.

So the term nodes the parser builds from user\@example.com, \&& and \|| print as df:user@example.com, df:&& and df:||, and parsing any of them again throws INVALID_SYNTAX_CANNOT_PARSE. Of all 866,495 printable-ASCII texts of length 1 to 3, each built as a term node, exactly 18,052 fail this round trip; with a three-line change to three of the escaper's tables, none do. The Query that StandardQueryParser.parse() builds is not affected: only code that turns node trees back into text, or calls the escaper itself, and then parses that text. The && / || half is in every release from 2.9.0 to 10.6.0, the inner-@ half in every release from 9.1.0, and main has both.

1. The parser builds the node. A term token becomes new FieldQueryNode(field, discardEscapeChar(term.image), ...) (StandardSyntaxParser.jj:804), so user\@example.com gives a node whose text is user@example.com.
2. The node prints itself. FieldQueryNode.toQueryString returns field + ":" + getTermEscaped(escaper), or only the escaped term when isDefaultField(field) is true: a null field, the reserved field name _plain, or a node told to ignore fields (FieldQueryNode.java:64-71, QueryNodeImpl.java:35, :206-211); getTermEscaped calls escaper.escape(text, Locale.getDefault(), Type.NORMAL) (:55-58). NORMAL is documented as "for escaping reserved words (like AND) in terms" (EscapeQuerySyntax.java:27-28).
3. The escaper. escape (EscapeQuerySyntaxImpl.java:166-183) handles wildcards and backslashes, then escapeTerm (:75-87) runs escapeChar and escapeWhiteChar and finally compares the whole term with escapableWordTokens (:83-85). escapeChar (:43-62) escapes each character of escapableTermChars (:32-34) at every position, but checks escapableTermExtraFirstChars = {"+", "-", "@"} (:30) against buffer.charAt(0) only (:53-59). escapableTermChars has no @, and escapableWordTokens (:39-41) is AND, OR, NOT, TO, WITHIN, SENTENCE, PARAGRAPH, INORDER.
4. The grammar. _TERM_START_CHAR excludes @ (StandardSyntaxParser.jj:131-133) and _TERM_CHAR adds back only - and + (:134). So + and - are legal inside a term, and escaping them only at the start is right, but an unescaped @ is legal nowhere in a term: it appears only as the min-should-match marker after a group, ( ... )@N (:358).
5. The operators. <AND: ("AND" | "&&")> and <OR: ("OR" | "||")> (:144-145) are declared before <TERM> (:161). For exactly &&, the AND and TERM tokens match the same length and the earlier one wins, so df:&& is a field followed by an operator: a syntax error. &&& and a&&b are longer TERM matches and round-trip.

• \@example.com (control, @ first) prints df:\@example.com and parses back.
• user\@example.com prints df:user@example.com: syntax error. With the fix: df:user\@example.com, parses.
• \&& prints df:&&: syntax error (read as AND). With the fix: df:\&&, parses.
• \|| prints df:||: syntax error (read as OR). With the fix: df:\||, parses.
• Controls: \AND, &&& and a&&b print df:\AND, df:&&& and df:a&&b and parse back. (Unescaped AND is rejected as input: Syntax Error, cannot parse AND:.)

The same nodes come from the public constructor new FieldQueryNode("f", "user@example.com", 0, 0); hand-built nodes with the texts AND, &&& and a&&b print f:\AND, f:&&& and f:a&&b and parse back. Quoted terms are not affected; they use the STRING escaping path.

Every printable-ASCII text of length 1 to 3 (95 + 95^2 + 95^3 = 866,495) was built directly as a term node (FieldQueryNode), printed and parsed again: 18,052 fail, exactly the 95 two-character texts x@, the texts && and ||, and 17,955 three-character texts with @ after the first character. With the fix, 0 fail. An independent reproducer repeated the enumeration and got the same count.

• Who is hit. Code that turns node trees into text and parses it again, for example to store, log or forward a rewritten query, or that escapes words with EscapeQuerySyntaxImpl.escape. The trigger is any term with an inner @ (e-mail addresses, user@host handles) or a literal && / || term, from escaped input or from hand-built nodes.
• Default vs opt-in. Default parser settings and the stock escaper, the only implementation in Lucene. The round trip itself is an uncommon API pattern; StandardQueryParser.parse() never re-parses its own output.
• What goes wrong. For a term printed with a field prefix (df:user@example.com, df:&&, df:||) the printed query is refused with a clean ParseException (INVALID_SYNTAX_CANNOT_PARSE): it fails loudly. A bare || printed without a prefix (a node whose field is null or the reserved name _plain, or a word escaped with EscapeQuerySyntaxImpl.escape and joined into a query) parses again as the OR operator and can change what the query means.
• The three cases differ. An inner @ (e-mail addresses, handles) is the realistic one and fails since 9.1; a literal && or || term is unusual; only || has a shown change of meaning, and only in text printed without a field prefix.
• Inside Lucene escaped output only feeds exception messages (QueryTreeBuilder.java:161, BooleanQueryNodeBuilder.java:72, MatchAllDocsQueryNodeBuilder.java:42, MatchNoDocsQueryNodeBuilder.java:42, AllowLeadingWildcardProcessor.java:74), which then show the term unescaped. IntervalQueryNode.toString() also calls toQueryString(new EscapeQuerySyntaxImpl()) (IntervalQueryNode.java:52-55), but its toQueryString ignores the escaper (:47-50).
• Scale and reach. One term is enough, deterministic. No public report since 9.1.0 (2022) for @ or since 2.9.0 (2009) for && / ||.

Two methods because the bug has two independent halves with different table entries and version ranges; || shares the && table entry and fix line and is left out. The class sits in the parser package, as an upstream test would, but uses no package-private member. It is also the suggested regression test.

Its printed line is one line over the google-java-format limit; ./gradlew :lucene:queryparser:tidy wraps it, nothing else changes, and :lucene:queryparser:check -x test then passes.

The test asserts the escaped text first, then parses it, the same "expected format, then parse" shape as TestAbstractRangeQueryNode. A maintainer who prefers a different parseable form, such as quoting, may want the assertion relaxed to parse-back only; the failure on stock code is the same.

On 9.0.0 and older the first assertion of testAtSignInsideTerm also fails, although df:user@example.com still parses there, because no release has ever escaped an inner @. The 9.1.0 boundary rests on the parse-back runs in the version history.

TestEscapeQuerySyntaxRoundTrip > testAtSignInsideTerm FAILED
    org.junit.ComparisonFailure: expected:<df:user[\]@example.com> but was:<df:user[]@example.com>
        at org.apache.lucene.queryparser.flexible.standard.parser.TestEscapeQuerySyntaxRoundTrip.assertRoundTrip(TestEscapeQuerySyntaxRoundTrip.java:27)
        at org.apache.lucene.queryparser.flexible.standard.parser.TestEscapeQuerySyntaxRoundTrip.testAtSignInsideTerm(TestEscapeQuerySyntaxRoundTrip.java:32)

TestEscapeQuerySyntaxRoundTrip > testAndAndTerm FAILED
    org.junit.ComparisonFailure: expected:<df:[\]&&> but was:<df:[]&&>
        at org.apache.lucene.queryparser.flexible.standard.parser.TestEscapeQuerySyntaxRoundTrip.assertRoundTrip(TestEscapeQuerySyntaxRoundTrip.java:27)
        at org.apache.lucene.queryparser.flexible.standard.parser.TestEscapeQuerySyntaxRoundTrip.testAndAndTerm(TestEscapeQuerySyntaxRoundTrip.java:36)

:lucene:queryparser:test (FAILURE): 2 tests, 2  failures

# re-parsing the unescaped strings (mechanism check on the same commit):
INVALID_SYNTAX_CANNOT_PARSE: Syntax Error, cannot parse df:user@example.com:
INVALID_SYNTAX_CANNOT_PARSE: Syntax Error, cannot parse df:&&:
INVALID_SYNTAX_CANNOT_PARSE: Syntax Error, cannot parse df:||:

• The documentation says the output parses. "convert to a query string understood by the query parser" (QueryNode.java:26); "A query node tree can also generate a query string that can be parsed back by the original text parser, at this point only the standard lucene syntax is supported" (core/nodes/package-info.java:42-43); NORMAL escapes "reserved words (like AND) in terms" (EscapeQuerySyntax.java:27-28). These describe the intended behaviour rather than state a formal round-trip guarantee, but the output here is a syntax error.
• The inner @ must be escaped by design. On #11800 (2022) Robert Muir called the 9.1 @ parse failure "not a bug", an intended grammar change, and Dawid Weiss wrote "You can escape the at character": am\@zing. So the escaped form is the intended syntax, and the escaper should produce it. Neither mentions toQueryString.
• The escaper was kept in step with the grammar once before. LUCENE-3981 (167b296353d, 2012) excluded / from term characters and added / to escapableTermChars in the same commit. LUCENE-10223 (f725b27e12a, #429) added @ to the exclusions and did not touch EscapeQuerySyntaxImpl. So the @ half is a missed follow-up in the escaper; the grammar change itself was intended (#11800).
• The escaper already escapes the word forms (AND prints \AND), and the package's other escaper, QueryParserUtil.escape, escapes & and | (QueryParserUtil.java:172-174), asserted since LUCENE-881 (TestQPHelper.java:923-925).
• Precedent. CHANGES.txt:296-298, Lucene 11.0.0 Bug Fixes: "GITHUB#7865 AbstractRangeQueryNode#toQueryString(EscapeQuerySyntax) now returns a string which can be parsed back into the original node"; TestAbstractRangeQueryNode asserts the "query string should have expected format and parse into a valid query" (:44-47).
• Nothing documents the gap. No javadoc, test, CHANGES or MIGRATE entry says these terms may print unparseable; MIGRATE.md:1116-1122 mentions only "the @ character after parentheses". No upstream test pins the escaper's current output.

The strongest case for "intended" (weak, as every verifier rated it; a later reading-only fact check by another model family rated it moderate): "There has never been a guarantee that going back and forth between a parsed query and its string representation is idempotent" (Erick Erickson, java-user, March 2017, about range nodes), and open #8667 questions toQueryString's design; toQueryString also reads like a display serializer (it takes the escaper as a parameter, and in-tree callers use it for messages).

Why it fails: both quotes are about output that differs from the input, not output that does not parse, and the javadoc describes the output as parseable. In the same 2017 thread Michael McCandless replied "Why don't we fix this in Lucene?", and that line of work ended in the #7865 Bug Fixes entry. A second argument, letting the grammar accept @ inside terms again, reverses the decision on #11800 and leaves && / || unfixed; the escaper fix is the consistent one. One range-node fix (#7865) does not make a general round-trip guarantee, so the claim easiest to accept is the narrow one: FieldQueryNode's term printing is out of step with the current grammar.

One fix was built and checked through gradle in a scratch worktree of 645d4fe14a20; the alternatives come from the verifiers and were not run.

Escape @ at every position, as LUCENE-3981 did for / (a leading @ still prints \@), and treat a term that is exactly && or || like AND: a leading backslash. &&& and a&&b stay as they are. Could add a CHANGES entry under Bug Fixes.

-  private static final String[] escapableTermExtraFirstChars = {"+", "-", "@"};
+  private static final String[] escapableTermExtraFirstChars = {"+", "-"};
   private static final String[] escapableTermChars = {
-    "\"", "<", ">", "=", "!", "(", ")", "^", "[", "{", ":", "]", "}", "~", "/"
+    "\"", "<", ">", "=", "!", "(", ")", "^", "[", "{", ":", "]", "}", "~", "/", "@"
   };
   private static final String[] escapableWordTokens = {
-    "AND", "OR", "NOT", "TO", "WITHIN", "SENTENCE", "PARAGRAPH", "INORDER"
+    "AND", "OR", "NOT", "TO", "WITHIN", "SENTENCE", "PARAGRAPH", "INORDER", "&&", "||"
   };

Trade-offs

• Only the printed text of a value with an inner @ or exactly && / || changes. For a term with a field prefix that text did not parse before and now does. A bare || printed without a prefix (a null or _plain field, or escape() on a single word) was read as OR and now stays a term. Range bounds also go through the escaper, and inside brackets @, && and || already parse: [a@b TO c] and [&& TO zz] now print df:[a\@b TO c] and df:[\&& TO zz] and parse back to the same range query (executed). With the fix the study's exhaustive length 1-3 round trip has 0 failures, and its security enumeration gives the intended term query for all 866,495 texts.
• Exception messages that print nodes now show the escaped text. No upstream test pins the old output.
• Open PR #16853 edits escapeIgnoringCase in the same file; its diff does not touch these tables.

What the checks validated

• Gradle, scratch worktree of 645d4fe14a20, seed DEADBEEF: stock, the repro fails 2 of 2 with assertions on and off; with the fix it passes 2 of 2 with assertions on and off.
• With the fix: TestQPHelper, TestStandardQP and TestAbstractRangeQueryNode 86 tests, 1 skipped (an existing @Ignore), 0 failures; the whole :lucene:queryparser:test 535 tests, 1 skipped, 0 failures; :lucene:queryparser:check -x test passes with the tidied repro test in place.
• Earlier, on the standalone runner: the repro, a parse-back-only test including a Query-level check (5 of 5), the exhaustive length 1-3 check (0 failures) and the security probe.

Add & and | to escapableTermChars, as QueryParserUtil.escape does.

Trade-offs

A simpler rule, consistent with the other helper, but it also changes the output of terms that round-trip today (a&&b would print a\&\&b). Not run.

Add @ to _TERM_CHAR in StandardSyntaxParser.jj.

Trade-offs

Reverses the intended 9.1 change (#11800), needs the JavaCC parser regenerated, and does nothing for && / ||. Not recommended; not run as a fix.

Method: semi-formal, translation validation of the stock main jar (a hunt unit, the flexible, simple and XML query parsers).

• Mode escape: every printable-ASCII text of length 1 to 3 built directly as a term node (FieldQueryNode), printed with toQueryString(new EscapeQuerySyntaxImpl()) and parsed again (the log counts 866,497 checks: two extra texts with non-ASCII whitespace). 18,052 ParseExceptions: the 95 texts x@, &&, || and 17,955 three-character texts with @ after the first character; 0 with the fix.
• Mode syntax: every query string of length up to 5 over a 20-symbol alphabet (3,368,420 strings) was parsed; the 163,434 that parse were printed and parsed again. 1,806 failures fall in the inner-@ class and 266 in the && / || class; 264 more combine an inner @ with an escaped field name, a separate bug.
• A first draft blamed the grammar (an unescaped @ inside a term stopped parsing in 9.1). It was withdrawn after reading #11800, where maintainers called that change intended; the surviving defect is the escaper.
• An AI tool then wrote the public-API JUnit test; an independent reproducer repeated the enumeration with its own code and got the same 18,052.