Enforce max values limit only when running a script (elastic#88295)

Runtime fields scripts have a hard maximum number of values that they can emit. This was accidentally inherited by some doc_value based queries that reuse the existing runtime fields infrastructure (in absence of a corresponding already existing lucene doc_value based query) as well as by script-less runtime fields that load from _source. This limit was introduced to prevent mistakes when writing a script hence when we are loading from either _source or doc_values the same limit should not be enforced. This commit addresses this, and applies the same to the max chars limit for string fields.
ywangd · Jul 8, 2022 · 3bcadb0 · 3bcadb0
1 parent d27e542
commit 3bcadb0
Show file tree

Hide file tree

Showing 20 changed files with 422 additions and 14 deletions.
diff --git a/docs/changelog/88295.yaml b/docs/changelog/88295.yaml
@@ -0,0 +1,5 @@
+pr: 88295
+summary: Enforce max values limit only when running a script
+area: Mapping
+type: bug
+issues: []
diff --git a/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java b/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java
@@ -99,10 +99,14 @@ protected final void emitFromCompositeScript(CompositeFieldScript compositeField
             return;
         }
         for (Object value : values) {
-            emitFromObject(value);
+            emitValueFromCompositeScript(value);
         }
     }
 
+    protected void emitValueFromCompositeScript(Object value) {
+        emitFromObject(value);
+    }
+
     protected abstract void emitFromObject(Object v);
 
     protected final void emitFromSource() {

diff --git a/server/src/main/java/org/elasticsearch/script/AbstractLongFieldScript.java b/server/src/main/java/org/elasticsearch/script/AbstractLongFieldScript.java
@@ -63,7 +63,6 @@ public final int count() {
     }
 
     public final void emit(long v) {
-        checkMaxSize(count);
         if (values.length < count + 1) {
             values = ArrayUtil.grow(values, count + 1);
         }

diff --git a/server/src/main/java/org/elasticsearch/script/CompositeFieldScript.java b/server/src/main/java/org/elasticsearch/script/CompositeFieldScript.java
@@ -64,6 +64,7 @@ public final Map<String, List<Object>> runForDoc(int doc) {
     protected final void emit(String field, Object value) {
         // fields will be emitted without the prefix, yet they will be looked up using their full name, hence we store the full name
         List<Object> values = this.fieldValues.computeIfAbsent(fieldName + "." + field, s -> new ArrayList<>());
+        checkMaxSize(values.size());
         values.add(value);
     }
 

diff --git a/server/src/main/java/org/elasticsearch/script/DateFieldScript.java b/server/src/main/java/org/elasticsearch/script/DateFieldScript.java
@@ -97,6 +97,7 @@ public Emit(DateFieldScript script) {
         }
 
         public void emit(long v) {
+            script.checkMaxSize(script.count());
             script.emit(v);
         }
     }

diff --git a/server/src/main/java/org/elasticsearch/script/DoubleFieldScript.java b/server/src/main/java/org/elasticsearch/script/DoubleFieldScript.java
@@ -124,7 +124,6 @@ protected void emitFromObject(Object v) {
     }
 
     public final void emit(double v) {
-        checkMaxSize(count);
         if (values.length < count + 1) {
             values = ArrayUtil.grow(values, count + 1);
         }
@@ -139,6 +138,7 @@ public Emit(DoubleFieldScript script) {
         }
 
         public void emit(double v) {
+            script.checkMaxSize(script.count());
             script.emit(v);
         }
     }

diff --git a/server/src/main/java/org/elasticsearch/script/GeoPointFieldScript.java b/server/src/main/java/org/elasticsearch/script/GeoPointFieldScript.java
@@ -155,6 +155,7 @@ public Emit(GeoPointFieldScript script) {
         }
 
         public void emit(double lat, double lon) {
+            script.checkMaxSize(script.count());
             script.emit(lat, lon);
         }
     }

diff --git a/server/src/main/java/org/elasticsearch/script/IpFieldScript.java b/server/src/main/java/org/elasticsearch/script/IpFieldScript.java
@@ -143,7 +143,6 @@ protected void emitFromObject(Object v) {
     }
 
     public final void emit(String v) {
-        checkMaxSize(count);
         if (values.length < count + 1) {
             values = ArrayUtil.grow(values, count + 1);
         }
@@ -161,6 +160,7 @@ public Emit(IpFieldScript script) {
         }
 
         public void emit(String v) {
+            script.checkMaxSize(script.count());
             script.emit(v);
         }
     }

diff --git a/server/src/main/java/org/elasticsearch/script/LongFieldScript.java b/server/src/main/java/org/elasticsearch/script/LongFieldScript.java
@@ -87,6 +87,7 @@ public Emit(LongFieldScript script) {
         }
 
         public void emit(long v) {
+            script.checkMaxSize(script.count());
             script.emit(v);
         }
     }

