[Scripting] Make Max Script Length Setting Dynamic

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 @@
  */
 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 @@ private static void addFactoryMethod(Map<String, Class<?>> additionalClasses, Cl
      * @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 @@ Constructor<?> compile(Loader loader, MainMethodReserved reserved, String name,
      * @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.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 void testLoopLimits() {
                    "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 static final class Builder {
          *                 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 EnumSet<MetaData.XContentContext> context() {
         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 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 ScriptService(Settings settings, Map<String, ScriptEngine> engines, Map<S
         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 @@ private ScriptEngine getEngine(String lang) {
         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 <FactoryType> FactoryType compile(Script script, ScriptContext<FactoryTyp
             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 boolean isAnyContextEnabled() {
         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 @@ StoredScriptSource getScriptFromClusterState(String id) {
 
     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.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 void setup() throws IOException {
     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 void testGetStoredScript() throws Exception {
         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);