From dd95dc7a0f49ada38793eb3270af2ac9774e7d78 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Mon, 4 Jul 2016 16:55:20 +0200 Subject: [PATCH] Fix potential AssertionError with include/exclude on terms aggregations. #19252 We call `LongBitSet.set(start, end)`, which fails when `start >= length` (0 in that case). Closes #18575 --- .../GlobalOrdinalsStringTermsAggregator.java | 2 +- .../bucket/terms/support/IncludeExclude.java | 11 +- .../support/IncludeExcludeTests.java | 128 ++++++++++++++++++ 3 files changed, 133 insertions(+), 8 deletions(-) create mode 100644 core/src/test/java/org/elasticsearch/search/aggregations/support/IncludeExcludeTests.java diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java index d4c138a64aa..c4ade93e602 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java @@ -92,7 +92,7 @@ public class GlobalOrdinalsStringTermsAggregator extends AbstractStringTermsAggr globalOrds = valuesSource.globalOrdinalsValues(ctx); if (acceptedGlobalOrdinals == null && includeExclude != null) { - acceptedGlobalOrdinals = includeExclude.acceptedGlobalOrdinals(globalOrds, valuesSource); + acceptedGlobalOrdinals = includeExclude.acceptedGlobalOrdinals(globalOrds); } if (acceptedGlobalOrdinals != null) { diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/support/IncludeExclude.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/support/IncludeExclude.java index f9ef22f2361..e751c54fb16 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/support/IncludeExclude.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/support/IncludeExclude.java @@ -44,8 +44,6 @@ import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.search.DocValueFormat; -import org.elasticsearch.search.aggregations.support.ValuesSource; -import org.elasticsearch.search.aggregations.support.ValuesSource.Bytes.WithOrdinals; import java.io.IOException; import java.util.HashSet; @@ -136,8 +134,7 @@ public class IncludeExclude implements Writeable, ToXContent { } public abstract static class OrdinalsFilter { - public abstract LongBitSet acceptedGlobalOrdinals(RandomAccessOrds globalOrdinals, ValuesSource.Bytes.WithOrdinals valueSource) - throws IOException; + public abstract LongBitSet acceptedGlobalOrdinals(RandomAccessOrds globalOrdinals) throws IOException; } @@ -154,7 +151,7 @@ public class IncludeExclude implements Writeable, ToXContent { * */ @Override - public LongBitSet acceptedGlobalOrdinals(RandomAccessOrds globalOrdinals, ValuesSource.Bytes.WithOrdinals valueSource) + public LongBitSet acceptedGlobalOrdinals(RandomAccessOrds globalOrdinals) throws IOException { LongBitSet acceptedGlobalOrdinals = new LongBitSet(globalOrdinals.getValueCount()); TermsEnum globalTermsEnum; @@ -180,7 +177,7 @@ public class IncludeExclude implements Writeable, ToXContent { } @Override - public LongBitSet acceptedGlobalOrdinals(RandomAccessOrds globalOrdinals, WithOrdinals valueSource) throws IOException { + public LongBitSet acceptedGlobalOrdinals(RandomAccessOrds globalOrdinals) throws IOException { LongBitSet acceptedGlobalOrdinals = new LongBitSet(globalOrdinals.getValueCount()); if (includeValues != null) { for (BytesRef term : includeValues) { @@ -189,7 +186,7 @@ public class IncludeExclude implements Writeable, ToXContent { acceptedGlobalOrdinals.set(ord); } } - } else { + } else if (acceptedGlobalOrdinals.length() > 0) { // default to all terms being acceptable acceptedGlobalOrdinals.set(0, acceptedGlobalOrdinals.length()); } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/support/IncludeExcludeTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/support/IncludeExcludeTests.java new file mode 100644 index 00000000000..90b8d14ef54 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/search/aggregations/support/IncludeExcludeTests.java @@ -0,0 +1,128 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.search.aggregations.support; + +import org.apache.lucene.index.DocValues; +import org.apache.lucene.index.RandomAccessOrds; +import org.apache.lucene.index.SortedSetDocValues; +import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.LongBitSet; +import org.elasticsearch.search.DocValueFormat; +import org.elasticsearch.search.aggregations.bucket.terms.support.IncludeExclude; +import org.elasticsearch.search.aggregations.bucket.terms.support.IncludeExclude.OrdinalsFilter; +import org.elasticsearch.test.ESTestCase; + +import java.io.IOException; +import java.util.Collections; +import java.util.TreeSet; + +public class IncludeExcludeTests extends ESTestCase { + + public void testEmptyTermsWithOrds() throws IOException { + IncludeExclude inexcl = new IncludeExclude( + new TreeSet<>(Collections.singleton(new BytesRef("foo"))), + null); + OrdinalsFilter filter = inexcl.convertToOrdinalsFilter(DocValueFormat.RAW); + LongBitSet acceptedOrds = filter.acceptedGlobalOrdinals(DocValues.emptySortedSet()); + assertEquals(0, acceptedOrds.length()); + + inexcl = new IncludeExclude( + null, + new TreeSet<>(Collections.singleton(new BytesRef("foo")))); + filter = inexcl.convertToOrdinalsFilter(DocValueFormat.RAW); + acceptedOrds = filter.acceptedGlobalOrdinals(DocValues.emptySortedSet()); + assertEquals(0, acceptedOrds.length()); + } + + public void testSingleTermWithOrds() throws IOException { + RandomAccessOrds ords = new RandomAccessOrds() { + + boolean consumed = true; + + @Override + public void setDocument(int docID) { + consumed = false; + } + + @Override + public long nextOrd() { + if (consumed) { + return SortedSetDocValues.NO_MORE_ORDS; + } else { + consumed = true; + return 0; + } + } + + @Override + public BytesRef lookupOrd(long ord) { + assertEquals(0, ord); + return new BytesRef("foo"); + } + + @Override + public long getValueCount() { + return 1; + } + + @Override + public long ordAt(int index) { + return 0; + } + + @Override + public int cardinality() { + return 1; + } + }; + IncludeExclude inexcl = new IncludeExclude( + new TreeSet<>(Collections.singleton(new BytesRef("foo"))), + null); + OrdinalsFilter filter = inexcl.convertToOrdinalsFilter(DocValueFormat.RAW); + LongBitSet acceptedOrds = filter.acceptedGlobalOrdinals(ords); + assertEquals(1, acceptedOrds.length()); + assertTrue(acceptedOrds.get(0)); + + inexcl = new IncludeExclude( + new TreeSet<>(Collections.singleton(new BytesRef("bar"))), + null); + filter = inexcl.convertToOrdinalsFilter(DocValueFormat.RAW); + acceptedOrds = filter.acceptedGlobalOrdinals(ords); + assertEquals(1, acceptedOrds.length()); + assertFalse(acceptedOrds.get(0)); + + inexcl = new IncludeExclude( + new TreeSet<>(Collections.singleton(new BytesRef("foo"))), + new TreeSet<>(Collections.singleton(new BytesRef("foo")))); + filter = inexcl.convertToOrdinalsFilter(DocValueFormat.RAW); + acceptedOrds = filter.acceptedGlobalOrdinals(ords); + assertEquals(1, acceptedOrds.length()); + assertFalse(acceptedOrds.get(0)); + + inexcl = new IncludeExclude( + null, // means everything included + new TreeSet<>(Collections.singleton(new BytesRef("foo")))); + filter = inexcl.convertToOrdinalsFilter(DocValueFormat.RAW); + acceptedOrds = filter.acceptedGlobalOrdinals(ords); + assertEquals(1, acceptedOrds.length()); + assertFalse(acceptedOrds.get(0)); + } + +}