This is an automated email from the ASF dual-hosted git repository.
mridulpathak pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/trunk by this push:
new 86a0bb67e2 Reject temporal expressions that contain themselves instead
of overflowing the stack
86a0bb67e2 is described below
commit 86a0bb67e215e48d0136021fa4488087af2fca51
Author: Mridul Pathak <[email protected]>
AuthorDate: Fri Oct 30 20:08:57 2026 +0530
Reject temporal expressions that contain themselves instead of overflowing
the stack
TemporalExpressionWorker loaded child expressions recursively through
TemporalExpressionAssoc without tracking which expressions it was already
loading, and createTemporalExpressionAssoc does not prevent cycles, so an
expression that contained itself, directly or through other expressions, ended
in a StackOverflowError when a persisted job or a calendar loaded it. The
worker now keeps the IDs of the expressions on the current load path and throws
an IllegalArgumentException naming the [...]
---
.../service/calendar/TemporalExpressionWorker.java | 51 +++++++++++++++++-----
1 file changed, 41 insertions(+), 10 deletions(-)
diff --git
a/framework/service/src/main/java/org/apache/ofbiz/service/calendar/TemporalExpressionWorker.java
b/framework/service/src/main/java/org/apache/ofbiz/service/calendar/TemporalExpressionWorker.java
index c18bece6a2..6c11bf093f 100644
---
a/framework/service/src/main/java/org/apache/ofbiz/service/calendar/TemporalExpressionWorker.java
+++
b/framework/service/src/main/java/org/apache/ofbiz/service/calendar/TemporalExpressionWorker.java
@@ -18,6 +18,7 @@
*******************************************************************************/
package org.apache.ofbiz.service.calendar;
+import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;
@@ -64,6 +65,11 @@ public final class TemporalExpressionWorker {
* @throws GenericEntityException
*/
public static TemporalExpression getTemporalExpression(Delegator
delegator, String tempExprId) throws GenericEntityException {
+ return getTemporalExpression(delegator, tempExprId, new HashSet<>());
+ }
+
+ private static TemporalExpression getTemporalExpression(Delegator
delegator, String tempExprId, Set<String> loadingIds)
+ throws GenericEntityException {
if (UtilValidate.isEmpty(tempExprId)) {
throw new IllegalArgumentException("tempExprId argument cannot be
empty");
}
@@ -71,7 +77,7 @@ public final class TemporalExpressionWorker {
if (UtilValidate.isEmpty(exprValue)) {
throw new IllegalArgumentException("tempExprId argument invalid -
expression not found");
}
- TemporalExpression result = makeTemporalExpression(delegator,
exprValue);
+ TemporalExpression result = makeTemporalExpression(delegator,
exprValue, loadingIds);
if (Debug.verboseOn()) {
TemporalExpressionPrinter printer = new
TemporalExpressionPrinter(result);
Debug.logVerbose(printer.toString(), MODULE);
@@ -80,8 +86,9 @@ public final class TemporalExpressionWorker {
}
/** Create a <code>TemporalExpression</code> instance from a
TemporalExpression
- * GenericValue.<p>This method makes recursive calls, so care must be
taken to
- * avoid endless loops.
+ * GenericValue.<p>Child expressions are loaded recursively. An expression
that
+ * contains itself, directly or through other expressions, is rejected
with an
+ * <code>IllegalArgumentException</code>.
*
* @param delegator
* @param exprValue
@@ -89,6 +96,28 @@ public final class TemporalExpressionWorker {
* @throws GenericEntityException
*/
public static TemporalExpression makeTemporalExpression(Delegator
delegator, GenericValue exprValue) throws GenericEntityException {
+ return makeTemporalExpression(delegator, exprValue, new HashSet<>());
+ }
+
+ /** Creates the expression while <code>loadingIds</code> holds the IDs of
the
+ * expressions being loaded above it, so that a cycle is detected instead
of
+ * recursing until the stack overflows. An ID is removed again once its
expression
+ * is built, because the same expression may appear more than once in a
tree. */
+ private static TemporalExpression makeTemporalExpression(Delegator
delegator, GenericValue exprValue, Set<String> loadingIds)
+ throws GenericEntityException {
+ String tempExprId = exprValue.getString("tempExprId");
+ if (!loadingIds.add(tempExprId)) {
+ throw new IllegalArgumentException("tempExprId argument invalid -
recursive expression " + tempExprId);
+ }
+ try {
+ return createTemporalExpression(delegator, exprValue, loadingIds);
+ } finally {
+ loadingIds.remove(tempExprId);
+ }
+ }
+
+ private static TemporalExpression createTemporalExpression(Delegator
delegator, GenericValue exprValue, Set<String> loadingIds)
+ throws GenericEntityException {
String tempExprId = exprValue.getString("tempExprId");
String tempExprTypeId = exprValue.getString("tempExprTypeId");
if (DATE_RANGE.equals(tempExprTypeId)) {
@@ -117,7 +146,8 @@ public final class TemporalExpressionWorker {
}
if (inclAssoc != null && exclAssoc != null) {
return setExpressionId(exprValue, new
TemporalExpressions.Difference(getTemporalExpression(delegator,
- inclAssoc.getString("toTempExprId")),
getTemporalExpression(delegator, exclAssoc.getString("toTempExprId"))));
+ inclAssoc.getString("toTempExprId"), loadingIds),
getTemporalExpression(delegator, exclAssoc.getString("toTempExprId"),
+ loadingIds)));
}
} else if (FREQUENCY.equals(tempExprTypeId)) {
return setExpressionId(exprValue, new
TemporalExpressions.Frequency(exprValue.getTimestamp("date1"),
@@ -126,7 +156,7 @@ public final class TemporalExpressionWorker {
return setExpressionId(exprValue, new
TemporalExpressions.HourRange(exprValue.getLong("integer1").intValue(),
exprValue.getLong("integer2").intValue()));
} else if (INTERSECTION.equals(tempExprTypeId)) {
- return setExpressionId(exprValue, new
TemporalExpressions.Intersection(getChildExpressions(delegator, tempExprId)));
+ return setExpressionId(exprValue, new
TemporalExpressions.Intersection(getChildExpressions(delegator, tempExprId,
loadingIds)));
} else if (MINUTE_RANGE.equals(tempExprTypeId)) {
return setExpressionId(exprValue, new
TemporalExpressions.MinuteRange(exprValue.getLong("integer1").intValue(),
exprValue.getLong("integer2").intValue()));
@@ -150,16 +180,17 @@ public final class TemporalExpressionWorker {
}
if (inclAssoc != null && exclAssoc != null && substAssoc != null) {
return setExpressionId(exprValue, new
TemporalExpressions.Substitution(getTemporalExpression(delegator,
- inclAssoc.getString("toTempExprId")),
getTemporalExpression(delegator, exclAssoc.getString("toTempExprId")),
- getTemporalExpression(delegator,
substAssoc.getString("toTempExprId"))));
+ inclAssoc.getString("toTempExprId"), loadingIds),
getTemporalExpression(delegator, exclAssoc.getString("toTempExprId"),
+ loadingIds), getTemporalExpression(delegator,
substAssoc.getString("toTempExprId"), loadingIds)));
}
} else if (UNION.equals(tempExprTypeId)) {
- return setExpressionId(exprValue, new
TemporalExpressions.Union(getChildExpressions(delegator, tempExprId)));
+ return setExpressionId(exprValue, new
TemporalExpressions.Union(getChildExpressions(delegator, tempExprId,
loadingIds)));
}
return TemporalExpressions.NULL_EXPRESSION;
}
- private static Set<TemporalExpression> getChildExpressions(Delegator
delegator, String tempExprId) throws GenericEntityException {
+ private static Set<TemporalExpression> getChildExpressions(Delegator
delegator, String tempExprId, Set<String> loadingIds)
+ throws GenericEntityException {
List<GenericValue> valueList =
EntityQuery.use(delegator).from("TemporalExpressionAssoc").where("fromTempExprId",
tempExprId).cache(true).queryList();
if (UtilValidate.isEmpty(valueList)) {
@@ -167,7 +198,7 @@ public final class TemporalExpressionWorker {
}
Set<TemporalExpression> exprList = new TreeSet<>();
for (GenericValue value : valueList) {
- exprList.add(makeTemporalExpression(delegator,
value.getRelatedOne("ToTemporalExpression", false)));
+ exprList.add(makeTemporalExpression(delegator,
value.getRelatedOne("ToTemporalExpression", false), loadingIds));
}
return exprList;
}
