gnodet-bot commented on code in PR #28391:
URL: https://github.com/apache/camel/pull/28391#discussion_r4367575615


##########
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)));

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)))`. If `v` is the same `PyDictionary`, 
`py.__tojava__(Object.class)` returns it again, hits the Map branch again, and 
the recursion is unbounded.
   
   Compare with `Python3Language.materialize(Value, Map<Value, Object> seen)`, 
which inserts the target collection into `seen` *before* recursing to break 
cycles. `PythonExpression.toJava` needs the same guard.
   
   ```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;
           }
           if (seen.containsKey(answer)) {
               return answer; // cycle: return the original (matches 
Python3Language behaviour)
           }
           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);
               for (Object element : list) {
                   copy.add(toJava(element, seen));
               }
               return copy;
           } else if (answer instanceof Set<?> set) {
               Set<Object> copy = new LinkedHashSet<>();
               seen.put(answer, copy);
               for (Object element : set) {
                   copy.add(toJava(element, seen));
               }
               return copy;
           }
           return answer;
       }
   ```



-- 
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]

Reply via email to