honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Throw error if query element doesn't end with END_OBJECT (#20528) 

* Throw error if query element doesn't end with END_OBJECT

Followup to #20515 where we added validation that after we parse a query within a query element, we should not get a field name. Truth is that the only token allowed at that point is END_OBJECT, as our DSL allows only one single query within the query object:

```
{
  "query" : {
    "term" : { "field" : "value" }
  }
}
```

We can then check that after parsing of the query we have an end_object that closes the query itself (which we already do). Following that we can check that the query object is immediately closed, as there are no other tokens that can be present in that position.

Relates to #20515
This commit is contained in:
Luca Cavanna 2016-09-16 21:02:12 +02:00 committed by GitHub
parent 697adfb3c4
commit 629e2b2aff
5 changed files with 51 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java
View File

@ -338,10 +338,6 @@ public class BoolQueryBuilder extends AbstractQueryBuilder<BoolQueryBuilder> {
default: default:
throw new ParsingException(parser.getTokenLocation(), "[bool] query does not support [" + currentFieldName + "]"); 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) { } else if (token == XContentParser.Token.START_ARRAY) {
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
switch (currentFieldName) { switch (currentFieldName) {

22
core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java
View File

@ -25,11 +25,9 @@ import org.elasticsearch.common.ParseFieldMatcherSupplier;
import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.indices.query.IndicesQueriesRegistry; import org.elasticsearch.indices.query.IndicesQueriesRegistry;
import org.elasticsearch.script.Script; import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptSettings;
import java.io.IOException; import java.io.IOException;
import java.util.Objects; import java.util.Objects;
@ -95,16 +93,12 @@ public class QueryParseContext implements ParseFieldMatcherSupplier {
* Parses a query excluding the query element that wraps it * Parses a query excluding the query element that wraps it
*/ */
public Optional<QueryBuilder> parseInnerQueryBuilder() throws IOException { public Optional<QueryBuilder> parseInnerQueryBuilder() throws IOException {
// move to START object
XContentParser.Token token;
if (parser.currentToken() != XContentParser.Token.START_OBJECT) { if (parser.currentToken() != XContentParser.Token.START_OBJECT) {
token = parser.nextToken(); if (parser.nextToken() != XContentParser.Token.START_OBJECT) {
if (token != XContentParser.Token.START_OBJECT) {
throw new ParsingException(parser.getTokenLocation(), "[_na] query malformed, must start with start_object"); throw new ParsingException(parser.getTokenLocation(), "[_na] query malformed, must start with start_object");
} }
} }
token = parser.nextToken(); if (parser.nextToken() == XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.END_OBJECT) {
// we encountered '{}' for a query clause // we encountered '{}' for a query clause
String msg = "query malformed, empty clause found at [" + parser.getTokenLocation() +"]"; String msg = "query malformed, empty clause found at [" + parser.getTokenLocation() +"]";
DEPRECATION_LOGGER.deprecated(msg); DEPRECATION_LOGGER.deprecated(msg);
@ -113,26 +107,26 @@ public class QueryParseContext implements ParseFieldMatcherSupplier {
} }
return Optional.empty(); return Optional.empty();
} }
if (token != XContentParser.Token.FIELD_NAME) { if (parser.currentToken() != XContentParser.Token.FIELD_NAME) {
throw new ParsingException(parser.getTokenLocation(), "[_na] query malformed, no field after start_object"); throw new ParsingException(parser.getTokenLocation(), "[_na] query malformed, no field after start_object");
} }
String queryName = parser.currentName(); String queryName = parser.currentName();
// move to the next START_OBJECT // move to the next START_OBJECT
token = parser.nextToken(); if (parser.nextToken() != XContentParser.Token.START_OBJECT) {
if (token != XContentParser.Token.START_OBJECT) {
throw new ParsingException(parser.getTokenLocation(), "[" + queryName + "] query malformed, no start_object after query name"); throw new ParsingException(parser.getTokenLocation(), "[" + queryName + "] query malformed, no start_object after query name");
} }
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
Optional<QueryBuilder> result = (Optional<QueryBuilder>) indicesQueriesRegistry.lookup(queryName, parseFieldMatcher, Optional<QueryBuilder> result = (Optional<QueryBuilder>) indicesQueriesRegistry.lookup(queryName, parseFieldMatcher,
parser.getTokenLocation()).fromXContent(this); parser.getTokenLocation()).fromXContent(this);
//end_object of the specific query (e.g. match, multi_match etc.) element
if (parser.currentToken() != XContentParser.Token.END_OBJECT) { if (parser.currentToken() != XContentParser.Token.END_OBJECT) {
throw new ParsingException(parser.getTokenLocation(), throw new ParsingException(parser.getTokenLocation(),
"[" + queryName + "] malformed query, expected [END_OBJECT] but found [" + parser.currentToken() + "]"); "[" + queryName + "] malformed query, expected [END_OBJECT] but found [" + parser.currentToken() + "]");
} }
parser.nextToken(); //end_object of the query object
if (parser.currentToken() == XContentParser.Token.FIELD_NAME) { if (parser.nextToken() != XContentParser.Token.END_OBJECT) {
throw new ParsingException(parser.getTokenLocation(), throw new ParsingException(parser.getTokenLocation(),
"[" + queryName + "] malformed query, unexpected [FIELD_NAME] found [" + parser.currentName() + "]"); "[" + queryName + "] malformed query, expected [END_OBJECT] but found [" + parser.currentToken() + "]");
} }
return result; return result;
} }

18
core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java
View File

@ -365,12 +365,22 @@ public class BoolQueryBuilderTests extends AbstractQueryTestCase<BoolQueryBuilde
* test that two queries in object throws error * test that two queries in object throws error
*/ */
public void testTooManyQueriesInObject() throws IOException { public void testTooManyQueriesInObject() throws IOException {
String clauseType = randomFrom(new String[] {"must", "should", "must_not", "filter"}); String clauseType = randomFrom("must", "should", "must_not", "filter");
// should also throw error if invalid query is preceded by a valid one // should also throw error if invalid query is preceded by a valid one
String query = "{\"bool\" : {\"" + clauseType String query = "{\n" +
+ "\" : { \"match\" : { \"foo\" : \"bar\" } , \"match\" : { \"baz\" : \"buzz\" } } } }"; " \"bool\": {\n" +
" \"" + clauseType + "\": {\n" +
" \"match\": {\n" +
" \"foo\": \"bar\"\n" +
" },\n" +
" \"match\": {\n" +
" \"baz\": \"buzz\"\n" +
" }\n" +
" }\n" +
" }\n" +
"}";
ParsingException ex = expectThrows(ParsingException.class, () -> parseQuery(query, ParseFieldMatcher.EMPTY)); ParsingException ex = expectThrows(ParsingException.class, () -> parseQuery(query, ParseFieldMatcher.EMPTY));
assertEquals("[match] malformed query, unexpected [FIELD_NAME] found [match]", ex.getMessage()); assertEquals("[match] malformed query, expected [END_OBJECT] but found [FIELD_NAME]", ex.getMessage());
} }
public void testRewrite() throws IOException { public void testRewrite() throws IOException {

27
core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java
View File

@ -700,7 +700,7 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
expectParsingException(json, "field [not_supported] is not supported"); expectParsingException(json, "field [not_supported] is not supported");
} }
public void testMalformedQuery() throws IOException { public void testMalformedQueryMultipleQueryObjects() throws IOException {
//verify that an error is thrown rather than setting the query twice (https://github.com/elastic/elasticsearch/issues/16583) //verify that an error is thrown rather than setting the query twice (https://github.com/elastic/elasticsearch/issues/16583)
String json = "{\n" + String json = "{\n" +
" \"function_score\":{\n" + " \"function_score\":{\n" +
@ -715,15 +715,34 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
" }\n" + " }\n" +
" }\n" + " }\n" +
"}"; "}";
expectParsingException(json, equalTo("[bool] malformed query, unexpected [FIELD_NAME] found [ignored_field_name]")); expectParsingException(json, equalTo("[bool] malformed query, expected [END_OBJECT] but found [FIELD_NAME]"));
} }
private void expectParsingException(String json, Matcher<String> messageMatcher) { public void testMalformedQueryMultipleQueryElements() throws IOException {
String json = "{\n" +
" \"function_score\":{\n" +
" \"query\":{\n" +
" \"bool\":{\n" +
" \"must\":{\"match\":{\"field\":\"value\"}}" +
" }\n" +
" },\n" +
" \"query\":{\n" +
" \"bool\":{\n" +
" \"must\":{\"match\":{\"field\":\"value\"}}" +
" }\n" +
" }\n" +
" }\n" +
" }\n" +
"}";
expectParsingException(json, "[query] is already defined.");
}
private static void expectParsingException(String json, Matcher<String> messageMatcher) {
ParsingException e = expectThrows(ParsingException.class, () -> parseQuery(json)); ParsingException e = expectThrows(ParsingException.class, () -> parseQuery(json));
assertThat(e.getMessage(), messageMatcher); assertThat(e.getMessage(), messageMatcher);
} }
private void expectParsingException(String json, String message) { private static void expectParsingException(String json, String message) {
expectParsingException(json, equalTo("failed to parse [function_score] query. " + message)); expectParsingException(json, equalTo("failed to parse [function_score] query. " + message));
} }

13
core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java
View File

@ -423,8 +423,9 @@ public class SearchSourceBuilderTests extends ESTestCase {
} }
} }
public void testInvalid() throws Exception { public void testMultipleQueryObjectsAreRejected() throws Exception {
String restContent = " { \"query\": {\n" + String restContent =
" { \"query\": {\n" +
" \"multi_match\": {\n" + " \"multi_match\": {\n" +
" \"query\": \"workd\",\n" + " \"query\": \"workd\",\n" +
" \"fields\": [\"title^5\", \"plain_body\"]\n" + " \"fields\": [\"title^5\", \"plain_body\"]\n" +
@ -436,11 +437,9 @@ public class SearchSourceBuilderTests extends ESTestCase {
" }\n" + " }\n" +
" } }"; " } }";
try (XContentParser parser = XContentFactory.xContent(restContent).createParser(restContent)) { try (XContentParser parser = XContentFactory.xContent(restContent).createParser(restContent)) {
SearchSourceBuilder.fromXContent(createParseContext(parser), ParsingException e = expectThrows(ParsingException.class, () -> SearchSourceBuilder.fromXContent(createParseContext(parser),
searchRequestParsers.aggParsers, searchRequestParsers.suggesters, searchRequestParsers.searchExtParsers); searchRequestParsers.aggParsers, searchRequestParsers.suggesters, searchRequestParsers.searchExtParsers));
fail("invalid query syntax multiple keys under query"); assertEquals("[multi_match] malformed query, expected [END_OBJECT] but found [FIELD_NAME]", e.getMessage());
} catch (ParsingException e) {
assertThat(e.getMessage(), containsString("filters"));
} }
} }