Fix ArrayIndexOutOfBoundsException when no ranges are specified in the query (#23241)

* Fix ArrayIndexOutOfBoundsException in Range Aggregation when no ranges are specified in the query

* Revert "Fix ArrayIndexOutOfBoundsException in Range Aggregation when no ranges are specified in the query"

This reverts commit ad57d8feb3577a64b37de28c6f3df96a3a49fe93.

* Fix range aggregation out of bounds exception when there are no ranges in a range or date_range query

* Fix range aggregation out of bounds exception when there are no ranges in the query

This fix is applied to range queries, date range queries, ip range queries and geo distance aggregation queries
This commit is contained in:
George Papadrosou 2017-05-17 12:34:01 +03:00 committed by Colin Goodheart-Smithe
parent bb3a59fa70
commit 8a4d8909a1
8 changed files with 86 additions and 12 deletions

View File

@ -140,6 +140,9 @@ public class RangeAggregationBuilder extends AbstractRangeBuilder<RangeAggregati
AggregatorFactory<?> parent, Builder subFactoriesBuilder) throws IOException {
// We need to call processRanges here so they are parsed before we make the decision of whether to cache the request
Range[] ranges = processRanges(context, config);
if (ranges.length == 0) {
throw new IllegalArgumentException("No [ranges] specified for the [" + this.getName() + "] aggregation");
}
return new RangeAggregatorFactory(name, config, ranges, keyed, rangeFactory, context, parent, subFactoriesBuilder,
metaData);
}

View File

@ -283,9 +283,12 @@ public class DateRangeAggregationBuilder extends AbstractRangeBuilder<DateRangeA
@Override
protected DateRangeAggregatorFactory innerBuild(SearchContext context, ValuesSourceConfig<Numeric> config,
AggregatorFactory<?> parent, Builder subFactoriesBuilder) throws IOException {
// We need to call processRanges here so they are parsed and we know whether `now` has been used before we make
// We need to call processRanges here so they are parsed and we know whether `now` has been used before we make
// the decision of whether to cache the request
Range[] ranges = processRanges(context, config);
if (ranges.length == 0) {
throw new IllegalArgumentException("No [ranges] specified for the [" + this.getName() + "] aggregation");
}
return new DateRangeAggregatorFactory(name, config, ranges, keyed, rangeFactory, context, parent, subFactoriesBuilder,
metaData);
}

View File

@ -384,6 +384,9 @@ public class GeoDistanceAggregationBuilder extends ValuesSourceAggregationBuilde
ValuesSourceConfig<ValuesSource.GeoPoint> config, AggregatorFactory<?> parent, Builder subFactoriesBuilder)
throws IOException {
Range[] ranges = this.ranges.toArray(new Range[this.range().size()]);
if (ranges.length == 0) {
throw new IllegalArgumentException("No [ranges] specified for the [" + this.getName() + "] aggregation");
}
return new GeoDistanceRangeAggregatorFactory(name, config, origin, ranges, unit, distanceType, keyed, context, parent,
subFactoriesBuilder, metaData);
}

View File

