You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@unomi.apache.org by js...@apache.org on 2026/09/30 09:09:41 UTC
(unomi) branch backport/bound-mvel-caches-3.0.x created (now b4aa48cef)
This is an automated email from the ASF dual-hosted git repository.
jsinovassin pushed a change to branch backport/bound-mvel-caches-3.0.x
in repository https://gitbox.apache.org/repos/asf/unomi.git
at b4aa48cef Bound the MVEL expression cache (unomi-3.0.x)
This branch includes the following new commits:
new b4aa48cef Bound the MVEL expression cache (unomi-3.0.x)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(unomi) 01/01: Bound the MVEL expression cache (unomi-3.0.x)
Posted by js...@apache.org.
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 <js...@jahia.com>
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();