Escape reserved words and special characters in wildcard phrase queries on tokenised fields

repository/src/main/java/org/alfresco/repo/search/impl/elasticsearch/query/language/lucene/LuceneQueryParser.java

@@ -166,6 +166,7 @@ public class LuceneQueryParser extends QueryParser
             FIELD_LOCALE_SUFFIX);
 
     private static final List<String> ES_RESERVED_WORDS = List.of("AND", "OR", "NOT");
+    private static final String WILDCARD_ONLY_TERM = "*";
 
     private final NamespacePrefixResolver namespaceResolver;
     private final DictionaryService dictionaryService;
@@ -356,6 +357,10 @@ public class LuceneQueryParser extends QueryParser
             // to split the terms to be able to use the wildcard in the term
             this.setDefaultOperator(Operator.AND);
             quoted = false;
+
+            // Since the phrase is no longer quoted, characters and words which are only special at the
+            // beginning of a term have to be escaped in every term and not just in the first one
+            queryText = sanitizeTerms(queryText);
         }
 
         return propertyFieldQuery(fieldName, queryText, quoted, exactTermSearch, untokenisedSearch);
@@ -1144,6 +1149,29 @@ public class LuceneQueryParser extends QueryParser
                 .replace("\"", "\\\"");
     }
 
+    /**
+     * Prepares the individual terms of a phrase query text which has already been escaped as a whole. To be used when we split the phrase into individual terms for the purpose of building a boolean query. Of the remaining terms only the reserved words and the leading "-" and "+" are escaped here, as the rest have been already escaped in the phrase escape.
+     */
+    String sanitizeTerms(String queryText)
+    {
+        var terms = Stream.of(queryText.split(" ", -1))
+                .filter(not(WILDCARD_ONLY_TERM::equals))
+                .map(this::escapeTermPrefix)
+                .collect(toList());
+
+        return terms.isEmpty() ? WILDCARD_ONLY_TERM : String.join(" ", terms);
+    }
+
+    private String escapeTermPrefix(String term)
+    {
+        if (ES_RESERVED_WORDS.contains(term) || term.startsWith("-") || term.startsWith("+"))
+        {
+            return "\\" + term;
+        }
+
+        return term;
+    }
+
     private Query unsupportedWithMessage(String message)
     {
         logIgnoringField(message);

repository/src/test/java/org/alfresco/repo/search/impl/elasticsearch/query/BaseWildcardQueryIT.java

@@ -49,6 +49,9 @@ public abstract class BaseWildcardQueryIT extends ElasticsearchBaseQueryIT
     protected NodeRef goslingDocument;
     protected NodeRef swimmingDocument;
     protected NodeRef supersizemyrepoDocument;
+    protected NodeRef specialCharTokenizedFieldDocument;
+    protected NodeRef specialCharUntokenizedFieldDocument;
+    protected NodeRef reservedWordsFieldDocument;
 
     @Before
     public void initDocuments() throws Exception
@@ -83,6 +86,21 @@ public abstract class BaseWildcardQueryIT extends ElasticsearchBaseQueryIT
                 .withContent("Swimming every morning is great exercise"));
         supersizemyrepoDocument = indexDocument(new IndexDocumentSourceBuilder().withName("supersizemyrepoDocument")
                 .withContent("supersizemyrepo tool helps with repository management"));
+
+        Map<String, Object> specialCharTokenisedPropertiesText = Map.of("acme:contractTokenisedField", "To be, or not to be - that is the question ( Hamlet, Act 3*, Scene 1 )");
+        Map<String, Object> specialCharUntokenisedPropertiesText = Map.of("acme:contractUntokenisedField", "Tomorrow, and tomorrow, and tomorrow - creeps in this petty pace. ( Macbeth, Act 5, Scene 5 )*");
+
+        specialCharTokenizedFieldDocument = indexDocument(new IndexDocumentSourceBuilder().withName("documentWithSpecialCharsInTokenisedText")
+                .withContent("content").withAdditionalProperties(specialCharTokenisedPropertiesText));
+
+        specialCharUntokenizedFieldDocument = indexDocument(new IndexDocumentSourceBuilder().withName("documentWithSpecialCharsInUntokenisedText")
+                .withContent("content").withAdditionalProperties(specialCharUntokenisedPropertiesText));
+
+        Map<String, Object> reservedWordsPropertiesText = Map.of("acme:contractTokenisedField",
+                "Enter Romeo AND Juliet NOT Hamlet OR Macbeth + Othello - Lear");
+
+        reservedWordsFieldDocument = indexDocument(new IndexDocumentSourceBuilder().withName("documentWithReservedWordsInTokenisedText")
+                .withContent("content").withAdditionalProperties(reservedWordsPropertiesText));
     }
 
     /* See https://alfresco.atlassian.net/browse/SEARCH-2862 for wildcards in phrase queries */
