From c38c12e3bf6622d2877e5c3137773981f10114dd Mon Sep 17 00:00:00 2001
From: Jim Ferenczi <jim.ferenczi@elastic.co>
Date: Thu, 18 Jan 2018 10:49:34 +0100
Subject: [PATCH] Fix simple_query_string on invalid input (#28219)

This change converts any exception that occurs during the parsing of
a simple_query_string to a match_no_docs query (instead of a null query)
when leniency is activated.

Closes #28204
---
 .../search/SimpleQueryStringQueryParser.java   | 13 +++++++------
 .../query/SimpleQueryStringBuilderTests.java   | 18 ++++++++++++++++++
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/server/src/main/java/org/elasticsearch/index/search/SimpleQueryStringQueryParser.java b/server/src/main/java/org/elasticsearch/index/search/SimpleQueryStringQueryParser.java
index 9f91b163592..aea3677e33e 100644
--- a/server/src/main/java/org/elasticsearch/index/search/SimpleQueryStringQueryParser.java
+++ b/server/src/main/java/org/elasticsearch/index/search/SimpleQueryStringQueryParser.java
@@ -33,6 +33,7 @@ import org.apache.lucene.search.PrefixQuery;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.SynonymQuery;
 import org.apache.lucene.util.BytesRef;
+import org.elasticsearch.common.lucene.search.Queries;
 import org.elasticsearch.common.unit.Fuzziness;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.query.AbstractQueryBuilder;
@@ -86,11 +87,11 @@ public class SimpleQueryStringQueryParser extends SimpleQueryParser {
     }
 
     /**
-     * Rethrow the runtime exception, unless the lenient flag has been set, returns null
+     * Rethrow the runtime exception, unless the lenient flag has been set, returns {@link MatchNoDocsQuery}
      */
     private Query rethrowUnlessLenient(RuntimeException e) {
         if (settings.lenient()) {
-            return null;
+            return Queries.newMatchNoDocsQuery("failed query, caused by " + e.getMessage());
         }
         throw e;
     }
@@ -115,7 +116,7 @@ public class SimpleQueryStringQueryParser extends SimpleQueryParser {
         try {
             return queryBuilder.parse(MultiMatchQueryBuilder.Type.MOST_FIELDS, weights, text, null);
         } catch (IOException e) {
-            return rethrowUnlessLenient(new IllegalArgumentException(e.getMessage()));
+            return rethrowUnlessLenient(new IllegalStateException(e.getMessage()));
         }
     }
 
@@ -135,7 +136,7 @@ public class SimpleQueryStringQueryParser extends SimpleQueryParser {
                     settings.fuzzyMaxExpansions, settings.fuzzyTranspositions);
                 disjuncts.add(wrapWithBoost(query, entry.getValue()));
             } catch (RuntimeException e) {
-                rethrowUnlessLenient(e);
+                disjuncts.add(rethrowUnlessLenient(e));
             }
         }
         if (disjuncts.size() == 1) {
@@ -156,7 +157,7 @@ public class SimpleQueryStringQueryParser extends SimpleQueryParser {
             }
             return queryBuilder.parse(MultiMatchQueryBuilder.Type.PHRASE, phraseWeights, text, null);
         } catch (IOException e) {
-            return rethrowUnlessLenient(new IllegalArgumentException(e.getMessage()));
+            return rethrowUnlessLenient(new IllegalStateException(e.getMessage()));
         } finally {
             queryBuilder.setPhraseSlop(0);
         }
@@ -184,7 +185,7 @@ public class SimpleQueryStringQueryParser extends SimpleQueryParser {
                     disjuncts.add(wrapWithBoost(query, entry.getValue()));
                 }
             } catch (RuntimeException e) {
-                return rethrowUnlessLenient(e);
+                disjuncts.add(rethrowUnlessLenient(e));
             }
         }
         if (disjuncts.size() == 1) {
diff --git a/server/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java
index bfc6fd06004..dc7c56ce04e 100644
--- a/server/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java
+++ b/server/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java
@@ -46,15 +46,18 @@ import org.elasticsearch.search.internal.SearchContext;
 import org.elasticsearch.test.AbstractQueryTestCase;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
 
 import static org.hamcrest.Matchers.anyOf;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.either;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
@@ -607,6 +610,21 @@ public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase<SimpleQ
         assertEquals(expected, query);
     }
 
+    public void testLenientToPrefixQuery() throws Exception {
+        assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length > 0);
+
+        Query query = new SimpleQueryStringBuilder("t*")
+            .field(DATE_FIELD_NAME)
+            .field(STRING_FIELD_NAME)
+            .lenient(true)
+            .toQuery(createShardContext());
+        List<Query> expectedQueries = new ArrayList<>();
+        expectedQueries.add(new MatchNoDocsQuery(""));
+        expectedQueries.add(new PrefixQuery(new Term(STRING_FIELD_NAME, "t")));
+        DisjunctionMaxQuery expected = new DisjunctionMaxQuery(expectedQueries, 1.0f);
+        assertEquals(expected, query);
+    }
+
     private static IndexMetaData newIndexMeta(String name, Settings oldIndexSettings, Settings indexSettings) {
         Settings build = Settings.builder().put(oldIndexSettings)
             .put(indexSettings)