This is an automated email from the ASF dual-hosted git repository. jsinovassin pushed a commit to branch backport/bound-mvel-caches-3.0.x-reland in repository https://gitbox.apache.org/repos/asf/unomi.git
commit 06fd017f4e03f238d14bfa64ff7c80f7968b15e8 Author: Jonathan SINOVASSIN-NAIK <[email protected]> AuthorDate: Wed Oct 14 17:32:11 2026 +0200 Bound the MVEL expression cache (unomi-3.0.x) (#891) Co-authored-by: Serge Huber <[email protected]> --- .../apache/unomi/scripting/MvelScriptExecutor.java | 38 +++++++++++++++++++--- .../unomi/scripting/MvelScriptExecutorTest.java | 32 ++++++++++++++++++ 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/scripting/src/main/java/org/apache/unomi/scripting/MvelScriptExecutor.java b/scripting/src/main/java/org/apache/unomi/scripting/MvelScriptExecutor.java index 58e9aee75..a52031915 100644 --- a/scripting/src/main/java/org/apache/unomi/scripting/MvelScriptExecutor.java +++ b/scripting/src/main/java/org/apache/unomi/scripting/MvelScriptExecutor.java @@ -21,15 +21,35 @@ import org.mvel2.ParserConfiguration; import org.mvel2.ParserContext; import java.io.Serializable; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.Collections; +import java.util.HexFormat; +import java.util.LinkedHashMap; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; /** * MVEL script executor implementation */ public class MvelScriptExecutor implements ScriptExecutor { - private Map<String, Serializable> mvelExpressions = new ConcurrentHashMap<>(); + private static final int DEFAULT_EXPRESSIONS_CACHE_MAX_SIZE = 1000; + + private final int expressionsCacheMaxSize = Integer.getInteger( + "org.apache.unomi.scripting.mvel.expressions.cache.max.size", DEFAULT_EXPRESSIONS_CACHE_MAX_SIZE); + + /** + * Size-bounded LRU cache keyed by a fixed-size hash of the script text. Only accepted expressions + * are stored. An unbounded map keyed by the raw script would grow without limit. + */ + private final Map<String, Serializable> mvelExpressions = Collections.synchronizedMap( + new LinkedHashMap<String, Serializable>(16, 0.75f, true) { + @Override + protected boolean removeEldestEntry(Map.Entry<String, Serializable> eldest) { + return size() > expressionsCacheMaxSize; + } + }); private SecureFilteringClassLoader secureFilteringClassLoader = new SecureFilteringClassLoader(getClass().getClassLoader()); private ExpressionFilterFactory expressionFilterFactory; @@ -52,7 +72,8 @@ public class MvelScriptExecutor implements ScriptExecutor { return null; } - Serializable compiledExpression = mvelExpressions.get(script); + String scriptCacheKey = getScriptCacheKey(script); + Serializable compiledExpression = mvelExpressions.get(scriptCacheKey); if (compiledExpression == null) { ParserConfiguration parserConfiguration = new ParserConfiguration(); parserConfiguration.setClassLoader(secureFilteringClassLoader); @@ -70,11 +91,20 @@ public class MvelScriptExecutor implements ScriptExecutor { parserContext.addImport("SecurityManager", String.class); compiledExpression = MVEL.compileExpression(script, parserContext); - mvelExpressions.put(script, compiledExpression); + mvelExpressions.put(scriptCacheKey, compiledExpression); } return MVEL.executeExpression(compiledExpression, context); } finally { Thread.currentThread().setContextClassLoader(tccl); } } + + private static String getScriptCacheKey(String script) { + try { + byte[] hash = MessageDigest.getInstance("SHA-256").digest(script.getBytes(StandardCharsets.UTF_8)); + return HexFormat.of().formatHex(hash); + } catch (NoSuchAlgorithmException e) { + return script; + } + } } diff --git a/scripting/src/test/java/org/apache/unomi/scripting/MvelScriptExecutorTest.java b/scripting/src/test/java/org/apache/unomi/scripting/MvelScriptExecutorTest.java index 539a5b98b..dcc0e8566 100644 --- a/scripting/src/test/java/org/apache/unomi/scripting/MvelScriptExecutorTest.java +++ b/scripting/src/test/java/org/apache/unomi/scripting/MvelScriptExecutorTest.java @@ -23,6 +23,7 @@ import org.junit.Test; import java.io.File; import java.io.IOException; +import java.lang.reflect.Field; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -150,6 +151,37 @@ public class MvelScriptExecutorTest { assertFalse("Vulnerability successfully executed ! File created at " + vulnFile.getCanonicalPath(), vulnFile.exists()); } + @Test + public void testExpressionCacheIsBoundedAndSkipsRejectedScripts() throws Exception { + System.setProperty("org.apache.unomi.scripting.mvel.expressions.cache.max.size", "10"); + try { + MvelScriptExecutor boundedExecutor = new MvelScriptExecutor(); + boundedExecutor.setExpressionFilterFactory(emptyAllowList()); + String padding = "x".repeat(4096); + Map<String, Object> ctx = new HashMap<>(); + for (int i = 0; i < 50; i++) { + boundedExecutor.execute("rejected-script-" + i + "-" + padding, ctx); + } + Field mvelExpressionsField = MvelScriptExecutor.class.getDeclaredField("mvelExpressions"); + mvelExpressionsField.setAccessible(true); + Map<?, ?> cache = (Map<?, ?>) mvelExpressionsField.get(boundedExecutor); + assertEquals("Rejected scripts must not be stored", 0, cache.size()); + + boundedExecutor.setExpressionFilterFactory(allowAllExpressions()); + for (int i = 0; i < 30; i++) { + boundedExecutor.execute(i + "+" + i, ctx); + } + assertTrue("Accepted-script cache must be size-bounded but grew to " + cache.size(), + cache.size() <= 10); + for (Object cacheKey : cache.keySet()) { + assertTrue("Cache keys must be fixed-size hashes, not the raw script text", + ((String) cacheKey).length() <= 64); + } + } finally { + System.clearProperty("org.apache.unomi.scripting.mvel.expressions.cache.max.size"); + } + } + private void assertPublicEvalDoesNotReturnTwo(String expression) { Object result = null; try {
