From 032da69ae336c8d8801562dc4a97d4ab3958d85d Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Mon, 15 Jan 2018 13:32:55 +0100 Subject: [PATCH 1/3] Fix simple_query_string on invalid input 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 --- .../index/search/SimpleQueryStringQueryParser.java | 3 ++- .../index/query/SimpleQueryStringBuilderTests.java | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) 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 9f91b16359287..c60ce36bb3596 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.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; @@ -90,7 +91,7 @@ private Analyzer getAnalyzer(MappedFieldType ft) { */ private Query rethrowUnlessLenient(RuntimeException e) { if (settings.lenient()) { - return null; + return Queries.newMatchNoDocsQuery("failed query, caused by " + e.getMessage()); } throw e; } 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 bfc6fd0600493..07032c61e24c4 100644 --- a/server/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java @@ -55,6 +55,7 @@ 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 +608,19 @@ public void testToFuzzyQuery() throws Exception { 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) + .lenient(true) + .toQuery(createShardContext()); + MatchNoDocsQuery expected = new MatchNoDocsQuery(""); + assertEquals(expected, query); + assertThat(query.toString(), + containsString("Can only use prefix queries on keyword and text fields - not on [mapped_date] which is of type [date]")); + } + private static IndexMetaData newIndexMeta(String name, Settings oldIndexSettings, Settings indexSettings) { Settings build = Settings.builder().put(oldIndexSettings) .put(indexSettings) From c7b809129f3ed8e3f5b4930568fb536a45ecf6fa Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Mon, 15 Jan 2018 17:41:32 +0100 Subject: [PATCH 2/3] we should not fail on exceptions if lenient is true --- .../index/search/SimpleQueryStringQueryParser.java | 8 ++++---- .../index/query/SimpleQueryStringBuilderTests.java | 10 +++++++--- 2 files changed, 11 insertions(+), 7 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 c60ce36bb3596..6129bf006ba6d 100644 --- a/server/src/main/java/org/elasticsearch/index/search/SimpleQueryStringQueryParser.java +++ b/server/src/main/java/org/elasticsearch/index/search/SimpleQueryStringQueryParser.java @@ -116,7 +116,7 @@ public Query newDefaultQuery(String text) { 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())); } } @@ -136,7 +136,7 @@ public Query newFuzzyQuery(String text, int fuzziness) { settings.fuzzyMaxExpansions, settings.fuzzyTranspositions); disjuncts.add(wrapWithBoost(query, entry.getValue())); } catch (RuntimeException e) { - rethrowUnlessLenient(e); + disjuncts.add(rethrowUnlessLenient(e)); } } if (disjuncts.size() == 1) { @@ -157,7 +157,7 @@ public Query newPhraseQuery(String text, int slop) { } return queryBuilder.parse(MultiMatchQueryBuilder.Type.PHRASE, phraseWeights, text, null); } catch (IOException e) { - return rethrowUnlessLenient(new IllegalArgumentException(e.getMessage())); + throw new IllegalStateException(e.getMessage()); } finally { queryBuilder.setPhraseSlop(0); } @@ -185,7 +185,7 @@ public Query newPrefixQuery(String text) { 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 07032c61e24c4..dc7c56ce04ebf 100644 --- a/server/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java @@ -46,10 +46,12 @@ 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; @@ -613,12 +615,14 @@ public void testLenientToPrefixQuery() throws Exception { Query query = new SimpleQueryStringBuilder("t*") .field(DATE_FIELD_NAME) + .field(STRING_FIELD_NAME) .lenient(true) .toQuery(createShardContext()); - MatchNoDocsQuery expected = new MatchNoDocsQuery(""); + List 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); - assertThat(query.toString(), - containsString("Can only use prefix queries on keyword and text fields - not on [mapped_date] which is of type [date]")); } private static IndexMetaData newIndexMeta(String name, Settings oldIndexSettings, Settings indexSettings) { From 00dd99d266ad221b0f18b2a16a23affc65f73184 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Wed, 17 Jan 2018 09:35:28 +0100 Subject: [PATCH 3/3] address review --- .../index/search/SimpleQueryStringQueryParser.java | 4 ++-- 1 file changed, 2 insertions(+), 2 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 6129bf006ba6d..aea3677e33e13 100644 --- a/server/src/main/java/org/elasticsearch/index/search/SimpleQueryStringQueryParser.java +++ b/server/src/main/java/org/elasticsearch/index/search/SimpleQueryStringQueryParser.java @@ -87,7 +87,7 @@ private Analyzer getAnalyzer(MappedFieldType ft) { } /** - * 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()) { @@ -157,7 +157,7 @@ public Query newPhraseQuery(String text, int slop) { } return queryBuilder.parse(MultiMatchQueryBuilder.Type.PHRASE, phraseWeights, text, null); } catch (IOException e) { - throw new IllegalStateException(e.getMessage()); + return rethrowUnlessLenient(new IllegalStateException(e.getMessage())); } finally { queryBuilder.setPhraseSlop(0); }