From 6d8fff65dcfde1b2d0b235707c189f07ac64ae34 Mon Sep 17 00:00:00 2001 From: Britta Weber Date: Tue, 8 Jul 2014 19:37:20 +0200 Subject: [PATCH] Throw exception if function in function score query is null closes #6292 #6784 --- .../FunctionScoreQueryBuilder.java | 10 ++ .../FunctionScoreQueryParser.java | 3 + .../DecayFunctionScoreTests.java | 95 +++++++++++++++++++ 3 files changed, 108 insertions(+) diff --git a/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilder.java b/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilder.java index b804c9e14c9..4826984d74b 100644 --- a/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilder.java +++ b/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilder.java @@ -19,6 +19,7 @@ package org.elasticsearch.index.query.functionscore; +import org.elasticsearch.ElasticsearchIllegalArgumentException; import org.elasticsearch.common.lucene.search.function.CombineFunction; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.index.query.BaseQueryBuilder; @@ -66,6 +67,9 @@ public class FunctionScoreQueryBuilder extends BaseQueryBuilder implements Boost } public FunctionScoreQueryBuilder(ScoreFunctionBuilder scoreFunctionBuilder) { + if (scoreFunctionBuilder == null) { + throw new ElasticsearchIllegalArgumentException("function_score: function must not be null"); + } queryBuilder = null; filterBuilder = null; this.filters.add(null); @@ -73,12 +77,18 @@ public class FunctionScoreQueryBuilder extends BaseQueryBuilder implements Boost } public FunctionScoreQueryBuilder add(FilterBuilder filter, ScoreFunctionBuilder scoreFunctionBuilder) { + if (scoreFunctionBuilder == null) { + throw new ElasticsearchIllegalArgumentException("function_score: function must not be null"); + } this.filters.add(filter); this.scoreFunctions.add(scoreFunctionBuilder); return this; } public FunctionScoreQueryBuilder add(ScoreFunctionBuilder scoreFunctionBuilder) { + if (scoreFunctionBuilder == null) { + throw new ElasticsearchIllegalArgumentException("function_score: function must not be null"); + } this.filters.add(null); this.scoreFunctions.add(scoreFunctionBuilder); return this; diff --git a/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryParser.java b/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryParser.java index 6272a955e51..3bd5a05b41c 100644 --- a/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryParser.java +++ b/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryParser.java @@ -195,6 +195,9 @@ public class FunctionScoreQueryParser implements QueryParser { if (filter == null) { filter = Queries.MATCH_ALL_FILTER; } + if (scoreFunction == null) { + throw new ElasticsearchParseException("function_score: One entry in functions list is missing a function."); + } filterFunctions.add(new FiltersFunctionScoreQuery.FilterFunction(filter, scoreFunction)); } diff --git a/src/test/java/org/elasticsearch/search/functionscore/DecayFunctionScoreTests.java b/src/test/java/org/elasticsearch/search/functionscore/DecayFunctionScoreTests.java index ae71f430d73..93d4c91e6ff 100644 --- a/src/test/java/org/elasticsearch/search/functionscore/DecayFunctionScoreTests.java +++ b/src/test/java/org/elasticsearch/search/functionscore/DecayFunctionScoreTests.java @@ -19,16 +19,20 @@ package org.elasticsearch.search.functionscore; +import org.elasticsearch.ElasticsearchIllegalArgumentException; import org.elasticsearch.ElasticsearchIllegalStateException; +import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.action.ActionFuture; import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.search.SearchPhaseExecutionException; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.search.SearchType; +import org.elasticsearch.action.search.ShardSearchFailure; import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.lucene.search.function.CombineFunction; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.index.query.FilterBuilders; import org.elasticsearch.index.query.MatchAllFilterBuilder; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.index.query.functionscore.DecayFunctionBuilder; @@ -38,6 +42,7 @@ import org.elasticsearch.test.ElasticsearchIntegrationTest; import org.joda.time.DateTime; import org.junit.Test; +import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -843,4 +848,94 @@ public class DecayFunctionScoreTests extends ElasticsearchIntegrationTest { } } + // issue https://github.com/elasticsearch/elasticsearch/issues/6292 + @Test + public void testMissingFunctionThrowsElasticsearchParseException() throws IOException { + + // example from issue https://github.com/elasticsearch/elasticsearch/issues/6292 + String doc = "{\n" + + " \"text\": \"baseball bats\"\n" + + "}\n"; + + String query = "{\n" + + " \"function_score\": {\n" + + " \"score_mode\": \"sum\",\n" + + " \"boost_mode\": \"replace\",\n" + + " \"functions\": [\n" + + " {\n" + + " \"filter\": {\n" + + " \"term\": {\n" + + " \"text\": \"baseball\"\n" + + " }\n" + + " }\n" + + " }\n" + + " ]\n" + + " }\n" + + "}\n"; + + client().prepareIndex("t", "test").setSource(doc).get(); + refresh(); + ensureYellow("t"); + try { + client().search( + searchRequest().source( + searchSource().query(query))).actionGet(); + fail("Should fail with SearchPhaseExecutionException"); + } catch (SearchPhaseExecutionException failure) { + assertTrue(failure.getMessage().contains("SearchParseException")); + assertFalse(failure.getMessage().contains("NullPointerException")); + } + + query = "{\n" + + " \"function_score\": {\n" + + " \"score_mode\": \"sum\",\n" + + " \"boost_mode\": \"replace\",\n" + + " \"functions\": [\n" + + " {\n" + + " \"filter\": {\n" + + " \"term\": {\n" + + " \"text\": \"baseball\"\n" + + " }\n" + + " },\n" + + " \"boost_factor\": 2\n" + + " },\n" + + " {\n" + + " \"filter\": {\n" + + " \"term\": {\n" + + " \"text\": \"baseball\"\n" + + " }\n" + + " }\n" + + " }\n" + + " ]\n" + + " }\n" + + "}"; + + try { + client().search( + searchRequest().source( + searchSource().query(query))).actionGet(); + fail("Should fail with SearchPhaseExecutionException"); + } catch (SearchPhaseExecutionException failure) { + assertTrue(failure.getMessage().contains("SearchParseException")); + assertFalse(failure.getMessage().contains("NullPointerException")); + assertTrue(failure.getMessage().contains("One entry in functions list is missing a function")); + } + + // next test java client + try { + client().prepareSearch("t").setQuery(QueryBuilders.functionScoreQuery(FilterBuilders.matchAllFilter(), null)).get(); + } catch (ElasticsearchIllegalArgumentException failure) { + assertTrue(failure.getMessage().contains("function must not be null")); + } + try { + client().prepareSearch("t").setQuery(QueryBuilders.functionScoreQuery().add(FilterBuilders.matchAllFilter(), null)).get(); + } catch (ElasticsearchIllegalArgumentException failure) { + assertTrue(failure.getMessage().contains("function must not be null")); + } + try { + client().prepareSearch("t").setQuery(QueryBuilders.functionScoreQuery().add(null)).get(); + } catch (ElasticsearchIllegalArgumentException failure) { + assertTrue(failure.getMessage().contains("function must not be null")); + } + } }