diff --git a/server/src/main/java/org/elasticsearch/script/StringFieldScript.java b/server/src/main/java/org/elasticsearch/script/StringFieldScript.java
@@ -99,6 +99,15 @@ public final void runForDoc(int docId, Consumer<String> consumer) {
         resultsForDoc(docId).forEach(consumer);
     }
 
+    @Override
+    protected void emitValueFromCompositeScript(Object value) {
+        if (value != null) {
+            String string = value.toString();
+            checkMaxChars(string);
+            emit(string);
+        }
+    }
+
     @Override
     protected void emitFromObject(Object v) {
         if (v != null) {
@@ -107,7 +116,10 @@ protected void emitFromObject(Object v) {
     }
 
     public final void emit(String v) {
-        checkMaxSize(results.size());
+        results.add(v);
+    }
+
+    private void checkMaxChars(String v) {
         chars += v.length();
         if (chars > MAX_CHARS) {
             throw new IllegalArgumentException(
@@ -120,7 +132,6 @@ public final void emit(String v) {
                 )
             );
         }
-        results.add(v);
     }
 
     public static class Emit {
@@ -131,6 +142,8 @@ public Emit(StringFieldScript script) {
         }
 
         public void emit(String v) {
+            script.checkMaxSize(script.results.size());
+            script.checkMaxChars(v);
             script.emit(v);
         }
     }

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldScriptTests.java b/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldScriptTests.java
@@ -63,7 +63,7 @@ public void testTooManyValues() throws IOException {
                     @Override
                     public void execute() {
                         for (int i = 0; i <= AbstractFieldScript.MAX_VALUES * 1000; i++) {
-                            emit(i % 2 == 0);
+                            new Emit(this).value(i % 2 == 0);
                         }
                     }
                 };

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DateFieldScriptTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DateFieldScriptTests.java
@@ -13,13 +13,18 @@
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.tests.index.RandomIndexWriter;
 import org.apache.lucene.util.BytesRef;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.time.DateFormatter;
 import org.elasticsearch.script.AbstractFieldScript;
 import org.elasticsearch.script.DateFieldScript;
 import org.elasticsearch.script.ScriptContext;
 import org.elasticsearch.search.lookup.SearchLookup;
+import org.elasticsearch.xcontent.XContentBuilder;
+import org.elasticsearch.xcontent.json.JsonXContent;
 
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -68,7 +73,7 @@ public void testTooManyValues() throws IOException {
                     @Override
                     public void execute() {
                         for (int i = 0; i <= AbstractFieldScript.MAX_VALUES; i++) {
-                            emit(0);
+                            new Emit(this).emit(0);
                         }
                     }
                 };
@@ -80,4 +85,31 @@ public void execute() {
             }
         }
     }
