gnodet-bot commented on code in PR #28391:
URL: https://github.com/apache/camel/pull/28391#discussion_r4367602851
##########
components/camel-python/src/main/java/org/apache/camel/language/python/PythonExpression.java:
##########
@@ -85,18 +92,53 @@ private <T> T doEvaluate(Exchange exchange, Class<T> type) {
compiler.set("properties", properties);
PyObject out = compiler.eval(compiledExpression);
- if (out != null) {
- String value = out.toString();
+ Object value = toJava(out);
+ if (value != null) {
return
exchange.getContext().getTypeConverter().convertTo(type, value);
}
} catch (Exception e) {
- throw new ExpressionIllegalSyntaxException(expressionString, e);
+ // the script compiled in the constructor, so this is an error
while it runs
+ throw new ExpressionEvaluationException(this, exchange, e);
} finally {
compiler.cleanup();
}
return null;
}
+ /**
+ * The Java value of what a script returns: None is null, a Python string,
number or boolean is the Java String,
+ * number or Boolean, and a Java object bound to the script (such as the
body) is that object. A dict, list, tuple
+ * or set is copied into a LinkedHashMap, ArrayList or LinkedHashSet of
Java values, as the Jython collections are
+ * live views that give their Python text from toString() and keep nested
Python collections.
+ */
+ static Object toJava(Object value) {
+ if (!(value instanceof PyObject py)) {
+ return value;
+ }
+ Object answer = py.__tojava__(Object.class);
+ if (!(answer instanceof PyObject)) {
+ // None, a Python string, number or boolean, or a Java object
+ return answer;
+ }
+ if (answer instanceof Map<?, ?> map) {
+ Map<Object, Object> copy = new LinkedHashMap<>();
+ map.forEach((k, v) -> copy.put(toJava(k), toJava(v)));
+ return copy;
+ } else if (answer instanceof List<?> list) {
+ return copyInto(list, new ArrayList<>(list.size()));
+ } else if (answer instanceof Set<?> set) {
+ return copyInto(set, new LinkedHashSet<>());
+ }
+ return answer;
+ }
+
+ private static <C extends Collection<Object>> C copyInto(Collection<?>
source, C target) {
+ for (Object element : source) {
+ target.add(toJava(element));
+ }
+ return target;
+ }
Review Comment:
⚠️ **`toJava` has no cycle detection — circular Python dicts cause
`StackOverflowError`**
A Jython script can return a circular dict (`d = {}; d['self'] = d`). When
`toJava` reaches the Map branch, it calls `map.forEach((k, v) ->
copy.put(toJava(k), toJava(v)))` with no `seen` tracking. If `v` is the same
`PyDictionary`, the Map branch recurses into it indefinitely →
`StackOverflowError`.
Compare with `Python3Language.materialize(Value, Map<Value, Object> seen)`,
which inserts the target collection into `seen` *before* recursing to break
cycles.
```suggestion
static Object toJava(Object value) {
return toJava(value, new java.util.IdentityHashMap<>());
}
private static Object toJava(Object value,
java.util.IdentityHashMap<Object, Object> seen) {
if (!(value instanceof PyObject py)) {
return value;
}
Object answer = py.__tojava__(Object.class);
if (!(answer instanceof PyObject)) {
// None, a Python string, number or boolean, or a Java object
return answer;
}
Object existing = seen.get(answer);
if (existing != null) {
return existing;
}
if (answer instanceof Map<?, ?> map) {
Map<Object, Object> copy = new LinkedHashMap<>();
seen.put(answer, copy);
map.forEach((k, v) -> copy.put(toJava(k, seen), toJava(v,
seen)));
return copy;
} else if (answer instanceof List<?> list) {
List<Object> copy = new ArrayList<>(list.size());
seen.put(answer, copy);
return copyInto(list, copy, seen);
} else if (answer instanceof Set<?> set) {
Set<Object> copy = new LinkedHashSet<>();
seen.put(answer, copy);
return copyInto(set, copy, seen);
}
return answer;
}
private static <C extends Collection<Object>> C copyInto(Collection<?>
source, C target,
java.util.IdentityHashMap<Object, Object> seen) {
for (Object element : source) {
target.add(toJava(element, seen));
}
return target;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
