mirror of
https://github.com/honeymoose/OpenSearch.git
synced 2025-03-25 09:28:27 +00:00
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
This commit is contained in:
parent
616d0c044f
commit
c597d8d56b
@ -19,6 +19,7 @@
|
|||||||
|
|
||||||
package org.elasticsearch.common.rounding;
|
package org.elasticsearch.common.rounding;
|
||||||
|
|
||||||
|
import org.elasticsearch.ElasticsearchIllegalArgumentException;
|
||||||
import org.elasticsearch.common.io.stream.StreamInput;
|
import org.elasticsearch.common.io.stream.StreamInput;
|
||||||
import org.elasticsearch.common.io.stream.StreamOutput;
|
import org.elasticsearch.common.io.stream.StreamOutput;
|
||||||
import org.elasticsearch.common.unit.TimeValue;
|
import org.elasticsearch.common.unit.TimeValue;
|
||||||
@ -62,6 +63,8 @@ public abstract class TimeZoneRounding extends Rounding {
|
|||||||
|
|
||||||
public Builder(TimeValue interval) {
|
public Builder(TimeValue interval) {
|
||||||
this.unit = null;
|
this.unit = null;
|
||||||
|
if (interval.millis() < 1)
|
||||||
|
throw new ElasticsearchIllegalArgumentException("Zero or negative time interval not supported");
|
||||||
this.interval = interval.millis();
|
this.interval = interval.millis();
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -303,6 +306,8 @@ public abstract class TimeZoneRounding extends Rounding {
|
|||||||
}
|
}
|
||||||
|
|
||||||
UTCIntervalTimeZoneRounding(long interval) {
|
UTCIntervalTimeZoneRounding(long interval) {
|
||||||
|
if (interval < 1)
|
||||||
|
throw new ElasticsearchIllegalArgumentException("Zero or negative time interval not supported");
|
||||||
this.interval = interval;
|
this.interval = interval;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -350,6 +355,8 @@ public abstract class TimeZoneRounding extends Rounding {
|
|||||||
}
|
}
|
||||||
|
|
||||||
TimeIntervalTimeZoneRounding(long interval, DateTimeZone preTz, DateTimeZone postTz) {
|
TimeIntervalTimeZoneRounding(long interval, DateTimeZone preTz, DateTimeZone postTz) {
|
||||||
|
if (interval < 1)
|
||||||
|
throw new ElasticsearchIllegalArgumentException("Zero or negative time interval not supported");
|
||||||
this.interval = interval;
|
this.interval = interval;
|
||||||
this.preTz = preTz;
|
this.preTz = preTz;
|
||||||
this.postTz = postTz;
|
this.postTz = postTz;
|
||||||
@ -408,6 +415,8 @@ public abstract class TimeZoneRounding extends Rounding {
|
|||||||
}
|
}
|
||||||
|
|
||||||
DayIntervalTimeZoneRounding(long interval, DateTimeZone preTz, DateTimeZone postTz) {
|
DayIntervalTimeZoneRounding(long interval, DateTimeZone preTz, DateTimeZone postTz) {
|
||||||
|
if (interval < 1)
|
||||||
|
throw new ElasticsearchIllegalArgumentException("Zero or negative time interval not supported");
|
||||||
this.interval = interval;
|
this.interval = interval;
|
||||||
this.preTz = preTz;
|
this.preTz = preTz;
|
||||||
this.postTz = postTz;
|
this.postTz = postTz;
|
||||||
|
@ -115,7 +115,7 @@ public class HistogramParser implements Aggregator.Parser {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if (interval < 0) {
|
if (interval < 1) {
|
||||||
throw new SearchParseException(context, "Missing required field [interval] for histogram aggregation [" + aggregationName + "]");
|
throw new SearchParseException(context, "Missing required field [interval] for histogram aggregation [" + aggregationName + "]");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -19,6 +19,7 @@
|
|||||||
package org.elasticsearch.search.aggregations.bucket;
|
package org.elasticsearch.search.aggregations.bucket;
|
||||||
|
|
||||||
import org.elasticsearch.action.index.IndexRequestBuilder;
|
import org.elasticsearch.action.index.IndexRequestBuilder;
|
||||||
|
import org.elasticsearch.action.search.SearchPhaseExecutionException;
|
||||||
import org.elasticsearch.action.search.SearchResponse;
|
import org.elasticsearch.action.search.SearchResponse;
|
||||||
import org.elasticsearch.common.joda.Joda;
|
import org.elasticsearch.common.joda.Joda;
|
||||||
import org.elasticsearch.common.settings.ImmutableSettings;
|
import org.elasticsearch.common.settings.ImmutableSettings;
|
||||||
@ -43,6 +44,7 @@ import java.io.IOException;
|
|||||||
import java.util.ArrayList;
|
import java.util.ArrayList;
|
||||||
import java.util.Arrays;
|
import java.util.Arrays;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
|
import java.util.concurrent.TimeUnit;
|
||||||
|
|
||||||
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
|
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
|
||||||
import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
|
import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
|
||||||
@ -52,8 +54,7 @@ import static org.elasticsearch.search.aggregations.AggregationBuilders.max;
|
|||||||
import static org.elasticsearch.search.aggregations.AggregationBuilders.stats;
|
import static org.elasticsearch.search.aggregations.AggregationBuilders.stats;
|
||||||
import static org.elasticsearch.search.aggregations.AggregationBuilders.sum;
|
import static org.elasticsearch.search.aggregations.AggregationBuilders.sum;
|
||||||
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
|
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
|
||||||
import static org.hamcrest.Matchers.equalTo;
|
import static org.hamcrest.Matchers.*;
|
||||||
import static org.hamcrest.Matchers.is;
|
|
||||||
import static org.hamcrest.core.IsNull.notNullValue;
|
import static org.hamcrest.core.IsNull.notNullValue;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -1348,4 +1349,18 @@ public class DateHistogramTests extends ElasticsearchIntegrationTest {
|
|||||||
assertThat(((DateTime) bucket.getKey()), equalTo(key));
|
assertThat(((DateTime) bucket.getKey()), equalTo(key));
|
||||||
assertThat(bucket.getDocCount(), equalTo(3l));
|
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"));
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
@ -20,7 +20,9 @@ package org.elasticsearch.search.aggregations.bucket;
|
|||||||
|
|
||||||
import com.carrotsearch.hppc.LongOpenHashSet;
|
import com.carrotsearch.hppc.LongOpenHashSet;
|
||||||
|
|
||||||
|
import org.apache.tools.ant.filters.TokenFilter.ContainsString;
|
||||||
import org.elasticsearch.action.index.IndexRequestBuilder;
|
import org.elasticsearch.action.index.IndexRequestBuilder;
|
||||||
|
import org.elasticsearch.action.search.SearchPhaseExecutionException;
|
||||||
import org.elasticsearch.action.search.SearchResponse;
|
import org.elasticsearch.action.search.SearchResponse;
|
||||||
import org.elasticsearch.search.aggregations.Aggregator.SubAggCollectionMode;
|
import org.elasticsearch.search.aggregations.Aggregator.SubAggCollectionMode;
|
||||||
import org.elasticsearch.search.aggregations.bucket.filter.Filter;
|
import org.elasticsearch.search.aggregations.bucket.filter.Filter;
|
||||||
@ -49,6 +51,7 @@ import static org.elasticsearch.search.aggregations.AggregationBuilders.sum;
|
|||||||
import static org.elasticsearch.search.aggregations.AggregationBuilders.terms;
|
import static org.elasticsearch.search.aggregations.AggregationBuilders.terms;
|
||||||
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
|
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
|
||||||
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
|
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
|
||||||
|
import static org.hamcrest.Matchers.containsString;
|
||||||
import static org.hamcrest.Matchers.equalTo;
|
import static org.hamcrest.Matchers.equalTo;
|
||||||
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
|
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
|
||||||
import static org.hamcrest.Matchers.is;
|
import static org.hamcrest.Matchers.is;
|
||||||
@ -992,4 +995,17 @@ public class HistogramTests extends ElasticsearchIntegrationTest {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* 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]"));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
Loading…
x
Reference in New Issue
Block a user