@@ -179,4 +197,13 @@ public abstract class BaseWildcardQueryIT extends ElasticsearchBaseQueryIT
 
     @Test
     public abstract void nonStemmedContentWildcardSuffixSearch();
+
+    @Test
+    public abstract void specialCharsUntokenisedFieldWildcardQuerySearch();
+
+    @Test
+    public abstract void specialCharsTokenisedFieldWildcardQuerySearch();
+
+    @Test
+    public abstract void reservedWordsTokenisedFieldWildcardQuerySearch();
 }

repository/src/test/java/org/alfresco/repo/search/impl/elasticsearch/query/language/afts/WildcardQueryIT.java

@@ -349,4 +349,55 @@ public class WildcardQueryIT extends BaseWildcardQueryIT
         assertContainsOnly(aftsSearch("TEXT:\"supersizemy*\""), supersizemyrepoDocument);
         assertContainsOnly(aftsSearch("TEXT:\"supersize*\""), supersizemyrepoDocument);
     }
+
+    @Override
+    public void specialCharsUntokenisedFieldWildcardQuerySearch()
+    {
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractUntokenisedField:\"Tomorrow, and tomorrow, and tomorrow - creeps *\""),
+                specialCharUntokenizedFieldDocument);
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractUntokenisedField:\"Tomorrow, and tomorrow, and tomorrow - creeps in this petty pace. ( *\""),
+                specialCharUntokenizedFieldDocument);
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractUntokenisedField:\"Tomorrow, and tomorrow, and tomorrow *\""),
+                specialCharUntokenizedFieldDocument);
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractUntokenisedField:\"Tomorrow, * - creeps in this petty pace. ( Macbeth, Act 5, Scene 5 )\\\\*\""),
+                specialCharUntokenizedFieldDocument);
+    }
+
+    @Override
+    public void specialCharsTokenisedFieldWildcardQuerySearch()
+    {
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractTokenisedField:\"To be, or not to be - that *\""),
+                specialCharTokenizedFieldDocument);
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractTokenisedField:\"To be, or not to be - that is the question ( *\""),
+                specialCharTokenizedFieldDocument);
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractTokenisedField:\"To be, or not to be *\""),
+                specialCharTokenizedFieldDocument);
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractTokenisedField:\"To be, or not to be - that is the question ( Hamlet, Act 3\\\\*, Scene *\""),
+                specialCharTokenizedFieldDocument);
+    }
+
+    @Override
+    public void reservedWordsTokenisedFieldWildcardQuerySearch()
+    {
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractTokenisedField:\"Enter Romeo AND *\""),
+                reservedWordsFieldDocument);
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractTokenisedField:\"Enter Romeo AND Juliet NOT Hamlet OR *\""),
+                reservedWordsFieldDocument);
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractTokenisedField:\"Enter Romeo AND Juliet NOT Hamlet OR Macbeth + Othello *\""),
+                reservedWordsFieldDocument);
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractTokenisedField:\"Enter * + Othello - Lear\""),
+                reservedWordsFieldDocument);
+    }
 }

repository/src/test/java/org/alfresco/repo/search/impl/elasticsearch/query/language/lucene/EscapeCharacterTest.java

@@ -53,6 +53,63 @@ public class EscapeCharacterTest
         assertEquals("\\\\(", parser.escapeSpecialCharacters("\\("));
     }
 