@ -369,6 +369,9 @@ public final class IpRangeAggregationBuilder
AggregatorFactory<?> parent, Builder subFactoriesBuilder)
throws IOException {
List<BinaryRangeAggregator.Range> ranges = new ArrayList<>();
if(this.ranges.size() == 0){
throw new IllegalArgumentException("No [ranges] specified for the [" + this.getName() + "] aggregation");
}
for (Range range : this.ranges) {
ranges.add(new BinaryRangeAggregator.Range(range.key, toBytesRef(range.from), toBytesRef(range.to)));
}

View File

@ -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.settings.Settings;
import org.elasticsearch.plugins.Plugin;
@ -51,6 +52,7 @@ import static org.elasticsearch.search.aggregations.AggregationBuilders.sum;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.core.IsNull.notNullValue;
import static org.hamcrest.core.IsNull.nullValue;
@ -866,6 +868,19 @@ public class DateRangeIT extends ESIntegTestCase {
assertThat(buckets.get(0).getAggregations().asList().isEmpty(), is(true));
}
public void testNoRangesInQuery() {
try {
client().prepareSearch("idx")
.addAggregation(dateRange("my_date_range_agg").field("value"))
.execute().actionGet();
fail();
} catch (SearchPhaseExecutionException spee){
Throwable rootCause = spee.getCause().getCause();
assertThat(rootCause, instanceOf(IllegalArgumentException.class));
assertEquals(rootCause.getMessage(), "No [ranges] specified for the [my_date_range_agg] aggregation");
}
}
/**
* Make sure that a request using a script does not get cached and a request
* not using a script does get cached.

View File

@ -20,6 +20,7 @@ package org.elasticsearch.search.aggregations.bucket;
import org.elasticsearch.Version;
import org.elasticsearch.action.index.IndexRequestBuilder;
import org.elasticsearch.action.search.SearchPhaseExecutionException;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.geo.GeoPoint;
@ -52,6 +53,7 @@ import static org.elasticsearch.search.aggregations.AggregationBuilders.histogra
import static org.elasticsearch.search.aggregations.AggregationBuilders.terms;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.sameInstance;
import static org.hamcrest.core.IsNull.notNullValue;
@ -441,6 +443,19 @@ public class GeoDistanceIT extends ESIntegTestCase {
assertThat(buckets.get(0).getDocCount(), equalTo(0L));
}
public void testNoRangesInQuery() {
try {
client().prepareSearch("idx")
.addAggregation(geoDistance("geo_dist", new GeoPoint(52.3760, 4.894)))
.execute().actionGet();
fail();
} catch (SearchPhaseExecutionException spee){
Throwable rootCause = spee.getCause().getCause();
assertThat(rootCause, instanceOf(IllegalArgumentException.class));
assertEquals(rootCause.getMessage(), "No [ranges] specified for the [geo_dist] aggregation");
}
}
public void testMultiValues() throws Exception {
SearchResponse response = client().prepareSearch("idx-multi")
.addAggregation(geoDistance("amsterdam_rings", new GeoPoint(52.3760, 4.894))

View File

@ -18,18 +18,9 @@
*/
package org.elasticsearch.search.aggregations.bucket;
import org.elasticsearch.cluster.health.ClusterHealthStatus;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
import static org.hamcrest.Matchers.containsString;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import org.elasticsearch.action.search.SearchPhaseExecutionException;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.cluster.health.ClusterHealthStatus;
import org.elasticsearch.common.inject.internal.Nullable;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.plugins.ScriptPlugin;
@ -42,6 +33,17 @@ import org.elasticsearch.search.aggregations.AggregationBuilders;
import org.elasticsearch.search.aggregations.bucket.range.Range;
import org.elasticsearch.test.ESIntegTestCase;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.instanceOf;
@ESIntegTestCase.SuiteScopeTestCase
public class IpRangeIT extends ESIntegTestCase {
@ -221,6 +223,20 @@ public class IpRangeIT extends ESIntegTestCase {
assertThat(e.getMessage(), containsString("[ip_range] does not support scripts"));
}
public void testNoRangesInQuery() {
try {
client().prepareSearch("idx").addAggregation(
AggregationBuilders.ipRange("my_range")
.field("ip"))
.execute().actionGet();
fail();
} catch (SearchPhaseExecutionException spee){
Throwable rootCause = spee.getCause().getCause();
assertThat(rootCause, instanceOf(IllegalArgumentException.class));
assertEquals(rootCause.getMessage(), "No [ranges] specified for the [my_range] aggregation");
}
}
public static class DummyScriptPlugin extends Plugin implements ScriptPlugin {
@Override
public List<NativeScriptFactory> getNativeScripts() {

View File

@ -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.settings.Settings;
import org.elasticsearch.index.fielddata.ScriptDocValues;
@ -53,6 +54,7 @@ import static org.elasticsearch.search.aggregations.AggregationBuilders.terms;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.core.IsNull.notNullValue;
import static org.hamcrest.core.IsNull.nullValue;
@ -661,6 +663,20 @@ public class RangeIT extends ESIntegTestCase {
assertThat(bucket.getDocCount(), equalTo(0L));
}
public void testNoRangesInQuery() {
try {
client().prepareSearch("idx")
.addAggregation(range("foobar")
.field(SINGLE_VALUED_FIELD_NAME))
.execute().actionGet();
fail();
} catch (SearchPhaseExecutionException spee){
Throwable rootCause = spee.getCause().getCause();
assertThat(rootCause, instanceOf(IllegalArgumentException.class));
assertEquals(rootCause.getMessage(), "No [ranges] specified for the [foobar] aggregation");
}
}
public void testScriptMultiValued() throws Exception {
Script script =
new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "doc['" + MULTI_VALUED_FIELD_NAME + "'].values", Collections.emptyMap());