[ML] Limit ML filter items to 10K (#31731)

Add hard limit to the number of items
a filter may have. This serves to protect
from excessive overhead due to the filters
taking too much memory or lookups becoming
too expensive.

Author: dimitris-athanasiou

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/MlFilter.java

@@ -29,6 +29,15 @@
 
 public class MlFilter implements ToXContentObject, Writeable {
 
+    /**
+     * The max number of items allowed per filter.
+     * Limiting the number of items protects users
+     * from running into excessive overhead due to
+     * filters using too much memory and lookups
+     * becoming too expensive.
+     */
+    private static final int MAX_ITEMS = 10000;
+
     public static final String DOCUMENT_ID_PREFIX = "filter_";
 
     public static final String FILTER_TYPE = "filter";
@@ -62,7 +71,7 @@ private static ObjectParser<Builder, Void> createParser(boolean ignoreUnknownFie
     private MlFilter(String id, String description, SortedSet<String> items) {
         this.id = Objects.requireNonNull(id);
         this.description = description;
-        this.items = Objects.requireNonNull(items, ITEMS.getPreferredName() + " must not be null");
+        this.items = Objects.requireNonNull(items);
     }
 
     public MlFilter(StreamInput in) throws IOException {
@@ -182,9 +191,13 @@ public Builder setItems(String... items) {
 
         public MlFilter build() {
             ExceptionsHelper.requireNonNull(id, MlFilter.ID.getPreferredName());
+            ExceptionsHelper.requireNonNull(items, MlFilter.ITEMS.getPreferredName());
             if (!MlStrings.isValidId(id)) {
                 throw ExceptionsHelper.badRequestException(Messages.getMessage(Messages.INVALID_ID, ID.getPreferredName(), id));
             }
+            if (items.size() > MAX_ITEMS) {
+                throw ExceptionsHelper.badRequestException(Messages.getMessage(Messages.FILTER_CONTAINS_TOO_MANY_ITEMS, id, MAX_ITEMS));
+            }
             return new MlFilter(id, description, items);
         }
     }

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/messages/Messages.java

@@ -43,6 +43,7 @@ public final class Messages {
             "Datafeed frequency [{0}] must be a multiple of the aggregation interval [{1}]";
 
     public static final String FILTER_NOT_FOUND = "No filter with id [{0}] exists";
+    public static final String FILTER_CONTAINS_TOO_MANY_ITEMS = "Filter [{0}] contains too many items; up to [{1}] items are allowed";
 
     public static final String INCONSISTENT_ID =
             "Inconsistent {0}; ''{1}'' specified in the body differs from ''{2}'' specified as a URL argument";

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/MlFilterTests.java

@@ -13,6 +13,8 @@
 import org.elasticsearch.test.AbstractSerializingTestCase;
 
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.SortedSet;
 import java.util.TreeSet;
 
@@ -71,9 +73,9 @@ public void testNullId() {
     }
 
     public void testNullItems() {
-        NullPointerException ex = expectThrows(NullPointerException.class,
+        Exception ex = expectThrows(IllegalArgumentException.class,
                 () -> MlFilter.builder(randomValidFilterId()).setItems((SortedSet<String>) null).build());
-        assertEquals(MlFilter.ITEMS.getPreferredName() + " must not be null", ex.getMessage());
+        assertEquals("[items] must not be null.", ex.getMessage());
     }
 
     public void testDocumentId() {
@@ -102,6 +104,27 @@ public void testInvalidId() {
         assertThat(e.getMessage(), startsWith("Invalid filter_id; 'Invalid id' can contain lowercase"));
     }
 
+    public void testTooManyItems() {
+        List<String> items = new ArrayList<>(10001);
+        for (int i = 0; i < 10001; ++i) {
+            items.add("item_" + i);
+        }
+        ElasticsearchStatusException e = expectThrows(ElasticsearchStatusException.class,
+                () -> MlFilter.builder("huge").setItems(items).build());
+        assertThat(e.getMessage(), startsWith("Filter [huge] contains too many items"));
+    }
+
+    public void testGivenItemsAreMaxAllowed() {
+        List<String> items = new ArrayList<>(10000);
+        for (int i = 0; i < 10000; ++i) {
+            items.add("item_" + i);
+        }
+
+        MlFilter hugeFilter = MlFilter.builder("huge").setItems(items).build();
+
+        assertThat(hugeFilter.getItems().size(), equalTo(items.size()));
+    }
+
     public void testItemsAreSorted() {
         MlFilter filter = MlFilter.builder("foo").setItems("c", "b", "a").build();
         assertThat(filter.getItems(), contains("a", "b", "c"));