From c79e27180ea14bd536d39e50d310a0345369d7c3 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sat, 25 Jun 2016 16:18:25 -0400 Subject: [PATCH] Require timeout units when parsing query body Today when parsing the timeout field in a query body, if time units are supplied the parser throws a NumberFormatException. Addtionally, the parsing allows the timeout field to not specify units (it assumes milliseconds). This commit fixes this behavior by not only allowing time units to be specified but requires time units to be specified. This is consistent with the documented behavior and the behavior in 2.x. Relates #19077 --- .../search/builder/SearchSourceBuilder.java | 7 +++--- .../builder/SearchSourceBuilderTests.java | 24 +++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java b/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java index e4b72ef3b28..648fcaa859a 100644 --- a/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java @@ -21,7 +21,6 @@ package org.elasticsearch.search.builder; import com.carrotsearch.hppc.ObjectFloatHashMap; import com.carrotsearch.hppc.cursors.ObjectCursor; - import org.elasticsearch.action.support.ToXContentToBytes; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; @@ -42,7 +41,6 @@ import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.script.Script; import org.elasticsearch.search.aggregations.AggregationBuilder; -import org.elasticsearch.search.slice.SliceBuilder; import org.elasticsearch.search.aggregations.AggregatorFactories; import org.elasticsearch.search.aggregations.AggregatorParsers; import org.elasticsearch.search.aggregations.PipelineAggregationBuilder; @@ -51,6 +49,7 @@ import org.elasticsearch.search.highlight.HighlightBuilder; import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.search.rescore.RescoreBuilder; import org.elasticsearch.search.searchafter.SearchAfterBuilder; +import org.elasticsearch.search.slice.SliceBuilder; import org.elasticsearch.search.sort.ScoreSortBuilder; import org.elasticsearch.search.sort.SortBuilder; import org.elasticsearch.search.sort.SortBuilders; @@ -959,7 +958,7 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ } else if (context.getParseFieldMatcher().match(currentFieldName, SIZE_FIELD)) { size = parser.intValue(); } else if (context.getParseFieldMatcher().match(currentFieldName, TIMEOUT_FIELD)) { - timeoutInMillis = parser.longValue(); + timeoutInMillis = TimeValue.parseTimeValue(parser.text(), null, TIMEOUT_FIELD.getPreferredName()).millis(); } else if (context.getParseFieldMatcher().match(currentFieldName, TERMINATE_AFTER_FIELD)) { terminateAfter = parser.intValue(); } else if (context.getParseFieldMatcher().match(currentFieldName, MIN_SCORE_FIELD)) { @@ -1105,7 +1104,7 @@ public final class SearchSourceBuilder extends ToXContentToBytes implements Writ } if (timeoutInMillis != -1) { - builder.field(TIMEOUT_FIELD.getPreferredName(), timeoutInMillis); + builder.field(TIMEOUT_FIELD.getPreferredName(), TimeValue.timeValueMillis(timeoutInMillis).toString()); } if (terminateAfter != SearchContext.DEFAULT_TERMINATE_AFTER) { diff --git a/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java b/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java index 419b11dac60..575018a3097 100644 --- a/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.search.builder; +import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.Version; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexMetaData; @@ -89,7 +90,9 @@ import java.util.concurrent.TimeUnit; import static org.elasticsearch.test.ClusterServiceUtils.createClusterService; import static org.elasticsearch.test.ClusterServiceUtils.setState; +import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasToString; public class SearchSourceBuilderTests extends ESTestCase { private static Injector injector; @@ -593,6 +596,27 @@ public class SearchSourceBuilderTests extends ESTestCase { } } + public void testTimeoutWithUnits() throws IOException { + final String timeout = randomTimeValue(); + final String query = "{ \"query\": { \"match_all\": {}}, \"timeout\": \"" + timeout + "\"}"; + try (XContentParser parser = XContentFactory.xContent(query).createParser(query)) { + final SearchSourceBuilder builder = SearchSourceBuilder.fromXContent(createParseContext(parser), aggParsers, suggesters); + assertThat(builder.timeoutInMillis(), equalTo(TimeValue.parseTimeValue(timeout, null, "timeout").millis())); + } + } + + public void testTimeoutWithoutUnits() throws IOException { + final int timeout = randomIntBetween(1, 1024); + final String query = "{ \"query\": { \"match_all\": {}}, \"timeout\": \"" + timeout + "\"}"; + try (XContentParser parser = XContentFactory.xContent(query).createParser(query)) { + final ElasticsearchParseException e = + expectThrows( + ElasticsearchParseException.class, + () -> SearchSourceBuilder.fromXContent(createParseContext(parser), aggParsers, suggesters)); + assertThat(e, hasToString(containsString("unit is missing or unrecognized"))); + } + } + public void testEmptyPostFilter() throws IOException { SearchSourceBuilder builder = new SearchSourceBuilder(); String query = "{ \"post_filter\": {} }";