Make parsing of bool queries stricter

Currently we don't throw an error when there is more than one query clause
specified in a must/must_not/should/filter object of the bool query without
using array notation, e.g.:
 { "bool" : { "must" : { "match" : { ... }, "match": { ... }}}}

In these cases, only the first query will be parsed and further behaviour is
unspecified, possibly leading to silently ignoring the rest of the query.
Instead we should throw a ParsingException if we don't encounter an END_OBJECT
token after having parsed the query clause.
This commit is contained in:
Christoph Büscher 2016-06-23 17:04:56 +02:00
parent 4ad5da2c11
commit 6d5b4a78fe
2 changed files with 27 additions and 0 deletions

View File

@ -340,6 +340,10 @@ public class BoolQueryBuilder extends AbstractQueryBuilder<BoolQueryBuilder> {
default:
throw new ParsingException(parser.getTokenLocation(), "[bool] query does not support [" + currentFieldName + "]");
}
if (parser.currentToken() != XContentParser.Token.END_OBJECT) {
throw new ParsingException(parser.getTokenLocation(),
"expected [END_OBJECT] but got [{}], possibly too many query clauses", parser.currentToken());
}
} else if (token == XContentParser.Token.START_ARRAY) {
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
switch (currentFieldName) {

View File

@ -25,6 +25,7 @@ import org.apache.lucene.search.ConstantScoreQuery;
import org.apache.lucene.search.MatchAllDocsQuery;
import org.apache.lucene.search.Query;
import org.elasticsearch.common.ParseFieldMatcher;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
@ -370,6 +371,28 @@ public class BoolQueryBuilderTests extends AbstractQueryTestCase<BoolQueryBuilde
assertThat(ex.getMessage(), startsWith("query malformed, empty clause found at"));
}
/**
* test that unknown query names in the clauses throw an error
*/
public void testUnknownQueryName() throws IOException {
String query = "{\"bool\" : {\"must\" : { \"unknown_query\" : { } } } }";
ParsingException ex = expectThrows(ParsingException.class, () -> parseQuery(query, ParseFieldMatcher.EMPTY));
assertEquals("no [query] registered for [unknown_query]", ex.getMessage());
}
/**
* test that two queries in object throws error
*/
public void testTooManyQueriesInObject() throws IOException {
String clauseType = randomFrom(new String[] {"must", "should", "must_not", "filter"});
// should also throw error if invalid query is preceded by a valid one
String query = "{\"bool\" : {\"" + clauseType
+ "\" : { \"match\" : { \"foo\" : \"bar\" } , \"match\" : { \"baz\" : \"buzz\" } } } }";
ParsingException ex = expectThrows(ParsingException.class, () -> parseQuery(query, ParseFieldMatcher.EMPTY));
assertEquals("expected [END_OBJECT] but got [FIELD_NAME], possibly too many query clauses", ex.getMessage());
}
public void testRewrite() throws IOException {
BoolQueryBuilder boolQueryBuilder = new BoolQueryBuilder();
boolean mustRewrite = false;