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 in repository https://gitbox.apache.org/repos/asf/unomi.git
commit b4aa48cefc049d8b1ec07f65294a0d9c57a8d0fa Author: jsinovassin <[email protected]> AuthorDate: Wed Sep 30 11:09:32 2026 +0200 Bound the MVEL expression cache (unomi-3.0.x) Adapted backport of apache/unomi#879 (be15fd61a) to unomi-3.0.x. The MVEL expression cache was a ConcurrentHashMap keyed by the raw script text, with no bound. Every distinct script a caller sends adds an entry, and this line also stores a REJECTED script under a marker, so a caller whose scripts never pass the filter still grows the map. The public context endpoint evaluates caller-supplied conditions, so the growth is reachable without a credential. The cache is now a size-bounded LRU keyed by a SHA-256 hash of the script, with the bound overridable by org.apache.unomi.scripting.mvel.expressions.cache.max.size. The other half of the master commit does not apply here. TypeResolutionServiceImpl does not exist on this line, and neither does the LOGGED_RECURSION_CHAINS set of EventServiceImpl, so both bounds it adds have nothing to bound. --- .../apache/unomi/scripting/MvelScriptExecutor.java | 46 ++++++++++++++++++---- .../unomi/scripting/MvelScriptExecutorTest.java | 31 +++++++++++++++ 2 files changed, 70 insertions(+), 7 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 8777b0f75..9214b0164 100644 --- a/scripting/src/main/java/org/apache/unomi/scripting/MvelScriptExecutor.java +++ b/scripting/src/main/java/org/apache/unomi/scripting/MvelScriptExecutor.java @@ -21,8 +21,13 @@ 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 @@ -31,7 +36,23 @@ public class MvelScriptExecutor implements ScriptExecutor { private final static String INVALID_SCRIPT_MARKER = "--- Invalid Script Marker ---"; - 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. An unbounded map keyed by + * the raw script grows with every distinct script a caller sends, and the marker below means a + * rejected script is kept too. + */ + 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; @@ -46,10 +67,11 @@ public class MvelScriptExecutor implements ScriptExecutor { try { Thread.currentThread().setContextClassLoader(secureFilteringClassLoader); - if (!mvelExpressions.containsKey(script)) { + String scriptCacheKey = getScriptCacheKey(script); + if (!mvelExpressions.containsKey(scriptCacheKey)) { if (expressionFilterFactory.getExpressionFilter("mvel").filter(script) == null) { - mvelExpressions.put(script, INVALID_SCRIPT_MARKER); + mvelExpressions.put(scriptCacheKey, INVALID_SCRIPT_MARKER); } else { ParserConfiguration parserConfiguration = new ParserConfiguration(); parserConfiguration.setClassLoader(secureFilteringClassLoader); @@ -66,11 +88,12 @@ public class MvelScriptExecutor implements ScriptExecutor { parserContext.addImport("ThreadLocal", String.class); parserContext.addImport("SecurityManager", String.class); - mvelExpressions.put(script, MVEL.compileExpression(script, parserContext)); + mvelExpressions.put(scriptCacheKey, MVEL.compileExpression(script, parserContext)); } } - if (mvelExpressions.containsKey(script) && mvelExpressions.get(script) != INVALID_SCRIPT_MARKER) { - return MVEL.executeExpression(mvelExpressions.get(script), context); + Serializable compiledExpression = mvelExpressions.get(scriptCacheKey); + if (compiledExpression != null && compiledExpression != INVALID_SCRIPT_MARKER) { + return MVEL.executeExpression(compiledExpression, context); } else { return null; } @@ -78,4 +101,13 @@ public class MvelScriptExecutor implements ScriptExecutor { 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 d843c43d5..7a51b05a1 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; @@ -30,6 +31,7 @@ import java.util.Set; import java.util.regex.Pattern; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; public class MvelScriptExecutorTest { @@ -115,6 +117,35 @@ public class MvelScriptExecutorTest { assertFalse("Vulnerability successfully executed ! File created at " + vulnFile.getCanonicalPath(), vulnFile.exists()); } + @Test + public void testExpressionCacheIsBounded() throws Exception { + System.setProperty("org.apache.unomi.scripting.mvel.expressions.cache.max.size", "10"); + try { + MvelScriptExecutor boundedExecutor = new MvelScriptExecutor(); + boundedExecutor.setExpressionFilterFactory(new ExpressionFilterFactory() { + @Override + public ExpressionFilter getExpressionFilter(String filterCollection) { + return new ExpressionFilter(new HashSet<>(), new HashSet<>()); + } + }); + Map<String, Object> ctx = new HashMap<>(); + for (int i = 0; i < 50; i++) { + boundedExecutor.execute(i + " + " + i, ctx); + } + + Field mvelExpressionsField = MvelScriptExecutor.class.getDeclaredField("mvelExpressions"); + mvelExpressionsField.setAccessible(true); + Map<?, ?> cache = (Map<?, ?>) mvelExpressionsField.get(boundedExecutor); + assertTrue("The expression 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 static Event generateMockEvent() { Event mockEvent = new Event(); CustomItem targetItem = new CustomItem();