+
+    public final void testFromSourceDoesNotEnforceValuesLimit() throws IOException {
+        try (Directory directory = newDirectory(); RandomIndexWriter iw = new RandomIndexWriter(random(), directory)) {
+            int numValues = AbstractFieldScript.MAX_VALUES + randomIntBetween(1, 100);
+            XContentBuilder builder = JsonXContent.contentBuilder();
+            builder.startObject();
+            builder.startArray("field");
+            for (int i = 0; i < numValues; i++) {
+                builder.value(i);
+            }
+            builder.endArray();
+            builder.endObject();
+            iw.addDocument(List.of(new StoredField("_source", new BytesRef(Strings.toString(builder)))));
+            try (DirectoryReader reader = iw.getReader()) {
+                DateFieldScript.LeafFactory leafFactory = fromSource().newFactory(
+                    "field",
+                    Collections.emptyMap(),
+                    new SearchLookup(field -> null, (ft, lookup) -> null),
+                    DateFormatter.forPattern("epoch_millis")
+                );
+                DateFieldScript dateFieldScript = leafFactory.newInstance(reader.leaves().get(0));
+                List<Long> results = new ArrayList<>();
+                dateFieldScript.runForDoc(0, results::add);
+                assertEquals(numValues, results.size());
+            }
+        }
+    }
 }
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldScriptTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldScriptTests.java
@@ -13,12 +13,17 @@
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.tests.index.RandomIndexWriter;
 import org.apache.lucene.util.BytesRef;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.script.AbstractFieldScript;
 import org.elasticsearch.script.DoubleFieldScript;
 import org.elasticsearch.script.ScriptContext;
 import org.elasticsearch.search.lookup.SearchLookup;
+import org.elasticsearch.xcontent.XContentBuilder;
+import org.elasticsearch.xcontent.json.JsonXContent;
 
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -65,7 +70,7 @@ public void testTooManyValues() throws IOException {
                     @Override
                     public void execute() {
                         for (int i = 0; i <= AbstractFieldScript.MAX_VALUES; i++) {
-                            emit(1.0);
+                            new Emit(this).emit(1.0);
                         }
                     }
                 };
@@ -77,4 +82,30 @@ public void execute() {
             }
         }
     }
+
+    public final void testFromSourceDoesNotEnforceValuesLimit() throws IOException {
+        try (Directory directory = newDirectory(); RandomIndexWriter iw = new RandomIndexWriter(random(), directory)) {
+            int numValues = AbstractFieldScript.MAX_VALUES + randomIntBetween(1, 100);
+            XContentBuilder builder = JsonXContent.contentBuilder();
+            builder.startObject();
+            builder.startArray("field");
+            for (int i = 0; i < numValues; i++) {
+                builder.value(i + 0.1);
+            }
+            builder.endArray();
+            builder.endObject();
+            iw.addDocument(List.of(new StoredField("_source", new BytesRef(Strings.toString(builder)))));
+            try (DirectoryReader reader = iw.getReader()) {
+                DoubleFieldScript.LeafFactory leafFactory = fromSource().newFactory(
+                    "field",
+                    Collections.emptyMap(),
+                    new SearchLookup(field -> null, (ft, lookup) -> null)
+                );
+                DoubleFieldScript doubleFieldScript = leafFactory.newInstance(reader.leaves().get(0));
+                List<Double> results = new ArrayList<>();
+                doubleFieldScript.runForDoc(0, results::add);
+                assertEquals(numValues, results.size());
+            }
+        }
+    }
 }
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldScriptTests.java b/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldScriptTests.java
@@ -65,7 +65,7 @@ public void testTooManyValues() throws IOException {
                     @Override
                     public void execute() {
                         for (int i = 0; i <= AbstractFieldScript.MAX_VALUES; i++) {
-                            emit(0, 0);
+                            new Emit(this).emit(0, 0);
                         }
                     }
                 };

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/IpFieldScriptTests.java b/server/src/test/java/org/elasticsearch/index/mapper/IpFieldScriptTests.java
@@ -13,12 +13,18 @@
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.tests.index.RandomIndexWriter;
 import org.apache.lucene.util.BytesRef;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.script.AbstractFieldScript;
 import org.elasticsearch.script.IpFieldScript;
 import org.elasticsearch.script.ScriptContext;
 import org.elasticsearch.search.lookup.SearchLookup;
+import org.elasticsearch.xcontent.XContentBuilder;
+import org.elasticsearch.xcontent.json.JsonXContent;
 
 import java.io.IOException;
