This is an automated email from the ASF dual-hosted git repository.
sergehuber pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/unomi.git
The following commit(s) were added to refs/heads/master by this push:
new be15fd61a Bound diagnostic type tracking and the MVEL expression cache
(#879)
be15fd61a is described below
commit be15fd61ac050400fb3a19e88123aa5d1a4faa5c
Author: Serge Huber <[email protected]>
AuthorDate: Thu Sep 24 09:29:22 2026 +0200
Bound diagnostic type tracking and the MVEL expression cache (#879)
---
.../apache/unomi/scripting/MvelScriptExecutor.java | 38 +++++++++++++++++++---
.../unomi/scripting/MvelScriptExecutorTest.java | 32 ++++++++++++++++++
.../services/impl/TypeResolutionServiceImpl.java | 34 ++++++++++++++++---
.../services/impl/events/EventServiceImpl.java | 5 ++-
.../impl/TypeResolutionServiceImplTest.java | 22 +++++++++++++
5 files changed, 122 insertions(+), 9 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 6cd53e0ad..37766c862 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;
@@ -57,7 +77,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);
@@ -75,11 +96,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 {
diff --git
a/services/src/main/java/org/apache/unomi/services/impl/TypeResolutionServiceImpl.java
b/services/src/main/java/org/apache/unomi/services/impl/TypeResolutionServiceImpl.java
index 329756a75..0e99cc571 100644
---
a/services/src/main/java/org/apache/unomi/services/impl/TypeResolutionServiceImpl.java
+++
b/services/src/main/java/org/apache/unomi/services/impl/TypeResolutionServiceImpl.java
@@ -45,6 +45,12 @@ public class TypeResolutionServiceImpl implements
TypeResolutionService {
private static final Logger LOGGER =
LoggerFactory.getLogger(TypeResolutionServiceImpl.class.getName());
private static final int MAX_RECURSION_DEPTH = 1000;
+
+ /**
+ * Upper bound for diagnostic de-duplication collections. Some identifiers
come from public
+ * input (for example condition type ids on context filters), so these
sets must not grow without limit.
+ */
+ private static final int MAX_TRACKED_DIAGNOSTIC_ENTRIES = 1000;
private volatile DefinitionsService definitionsService;
@@ -127,7 +133,7 @@ public class TypeResolutionServiceImpl implements
TypeResolutionService {
}
ConditionType conditionType =
definitionsService.getConditionType(conditionTypeId);
if (conditionType == null) {
- if (unresolvedConditionTypes.add(conditionTypeId)) {
+ if (addBoundedDiagnostic(unresolvedConditionTypes,
conditionTypeId)) {
LOGGER.warn("Couldn't resolve condition type: {} for
{}", conditionTypeId, contextObjectName);
}
return false;
@@ -217,6 +223,20 @@ public class TypeResolutionServiceImpl implements
TypeResolutionService {
}
}
+ /**
+ * Adds a value to a bounded diagnostic de-duplication set. Returns {@code
true} if the value
+ * was newly stored (the caller should log). Once the cap is reached,
further values are ignored.
+ */
+ private static boolean addBoundedDiagnostic(Set<String> set, String value)
{
+ if (set.contains(value)) {
+ return false;
+ }
+ if (set.size() >= MAX_TRACKED_DIAGNOSTIC_ENTRIES) {
+ return false;
+ }
+ return set.add(value);
+ }
+
@Override
public boolean resolveActionTypes(Rule rule, boolean ignoreErrors) {
if (definitionsService == null) {
@@ -228,7 +248,7 @@ public class TypeResolutionServiceImpl implements
TypeResolutionService {
if (rule.getActions() == null) {
if (!ignoreErrors) {
// Only warn once per rule to avoid log spam
- if (warnedRulesWithNullActions.add(ruleId)) {
+ if (addBoundedDiagnostic(warnedRulesWithNullActions, ruleId)) {
LOGGER.warn("Rule {}:{} has null actions", ruleId,
rule.getMetadata() != null ? rule.getMetadata().getName() : "unknown");
}
}
@@ -237,7 +257,7 @@ public class TypeResolutionServiceImpl implements
TypeResolutionService {
if (rule.getActions().isEmpty()) {
if (!ignoreErrors) {
// Only warn once per rule to avoid log spam
- if (warnedRulesWithNullActions.add(ruleId)) {
+ if (addBoundedDiagnostic(warnedRulesWithNullActions, ruleId)) {
LOGGER.warn("Rule {}:{} has empty actions", ruleId,
rule.getMetadata() != null ? rule.getMetadata().getName() : "unknown");
}
}
@@ -262,7 +282,7 @@ public class TypeResolutionServiceImpl implements
TypeResolutionService {
unresolvedActionTypes.remove(action.getActionTypeId());
action.setActionType(actionType);
} else {
- if (unresolvedActionTypes.add(action.getActionTypeId())) {
+ if (addBoundedDiagnostic(unresolvedActionTypes,
action.getActionTypeId())) {
LOGGER.warn("Couldn't resolve action type : {}",
action.getActionTypeId());
}
return false;
@@ -454,6 +474,12 @@ public class TypeResolutionServiceImpl implements
TypeResolutionService {
}
Map<String, InvalidObjectInfo> typeMap =
invalidObjects.computeIfAbsent(objectType, k -> new ConcurrentHashMap<>());
+
+ if (!typeMap.containsKey(objectId) && typeMap.size() >=
MAX_TRACKED_DIAGNOSTIC_ENTRIES) {
+ LOGGER.debug("Invalid object tracking for type {} is full ({}
entries), not tracking {}",
+ objectType, MAX_TRACKED_DIAGNOSTIC_ENTRIES, objectId);
+ return;
+ }
InvalidObjectInfo newInfo = new InvalidObjectInfo(
objectType,
diff --git
a/services/src/main/java/org/apache/unomi/services/impl/events/EventServiceImpl.java
b/services/src/main/java/org/apache/unomi/services/impl/events/EventServiceImpl.java
index 3442a59ac..493cc03ca 100644
---
a/services/src/main/java/org/apache/unomi/services/impl/events/EventServiceImpl.java
+++
b/services/src/main/java/org/apache/unomi/services/impl/events/EventServiceImpl.java
@@ -48,6 +48,7 @@ public class EventServiceImpl implements EventService {
/** Event-type chains already logged at recursion limit (see {@link
#recursionChainKey}). */
private static final Set<String> LOGGED_RECURSION_CHAINS =
ConcurrentHashMap.newKeySet();
+ private static final int MAX_LOGGED_RECURSION_CHAINS = 1000;
/**
* Simple data class to hold event information for recursion tracking.
@@ -221,7 +222,9 @@ public class EventServiceImpl implements EventService {
// Original allowed depths 0-10 (11 calls), blocking at depth 11
if (eventStack.size() > MAX_RECURSION_DEPTH) {
String chainKey = recursionChainKey(eventStack);
- if (LOGGED_RECURSION_CHAINS.add(chainKey)) {
+ if (!LOGGED_RECURSION_CHAINS.contains(chainKey)
+ && LOGGED_RECURSION_CHAINS.size() <
MAX_LOGGED_RECURSION_CHAINS
+ && LOGGED_RECURSION_CHAINS.add(chainKey)) {
EventInfo currentEventInfo = new EventInfo(event);
if (tracer != null) {
tracer.trace("Max recursion depth reached for event: " +
event.getEventType(), event.getItemId());
diff --git
a/services/src/test/java/org/apache/unomi/services/impl/TypeResolutionServiceImplTest.java
b/services/src/test/java/org/apache/unomi/services/impl/TypeResolutionServiceImplTest.java
index 9e62633e8..db69770ae 100644
---
a/services/src/test/java/org/apache/unomi/services/impl/TypeResolutionServiceImplTest.java
+++
b/services/src/test/java/org/apache/unomi/services/impl/TypeResolutionServiceImplTest.java
@@ -1362,5 +1362,27 @@ public class TypeResolutionServiceImplTest {
assertTrue(info.getReason().contains("missingLeafType"),
"Reason text must name the actual failing leaf type");
}
+
+ @Test
+ public void unresolvedConditionTypeTracking_isBounded() {
+
when(definitionsService.getConditionType(startsWith("unknownType-"))).thenReturn(null);
+
+ for (int i = 0; i < 2500; i++) {
+ Condition condition = new Condition();
+ condition.setConditionTypeId("unknownType-" + i);
+ assertFalse(typeResolutionService.resolveConditionType(condition,
"boundedness test"));
+ }
+
+ java.lang.reflect.Field field;
+ try {
+ field =
TypeResolutionServiceImpl.class.getDeclaredField("unresolvedConditionTypes");
+ field.setAccessible(true);
+ Set<?> tracked = (Set<?>) field.get(typeResolutionService);
+ assertTrue(tracked.size() <= 1000,
+ "unresolved condition type tracking must stay bounded, but
had " + tracked.size() + " entries");
+ } catch (ReflectiveOperationException e) {
+ throw new AssertionError(e);
+ }
+ }
}
