Aggregations: Prevent negative intervals in date_histogram

Negative settings for interval in date_histogram could lead to OOM errors in conjunction
with min_doc_count=0. This fix raises exceptions in the histogram builder and the
 TimeZoneRounding classes so that the query fails before this can happen.

Closes #9634
Closes #9690

src/main/java/org/elasticsearch/common/rounding/TimeZoneRounding.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.common.rounding;
 
+import org.elasticsearch.ElasticsearchIllegalArgumentException;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.unit.TimeValue;
@@ -63,6 +64,8 @@ public Builder(DateTimeUnit unit) {
 
         public Builder(TimeValue interval) {
             this.unit = null;
+            if (interval.millis() < 1)
+                throw new ElasticsearchIllegalArgumentException("Zero or negative time interval not supported");
             this.interval = interval.millis();
         }
 
@@ -309,6 +312,8 @@ static class UTCIntervalTimeZoneRounding extends TimeZoneRounding {
         }
 
         UTCIntervalTimeZoneRounding(long interval) {
+            if (interval < 1)
+                throw new ElasticsearchIllegalArgumentException("Zero or negative time interval not supported");
             this.interval = interval;
         }
 
@@ -356,6 +361,8 @@ static class TimeIntervalTimeZoneRounding extends TimeZoneRounding {
         }
 
         TimeIntervalTimeZoneRounding(long interval, DateTimeZone preTz, DateTimeZone postTz) {
+            if (interval < 1)
+                throw new ElasticsearchIllegalArgumentException("Zero or negative time interval not supported");
             this.interval = interval;
             this.preTz = preTz;
             this.postTz = postTz;
@@ -414,6 +421,8 @@ static class DayIntervalTimeZoneRounding extends TimeZoneRounding {
         }
 
         DayIntervalTimeZoneRounding(long interval, DateTimeZone preTz, DateTimeZone postTz) {
+            if (interval < 1)
+                throw new ElasticsearchIllegalArgumentException("Zero or negative time interval not supported");
             this.interval = interval;
             this.preTz = preTz;
             this.postTz = postTz;

src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramParser.java

@@ -118,7 +118,7 @@ public AggregatorFactory parse(String aggregationName, XContentParser parser, Se
             }
         }
 
-        if (interval < 0) {
+        if (interval < 1) {
             throw new SearchParseException(context, "Missing required field [interval] for histogram aggregation [" + aggregationName + "]");
         }
 

src/test/java/org/elasticsearch/search/aggregations/bucket/DateHistogramTests.java

@@ -19,6 +19,7 @@
 package org.elasticsearch.search.aggregations.bucket;
 
 import org.elasticsearch.action.index.IndexRequestBuilder;
+import org.elasticsearch.action.search.SearchPhaseExecutionException;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.common.joda.Joda;
 import org.elasticsearch.common.settings.ImmutableSettings;
@@ -43,13 +44,13 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
+import java.util.concurrent.TimeUnit;
 
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
 import static org.elasticsearch.search.aggregations.AggregationBuilders.*;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
-import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.*;
 import static org.hamcrest.core.IsNull.notNullValue;
 
 /**
@@ -1261,4 +1262,18 @@ public void testIssue6965() {
         assertThat(bucket.getKeyAsNumber().longValue(), equalTo(key.getMillis()));
         assertThat(bucket.getDocCount(), equalTo(3l));
     }
+
+    /**
+     * see issue #9634, negative interval in date_histogram should raise exception
+     */
+    public void testExeptionOnNegativerInterval() {
+        try {
+            client().prepareSearch("idx")
+                    .addAggregation(dateHistogram("histo").field("date").interval(-TimeUnit.DAYS.toMillis(1)).minDocCount(0)).execute()
+                    .actionGet();
+            fail();
+        } catch (SearchPhaseExecutionException e) {
+            assertThat(e.getMessage(), containsString("IllegalArgumentException"));
+        }
+    }
 }

src/test/java/org/elasticsearch/search/aggregations/bucket/HistogramTests.java

@@ -20,6 +20,7 @@
 
 import com.carrotsearch.hppc.LongOpenHashSet;
 import org.elasticsearch.action.index.IndexRequestBuilder;
+import org.elasticsearch.action.search.SearchPhaseExecutionException;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.search.aggregations.Aggregator.SubAggCollectionMode;
 import org.elasticsearch.search.aggregations.bucket.filter.Filter;
@@ -941,4 +942,17 @@ public void singleValuedField_WithExtendedBounds() throws Exception {
         }
     }
 
+    /**
+     * see issue #9634, negative interval in histogram should raise exception
+     */
+    public void testExeptionOnNegativerInterval() {
+        try {
+            client().prepareSearch("empty_bucket_idx")
+                    .addAggregation(histogram("histo").field(SINGLE_VALUED_FIELD_NAME).interval(-1).minDocCount(0)).execute().actionGet();
+            fail();
+        } catch (SearchPhaseExecutionException e) {
+            assertThat(e.getMessage(), containsString("Missing required field [interval]"));
+        }
+    }
+
 }