[Scripting] Make Max Script Length Setting Dynamic (#35184)

This changes the current script.max_size_in_bytes to be dynamic so it can be 
set through the cluster settings API. This setting is also applied to inline scripts 
in the compile method of ScriptService to prevent excessively long inline 
scripts from being compiled. The script length limit is removed from Painless as 
this is no longer necessary with the protection in compile.

docs/reference/modules/scripting/using.asciidoc

@@ -204,7 +204,7 @@ you can change this behavior by using the `script.cache.expire` setting.
 You can configure the size of this cache by using the `script.cache.max_size` setting.
 By default, the cache size is `100`.
 
-NOTE: The size of stored scripts is limited to 65,535 bytes. This can be
+NOTE: The size of scripts is limited to 65,535 bytes. This can be
 changed by setting `script.max_size_in_bytes` setting to increase that soft
 limit, but if scripts are really large then a
 <<modules-scripting-engine,native script engine>> should be considered.

modules/lang-painless/src/main/java/org/elasticsearch/painless/Compiler.java

@@ -48,11 +48,6 @@ import static org.elasticsearch.painless.node.SSource.MainMethodReserved;
  */
 final class Compiler {
 
-    /**
-     * The maximum number of characters allowed in the script source.
-     */
-    static final int MAXIMUM_SOURCE_LENGTH = 16384;
-
     /**
      * Define the class with lowest privileges.
      */
@@ -212,12 +207,6 @@ final class Compiler {
      * @return An executable script that implements both a specified interface and is a subclass of {@link PainlessScript}
      */
     Constructor<?> compile(Loader loader, MainMethodReserved reserved, String name, String source, CompilerSettings settings) {
-        if (source.length() > MAXIMUM_SOURCE_LENGTH) {
-            throw new IllegalArgumentException("Scripts may be no longer than " + MAXIMUM_SOURCE_LENGTH +
-                " characters.  The passed in script is " + source.length() + " characters.  Consider using a" +
-                " plugin if a script longer than this length is a requirement.");
-        }
-
         ScriptClassInfo scriptClassInfo = new ScriptClassInfo(painlessLookup, scriptClass);
         SSource root = Walker.buildPainlessTree(scriptClassInfo, reserved, name, source, settings, painlessLookup,
                 null);
@@ -248,12 +237,6 @@ final class Compiler {
      * @return The bytes for compilation.
      */
     byte[] compile(String name, String source, CompilerSettings settings, Printer debugStream) {
-        if (source.length() > MAXIMUM_SOURCE_LENGTH) {
-            throw new IllegalArgumentException("Scripts may be no longer than " + MAXIMUM_SOURCE_LENGTH +
-                " characters.  The passed in script is " + source.length() + " characters.  Consider using a" +
-                " plugin if a script longer than this length is a requirement.");
-        }
-
         ScriptClassInfo scriptClassInfo = new ScriptClassInfo(painlessLookup, scriptClass);
         SSource root = Walker.buildPainlessTree(scriptClassInfo, new MainMethodReserved(), name, source, settings, painlessLookup,
                 debugStream);

modules/lang-painless/src/test/java/org/elasticsearch/painless/WhenThingsGoWrongTests.java

@@ -24,7 +24,6 @@ import org.apache.lucene.util.Constants;
 import org.elasticsearch.script.ScriptException;
 
 import java.lang.invoke.WrongMethodTypeException;
-import java.util.Arrays;
 import java.util.Collections;
 
 import static java.util.Collections.emptyMap;
@@ -200,21 +199,6 @@ public class WhenThingsGoWrongTests extends ScriptTestCase {
                    "The maximum number of statements that can be executed in a loop has been reached."));
     }
 
-    public void testSourceLimits() {
-        final char[] tooManyChars = new char[Compiler.MAXIMUM_SOURCE_LENGTH + 1];
-        Arrays.fill(tooManyChars, '0');
-
-        IllegalArgumentException expected = expectScriptThrows(IllegalArgumentException.class, false, () -> {
-            exec(new String(tooManyChars));
-        });
-        assertTrue(expected.getMessage().contains("Scripts may be no longer than"));
-
-        final char[] exactlyAtLimit = new char[Compiler.MAXIMUM_SOURCE_LENGTH];
-        Arrays.fill(exactlyAtLimit, '0');
-        // ok
-        assertEquals(0, exec(new String(exactlyAtLimit)));
-    }
-
     public void testIllegalDynamicMethod() {
         IllegalArgumentException expected = expectScriptThrows(IllegalArgumentException.class, () -> {
             exec("def x = 'test'; return x.getClass().toString()");

server/src/main/java/org/elasticsearch/script/ScriptMetaData.java

@@ -68,7 +68,7 @@ public final class ScriptMetaData implements MetaData.Custom, Writeable, ToXCont
          *                 is no existing {@link ScriptMetaData}.
          */
         public Builder(ScriptMetaData previous) {
-            this.scripts = previous == null ? new HashMap<>() :new HashMap<>(previous.scripts);
+            this.scripts = previous == null ? new HashMap<>() : new HashMap<>(previous.scripts);
         }
 
         /**
@@ -352,6 +352,13 @@ public final class ScriptMetaData implements MetaData.Custom, Writeable, ToXCont
         return MetaData.ALL_CONTEXTS;
     }
 
+    /**
+     * Returns the map of stored scripts.
+     */
+    Map<String, StoredScriptSource> getStoredScripts() {
+        return scripts;
+    }
+
     /**
      * Retrieves a stored script based on a user-specified id.
      */

server/src/main/java/org/elasticsearch/script/ScriptService.java

@@ -49,6 +49,7 @@ import org.elasticsearch.core.internal.io.IOUtils;
 
 import java.io.Closeable;
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
@@ -98,8 +99,7 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust
     public static final Setting<TimeValue> SCRIPT_CACHE_EXPIRE_SETTING =
         Setting.positiveTimeSetting("script.cache.expire", TimeValue.timeValueMillis(0), Property.NodeScope);
     public static final Setting<Integer> SCRIPT_MAX_SIZE_IN_BYTES =
-        Setting.intSetting("script.max_size_in_bytes", 65535, Property.NodeScope);
-    // public Setting(String key, Function<Settings, String> defaultValue, Function<String, T> parser, Property... properties) {
+        Setting.intSetting("script.max_size_in_bytes", 65535, 0, Property.Dynamic, Property.NodeScope);
     public static final Setting<Tuple<Integer, TimeValue>> SCRIPT_MAX_COMPILATIONS_RATE =
             new Setting<>("script.max_compilations_rate", "75/5m", MAX_COMPILATION_RATE_FUNCTION, Property.Dynamic, Property.NodeScope);
 
@@ -123,6 +123,8 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust
 
     private ClusterState clusterState;
 
+    private int maxSizeInBytes;
+
     private Tuple<Integer, TimeValue> rate;
     private long lastInlineCompileTime;
     private double scriptsPerTimeWindow;
@@ -221,10 +223,12 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust
         this.cache = cacheBuilder.removalListener(new ScriptCacheRemovalListener()).build();
 
         this.lastInlineCompileTime = System.nanoTime();
+        this.setMaxSizeInBytes(SCRIPT_MAX_SIZE_IN_BYTES.get(settings));
         this.setMaxCompilationRate(SCRIPT_MAX_COMPILATIONS_RATE.get(settings));
     }
 
     void registerClusterSettingsListeners(ClusterSettings clusterSettings) {
+        clusterSettings.addSettingsUpdateConsumer(SCRIPT_MAX_SIZE_IN_BYTES, this::setMaxSizeInBytes);
         clusterSettings.addSettingsUpdateConsumer(SCRIPT_MAX_COMPILATIONS_RATE, this::setMaxCompilationRate);
     }
 
@@ -241,6 +245,22 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust
         return scriptEngine;
     }
 
+    /**
+     * Changes the maximum number of bytes a script's source is allowed to have.
+     * @param newMaxSizeInBytes The new maximum number of bytes.
+     */
+    void setMaxSizeInBytes(int newMaxSizeInBytes) {
+        for (Map.Entry<String, StoredScriptSource> source : getScriptsFromClusterState().entrySet()) {
+            if (source.getValue().getSource().getBytes(StandardCharsets.UTF_8).length > newMaxSizeInBytes) {
+                throw new IllegalArgumentException("script.max_size_in_bytes cannot be set to [" + newMaxSizeInBytes + "], " +
+                        "stored script [" + source.getKey() + "] exceeds the new value with a size of " +
+                        "[" + source.getValue().getSource().getBytes(StandardCharsets.UTF_8).length + "]");
+            }
+        }
+
+        maxSizeInBytes = newMaxSizeInBytes;
+    }
+
     /**
      * This configures the maximum script compilations per five minute window.
      *
@@ -295,6 +315,13 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust
             throw new IllegalArgumentException("cannot execute scripts using [" + context.name + "] context");
         }
 
+        if (type == ScriptType.INLINE) {
+            if (idOrCode.getBytes(StandardCharsets.UTF_8).length > maxSizeInBytes) {
+                throw new IllegalArgumentException("exceeded max allowed inline script size in bytes [" + maxSizeInBytes + "] " +
+                        "with size [" + idOrCode.getBytes(StandardCharsets.UTF_8).length + "] for script [" + idOrCode + "]");
+            }
+        }
+
         if (logger.isTraceEnabled()) {
             logger.trace("compiling lang: [{}] type: [{}] script: {}", lang, type, idOrCode);
         }
@@ -391,6 +418,20 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust
         return contextsAllowed == null || contextsAllowed.isEmpty() == false;
     }
 
+    Map<String, StoredScriptSource> getScriptsFromClusterState() {
+        if (clusterState == null) {
+            return Collections.emptyMap();
+        }
+
+        ScriptMetaData scriptMetadata = clusterState.metaData().custom(ScriptMetaData.TYPE);
+
+        if (scriptMetadata == null) {
+            return Collections.emptyMap();
+        }
+
+        return scriptMetadata.getStoredScripts();
+    }
+
     StoredScriptSource getScriptFromClusterState(String id) {
         ScriptMetaData scriptMetadata = clusterState.metaData().custom(ScriptMetaData.TYPE);
 
@@ -409,10 +450,8 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust
 
     public void putStoredScript(ClusterService clusterService, PutStoredScriptRequest request,
                                 ActionListener<AcknowledgedResponse> listener) {
-        int max = SCRIPT_MAX_SIZE_IN_BYTES.get(settings);
-
-        if (request.content().length() > max) {
-            throw new IllegalArgumentException("exceeded max allowed stored script size in bytes [" + max + "] with size [" +
+        if (request.content().length() > maxSizeInBytes) {
+            throw new IllegalArgumentException("exceeded max allowed stored script size in bytes [" + maxSizeInBytes + "] with size [" +
                 request.content().length() + "] for script [" + request.id() + "]");
         }
 

server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java

@@ -27,6 +27,7 @@ import org.elasticsearch.common.breaker.CircuitBreakingException;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.collect.Tuple;
+import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.common.xcontent.XContentFactory;
@@ -54,6 +55,7 @@ public class ScriptServiceTests extends ESTestCase {
     private Map<String, ScriptContext<?>> contexts;
     private ScriptService scriptService;
     private Settings baseSettings;
+    private ClusterSettings clusterSettings;
 
     @Before
     public void setup() throws IOException {
@@ -78,12 +80,22 @@ public class ScriptServiceTests extends ESTestCase {
     private void buildScriptService(Settings additionalSettings) throws IOException {
         Settings finalSettings = Settings.builder().put(baseSettings).put(additionalSettings).build();
         scriptService = new ScriptService(finalSettings, engines, contexts) {
+            @Override
+            Map<String, StoredScriptSource> getScriptsFromClusterState() {
+                Map<String, StoredScriptSource> scripts = new HashMap<>();
+                scripts.put("test1", new StoredScriptSource("test", "1+1", Collections.emptyMap()));
+                scripts.put("test2", new StoredScriptSource("test", "1", Collections.emptyMap()));
+                return scripts;
+            }
+
             @Override
             StoredScriptSource getScriptFromClusterState(String id) {
                 //mock the script that gets retrieved from an index
                 return new StoredScriptSource("test", "1+1", Collections.emptyMap());
             }
         };
+        clusterSettings = new ClusterSettings(finalSettings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
+        scriptService.registerClusterSettingsListeners(clusterSettings);
     }
 
     // even though circuit breaking is allowed to be configured per minute, we actually weigh this over five minutes
@@ -305,6 +317,24 @@ public class ScriptServiceTests extends ESTestCase {
         assertNull(scriptService.getStoredScript(cs, new GetStoredScriptRequest("_id")));
     }
 
+    public void testMaxSizeLimit() throws Exception {
+        buildScriptService(Settings.builder().put(ScriptService.SCRIPT_MAX_SIZE_IN_BYTES.getKey(), 4).build());
+        scriptService.compile(new Script(ScriptType.INLINE, "test", "1+1", Collections.emptyMap()), randomFrom(contexts.values()));
+        IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> {
+            scriptService.compile(new Script(ScriptType.INLINE, "test", "10+10", Collections.emptyMap()), randomFrom(contexts.values()));
+        });
+        assertEquals("exceeded max allowed inline script size in bytes [4] with size [5] for script [10+10]", iae.getMessage());
+        clusterSettings.applySettings(Settings.builder().put(ScriptService.SCRIPT_MAX_SIZE_IN_BYTES.getKey(), 6).build());
+        scriptService.compile(new Script(ScriptType.INLINE, "test", "10+10", Collections.emptyMap()), randomFrom(contexts.values()));
+        clusterSettings.applySettings(Settings.builder().put(ScriptService.SCRIPT_MAX_SIZE_IN_BYTES.getKey(), 5).build());
+        scriptService.compile(new Script(ScriptType.INLINE, "test", "10+10", Collections.emptyMap()), randomFrom(contexts.values()));
+        iae = expectThrows(IllegalArgumentException.class, () -> {
+            clusterSettings.applySettings(Settings.builder().put(ScriptService.SCRIPT_MAX_SIZE_IN_BYTES.getKey(), 2).build());
+        });
+        assertEquals("script.max_size_in_bytes cannot be set to [2], stored script [test1] exceeds the new value with a size of [3]",
+                iae.getMessage());
+    }
+
     private void assertCompileRejected(String lang, String script, ScriptType scriptType, ScriptContext scriptContext) {
         try {
             scriptService.compile(new Script(scriptType, lang, script, Collections.emptyMap()), scriptContext);