+import java.net.InetAddress;
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -65,7 +71,7 @@ public void testTooManyValues() throws IOException {
                     @Override
                     public void execute() {
                         for (int i = 0; i <= AbstractFieldScript.MAX_VALUES; i++) {
-                            emit("192.168.0.1");
+                            new Emit(this).emit("192.168.0.1");
                         }
                     }
                 };
@@ -77,4 +83,30 @@ public void execute() {
             }
         }
     }
+
+    public final void testFromSourceDoesNotEnforceValuesLimit() throws IOException {
+        try (Directory directory = newDirectory(); RandomIndexWriter iw = new RandomIndexWriter(random(), directory)) {
+            int numValues = AbstractFieldScript.MAX_VALUES + randomIntBetween(1, 100);
+            XContentBuilder builder = JsonXContent.contentBuilder();
+            builder.startObject();
+            builder.startArray("field");
+            for (int i = 0; i < numValues; i++) {
+                builder.value("192.168.0." + i);
+            }
+            builder.endArray();
+            builder.endObject();
+            iw.addDocument(List.of(new StoredField("_source", new BytesRef(Strings.toString(builder)))));
+            try (DirectoryReader reader = iw.getReader()) {
+                IpFieldScript.LeafFactory leafFactory = fromSource().newFactory(
+                    "field",
+                    Collections.emptyMap(),
+                    new SearchLookup(field -> null, (ft, lookup) -> null)
+                );
+                IpFieldScript ipFieldScript = leafFactory.newInstance(reader.leaves().get(0));
+                List<InetAddress> results = new ArrayList<>();
+                ipFieldScript.runForDoc(0, results::add);
+                assertEquals(numValues, results.size());
+            }
+        }
+    }
 }
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/LongFieldScriptTests.java b/server/src/test/java/org/elasticsearch/index/mapper/LongFieldScriptTests.java
@@ -13,12 +13,17 @@
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.tests.index.RandomIndexWriter;
 import org.apache.lucene.util.BytesRef;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.script.AbstractFieldScript;
 import org.elasticsearch.script.LongFieldScript;
 import org.elasticsearch.script.ScriptContext;
 import org.elasticsearch.search.lookup.SearchLookup;
+import org.elasticsearch.xcontent.XContentBuilder;
+import org.elasticsearch.xcontent.json.JsonXContent;
 
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -65,7 +70,7 @@ public void testTooManyValues() throws IOException {
                     @Override
                     public void execute() {
                         for (int i = 0; i <= AbstractFieldScript.MAX_VALUES; i++) {
-                            emit(0);
+                            new Emit(this).emit(0);
                         }
                     }
                 };
@@ -77,4 +82,30 @@ public void execute() {
             }
         }
     }
+
+    public final void testFromSourceDoesNotEnforceValuesLimit() throws IOException {
+        try (Directory directory = newDirectory(); RandomIndexWriter iw = new RandomIndexWriter(random(), directory)) {
+            int numValues = AbstractFieldScript.MAX_VALUES + randomIntBetween(1, 100);
+            XContentBuilder builder = JsonXContent.contentBuilder();
+            builder.startObject();
+            builder.startArray("field");
+            for (int i = 0; i < numValues; i++) {
+                builder.value(i);
+            }
+            builder.endArray();
+            builder.endObject();
+            iw.addDocument(List.of(new StoredField("_source", new BytesRef(Strings.toString(builder)))));
+            try (DirectoryReader reader = iw.getReader()) {
+                LongFieldScript.LeafFactory leafFactory = fromSource().newFactory(
+                    "field",
+                    Collections.emptyMap(),
+                    new SearchLookup(field -> null, (ft, lookup) -> null)
+                );
+                LongFieldScript longFieldScript = leafFactory.newInstance(reader.leaves().get(0));
+                List<Long> results = new ArrayList<>();
+                longFieldScript.runForDoc(0, results::add);
+                assertEquals(numValues, results.size());
+            }
+        }
+    }
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -97,6 +97,7 @@ public Emit(DateFieldScript script) { @@
             }
             public void emit(long v) {
+                script.checkMaxSize(script.count());
                 script.emit(v);
             }
         }
@@ Expand Down @@