From 51ea913248c07f4567c6dd2163bd5264622fedd8 Mon Sep 17 00:00:00 2001 From: javanna Date: Wed, 3 Aug 2016 18:44:23 +0200 Subject: [PATCH] Throw parsing error if wildcard query contains multiple fields Wildcard Query, like many other queries, used to parse even when the query referred to multiple fields and the first one would win. We rather throw an exception now instead. Also added test for short prefix query variant and modified the parsing code to consume the whole query object. --- .../index/query/WildcardQueryBuilder.java | 69 ++++++++++--------- .../query/WildcardQueryBuilderTests.java | 61 +++++++++++++--- 2 files changed, 85 insertions(+), 45 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java index ba223f5b1b6..105acf19a13 100644 --- a/core/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java @@ -75,7 +75,7 @@ public class WildcardQueryBuilder extends AbstractQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { XContentParser parser = parseContext.parser(); - - XContentParser.Token token = parser.nextToken(); - if (token != XContentParser.Token.FIELD_NAME) { - throw new ParsingException(parser.getTokenLocation(), "[wildcard] query malformed, no field"); - } - String fieldName = parser.currentName(); + String fieldName = null; String rewrite = null; - String value = null; float boost = AbstractQueryBuilder.DEFAULT_BOOST; String queryName = null; - token = parser.nextToken(); - if (token == XContentParser.Token.START_OBJECT) { - String currentFieldName = null; - while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { - if (token == XContentParser.Token.FIELD_NAME) { - currentFieldName = parser.currentName(); - } else { - if (parseContext.getParseFieldMatcher().match(currentFieldName, WILDCARD_FIELD)) { - value = parser.text(); - } else if (parseContext.getParseFieldMatcher().match(currentFieldName, VALUE_FIELD)) { - value = parser.text(); - } else if (parseContext.getParseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.BOOST_FIELD)) { - boost = parser.floatValue(); - } else if (parseContext.getParseFieldMatcher().match(currentFieldName, REWRITE_FIELD)) { - rewrite = parser.textOrNull(); - } else if (parseContext.getParseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.NAME_FIELD)) { - queryName = parser.text(); + String currentFieldName = null; + XContentParser.Token token; + while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { + if (token == XContentParser.Token.FIELD_NAME) { + currentFieldName = parser.currentName(); + } else if (parseContext.isDeprecatedSetting(currentFieldName)) { + // skip + } else if (token == XContentParser.Token.START_OBJECT) { + if (fieldName != null) { + throw new ParsingException(parser.getTokenLocation(), "[wildcard] query doesn't support multiple fields, found [" + + fieldName + "] and [" + currentFieldName + "]"); + } + fieldName = currentFieldName; + while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { + if (token == XContentParser.Token.FIELD_NAME) { + currentFieldName = parser.currentName(); } else { - throw new ParsingException(parser.getTokenLocation(), - "[wildcard] query does not support [" + currentFieldName + "]"); + if (parseContext.getParseFieldMatcher().match(currentFieldName, WILDCARD_FIELD)) { + value = parser.text(); + } else if (parseContext.getParseFieldMatcher().match(currentFieldName, VALUE_FIELD)) { + value = parser.text(); + } else if (parseContext.getParseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.BOOST_FIELD)) { + boost = parser.floatValue(); + } else if (parseContext.getParseFieldMatcher().match(currentFieldName, REWRITE_FIELD)) { + rewrite = parser.textOrNull(); + } else if (parseContext.getParseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.NAME_FIELD)) { + queryName = parser.text(); + } else { + throw new ParsingException(parser.getTokenLocation(), + "[wildcard] query does not support [" + currentFieldName + "]"); + } } } + } else { + fieldName = parser.currentName(); + value = parser.text(); } - parser.nextToken(); - } else { - value = parser.text(); - parser.nextToken(); } - if (value == null) { - throw new ParsingException(parser.getTokenLocation(), "No value specified for wildcard query"); - } return Optional.of(new WildcardQueryBuilder(fieldName, value) .rewrite(rewrite) .boost(boost) diff --git a/core/src/test/java/org/elasticsearch/index/query/WildcardQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/WildcardQueryBuilderTests.java index 34a006bf8e3..b987c3b9a3d 100644 --- a/core/src/test/java/org/elasticsearch/index/query/WildcardQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/WildcardQueryBuilderTests.java @@ -21,9 +21,12 @@ package org.elasticsearch.index.query; import org.apache.lucene.search.Query; import org.apache.lucene.search.WildcardQuery; +import org.elasticsearch.common.ParsingException; import org.elasticsearch.test.AbstractQueryTestCase; import java.io.IOException; +import java.util.HashMap; +import java.util.Map; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; @@ -32,21 +35,36 @@ public class WildcardQueryBuilderTests extends AbstractQueryTestCase getAlternateVersions() { + Map alternateVersions = new HashMap<>(); + WildcardQueryBuilder wildcardQuery = randomWildcardQuery(); + String contentString = "{\n" + + " \"wildcard\" : {\n" + + " \"" + wildcardQuery.fieldName() + "\" : \"" + wildcardQuery.value() + "\"\n" + + " }\n" + + "}"; + alternateVersions.put(contentString, wildcardQuery); + return alternateVersions; + } + + private static WildcardQueryBuilder randomWildcardQuery() { + // mapped or unmapped field + String text = randomAsciiOfLengthBetween(1, 10); + if (randomBoolean()) { + return new WildcardQueryBuilder(STRING_FIELD_NAME, text); + } else { + return new WildcardQueryBuilder(randomAsciiOfLengthBetween(1, 10), text); + } + } + @Override protected void doAssertLuceneQuery(WildcardQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException { assertThat(query, instanceOf(WildcardQuery.class)); @@ -65,14 +83,14 @@ public class WildcardQueryBuilderTests extends AbstractQueryTestCase