gnodet-bot commented on code in PR #28391:
URL: https://github.com/apache/camel/pull/28391#discussion_r4367010673
##########
components/camel-quickjs/src/main/java/org/apache/camel/language/quickjs/QuickjsHelper.java:
##########
@@ -152,6 +152,78 @@ final class QuickjsHelper {
*/
static final String CAMEL_FACADE = camelFacade();
+ /**
+ * JavaScript that prepares the script result for the JSON hand-over to
Java, where a {@code Set} or a {@code Map}
+ * would become an empty object and a typed array an object of index to
value: a {@code Set} becomes an array, a
+ * {@code Map} an object (its keys as strings), and a typed array or an
{@code ArrayBuffer} an array of numbers,
+ * also when nested in objects and arrays. A primitive is returned after
one type check, and an object or array is
+ * only copied when something in it is converted. A value with {@code
toJSON} (such as a {@code Date}) and a cyclic
+ * reference are left to JSON as before. The built-ins are captured once
so a script cannot replace them.
+ */
+ private static final String RESULT_CONVERTER = """
+ const __camelResult = (() => {
+ const SetType = Set;
+ const MapType = Map;
+ const ArrayBufferType = ArrayBuffer;
+ const DataViewType = DataView;
+ const Uint8ArrayType = Uint8Array;
+ const isView = ArrayBuffer.isView;
+ const isArray = Array.isArray;
+ const from = Array.from;
+ const keys = Object.keys;
+ const fromEntries = Object.fromEntries;
+ const define = Object.defineProperty;
+ const isObject = v => typeof v === "object" && v !== null;
+ const put = (o, k, v) => define(o, k, { value: v, writable:
true, enumerable: true, configurable: true });
+ const convert = (v, path) => {
+ if (typeof v.toJSON === "function" || path.includes(v)) {
+ return v;
+ }
+ if (v instanceof ArrayBufferType) {
+ return from(new Uint8ArrayType(v));
+ }
+ if (isView(v)) {
+ return v instanceof DataViewType ? v : from(v);
Review Comment:
⚠️ **DataView silently becomes `{}` when returned from a script**
The `isView(v)` branch returns `v` unchanged when `v instanceof
DataViewType`. A `DataView` has no own enumerable string-keyed properties, so
`JSON.stringify(new DataView(new ArrayBuffer(4)))` produces `"{}"`. Any script
returning a `DataView` (or an object containing one) will silently lose its
contents in the result.
Since DataView is a view over an ArrayBuffer, converting it like an
ArrayBuffer makes sense:
```suggestion
return v instanceof DataViewType ? from(new
Uint8ArrayType(v.buffer, v.byteOffset, v.byteLength)) : from(v);
```
Or if DataView-as-bytes is not a use-case worth supporting, at minimum a
comment explaining why it's left as-is (and what callers will see) would
prevent future confusion.
##########
components/camel-python3/src/main/java/org/apache/camel/language/python3/Python3Language.java:
##########
@@ -326,6 +341,11 @@ private static Object materialize(Value value, Map<Value,
Object> seen) {
if (isMainModule(value)) {
return null;
}
+ if (value.hasBufferElements()) {
+ byte[] bytes = new byte[Math.toIntExact(value.getBufferSize())];
Review Comment:
⚠️ **Same unbounded buffer allocation as in `JavaScriptLanguage.java`**
A Python `bytes` or `bytearray` of a few hundred MB will be eagerly
allocated. Same cap as suggested in JavaScriptLanguage applies here.
##########
components/camel-javascript/src/main/java/org/apache/camel/language/js/JavaScriptLanguage.java:
##########
@@ -149,6 +168,11 @@ private static Object materialize(Value value, Map<Value,
Object> seen) {
if (value.isInstant()) {
return value.asInstant();
}
+ if (value.hasBufferElements()) {
+ byte[] bytes = new byte[Math.toIntExact(value.getBufferSize())];
Review Comment:
⚠️ **No size guard before allocating `new
byte[Math.toIntExact(value.getBufferSize())]`**
A script returning `new ArrayBuffer(2_000_000_000)` will cause an OOM
allocation. `Math.toIntExact` throws `ArithmeticException` for buffers > 2 GB,
but for large-but-valid sizes (e.g. 500 MB) it allocates eagerly. In a route
where the script input is untrusted or externally influenced, this is a DoS
vector.
Consider a reasonable cap before allocating:
```suggestion
if (value.getBufferSize() > 100 * 1024 * 1024) {
throw new ExpressionEvaluationException(expression, "Script
returned an ArrayBuffer of " + value.getBufferSize() + " bytes, which exceeds
the 100 MB limit", exchange, null);
}
byte[] bytes = new byte[Math.toIntExact(value.getBufferSize())];
```
--
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]