+    /**
+     * sanitizeTerms() receives a query text which escape() has already escaped as a whole, so it only has to take care of what is special at the beginning of a term.
+     */
+    @Test
+    public void sanitizeTermsEscapesEveryTermPrefixTest()
+    {
+        // "-" and "+" are escaped in the terms which escape() left untouched
+        assertEquals("foo \\-bar", parser.sanitizeTerms("foo -bar"));
+        assertEquals("foo \\+bar", parser.sanitizeTerms("foo +bar"));
+        assertEquals("a* \\-b \\+c", parser.sanitizeTerms("a* -b +c"));
+
+        // reserved words are escaped wherever they appear
+        assertEquals("foo \\AND bar*", parser.sanitizeTerms("foo AND bar*"));
+        assertEquals("\\NOT \\OR", parser.sanitizeTerms("NOT OR"));
+
+        // a single term, or a term where the character isn't leading, is left alone
+        assertEquals("single*", parser.sanitizeTerms("single*"));
+        assertEquals("foo a-b c+d", parser.sanitizeTerms("foo a-b c+d"));
+        assertEquals("foo ANDROID", parser.sanitizeTerms("foo ANDROID"));
+    }
+
+    /**
+     * A term which is only a wildcard contributes nothing to the split query, as Elasticsearch reads "field:*" as an existence check.
+     */
+    @Test
+    public void sanitizeTermsDropsWildcardOnlyTermsTest()
+    {
+        assertEquals("apple banana", parser.sanitizeTerms("apple banana *"));
+        assertEquals("apple banana", parser.sanitizeTerms("apple * banana"));
+        assertEquals("apple banana", parser.sanitizeTerms("* apple * banana *"));
+
+        // a wildcard which is part of a term, or an escaped one, is not a wildcard-only term
+        assertEquals("apple ban*", parser.sanitizeTerms("apple ban*"));
+        assertEquals("apple \\*", parser.sanitizeTerms("apple \\*"));
+
+        // "?" and "%" are not dropped: "field:?" is a real single character wildcard and "%" reaches Elasticsearch as a literal
+        assertEquals("apple ? %", parser.sanitizeTerms("apple ? %"));
+
+        // dropping must never leave an empty query text
+        assertEquals("*", parser.sanitizeTerms("*"));
+        assertEquals("*", parser.sanitizeTerms("* *"));
+    }
+
+    /**
+     * The special characters escape() escapes in any position have already been escaped in every term, so sanitizeTerms() must not escape them again.
+     */
+    @Test
+    public void sanitizeTermsDoesNotDoubleEscapeTest()
+    {
+        assertEquals("foo \\(bar\\) \\-baz*", parser.sanitizeTerms(parser.escape("foo (bar) -baz*", true)));
+        assertEquals("a\\:b \\-c\\(d\\)* e", parser.sanitizeTerms(parser.escape("a:b -c(d)* e", true)));
+        assertEquals("path\\/to \\-file\\[1\\]*", parser.sanitizeTerms(parser.escape("path/to -file[1]*", true)));
+
+        // the leading "-" of the whole query text has been escaped by escape() already
+        assertEquals("\\-lead* trail\\~", parser.sanitizeTerms(parser.escape("-lead* trail~", true)));
+    }
+
     @SuppressWarnings("PMD.TestClassWithoutTestCases")
     protected class LuceneQueryParserUnderTest extends LuceneQueryParser
     {

repository/src/test/java/org/alfresco/repo/search/impl/elasticsearch/query/language/lucene/WildcardQueryIT.java

@@ -349,4 +349,55 @@ public class WildcardQueryIT extends BaseWildcardQueryIT
         assertContainsOnly(aftsSearch("TEXT:\"supersizemy*\""), supersizemyrepoDocument);
         assertContainsOnly(aftsSearch("TEXT:\"supersize*\""), supersizemyrepoDocument);
     }
+
+    @Override
+    public void specialCharsUntokenisedFieldWildcardQuerySearch()
+    {
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractUntokenisedField:\"Tomorrow, and tomorrow, and tomorrow - creeps *\""),
+                specialCharUntokenizedFieldDocument);
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractUntokenisedField:\"Tomorrow, and tomorrow, and tomorrow - creeps in this petty pace. ( *\""),
+                specialCharUntokenizedFieldDocument);
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractUntokenisedField:\"Tomorrow, and tomorrow, and tomorrow *\""),
+                specialCharUntokenizedFieldDocument);
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractUntokenisedField:\"Tomorrow, * - creeps in this petty pace. ( Macbeth, Act 5, Scene 5 )\\\\*\""),
+                specialCharUntokenizedFieldDocument);
+    }
+
+    @Override
+    public void specialCharsTokenisedFieldWildcardQuerySearch()
+    {
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractTokenisedField:\"To be, or not to be - that *\""),
+                specialCharTokenizedFieldDocument);
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractTokenisedField:\"To be, or not to be - that is the question ( *\""),
+                specialCharTokenizedFieldDocument);
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractTokenisedField:\"To be, or not to be *\""),
+                specialCharTokenizedFieldDocument);
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractTokenisedField:\"To be, or not to be - that is the question ( Hamlet, Act 3\\\\*, Scene *\""),
+                specialCharTokenizedFieldDocument);
+    }
+
+    @Override
+    public void reservedWordsTokenisedFieldWildcardQuerySearch()
+    {
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractTokenisedField:\"Enter Romeo AND *\""),
+                reservedWordsFieldDocument);
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractTokenisedField:\"Enter Romeo AND Juliet NOT Hamlet OR *\""),
+                reservedWordsFieldDocument);
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractTokenisedField:\"Enter Romeo AND Juliet NOT Hamlet OR Macbeth + Othello *\""),
+                reservedWordsFieldDocument);
+        assertContainsOnly(aftsSearch(
+                "acme\\:contractTokenisedField:\"Enter * + Othello - Lear\""),
+                reservedWordsFieldDocument);
+    }
 }