add infra around new stream module - #67
Conversation
| ant_value_t prim = js_to_primitive(js, space, 0); | ||
| if (is_err(prim)) return; | ||
| space = prim; |
There was a problem hiding this comment.
🟡 Medium modules/json.c:902
json_set_indent silently swallows errors from js_to_primitive when coercing a boxed space argument. If a boxed Number or String's toString()/valueOf() throws, JSON.stringify still succeeds without indentation instead of propagating the abrupt completion as the spec requires. The is_err(prim) branch does an early return with no way to signal the error to the caller. Consider storing the error in ctx (e.g. ctx->error = prim) and returning a status so js_json_stringify can abort and throw.
| ant_value_t prim = js_to_primitive(js, space, 0); | |
| if (is_err(prim)) return; | |
| space = prim; | |
| ant_value_t prim = js_to_primitive(js, space, 0); | |
| if (is_err(prim)) { ctx->error = prim; return; } | |
| space = prim; |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/modules/json.c around lines 902-904:
`json_set_indent` silently swallows errors from `js_to_primitive` when coercing a boxed `space` argument. If a boxed Number or String's `toString()`/`valueOf()` throws, `JSON.stringify` still succeeds without indentation instead of propagating the abrupt completion as the spec requires. The `is_err(prim)` branch does an early `return` with no way to signal the error to the caller. Consider storing the error in `ctx` (e.g. `ctx->error = prim`) and returning a status so `js_json_stringify` can abort and throw.
| if (!json_out_reserve(o, 48)) return false; | ||
|
|
||
| char *p = o->buf + o->len; | ||
| int64_t as_int = (int64_t)num; |
There was a problem hiding this comment.
🟡 Medium modules/json.c:287
json_out_number casts num to int64_t before the range check, so calling it with a finite non-integer like 1e100 triggers undefined behavior — the conversion is UB when the integral part exceeds int64_t range, and this happens before the non-integer formatter is reached. Move the range check before the cast to (int64_t).
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/modules/json.c around line 287:
`json_out_number` casts `num` to `int64_t` before the range check, so calling it with a finite non-integer like `1e100` triggers undefined behavior — the conversion is UB when the integral part exceeds `int64_t` range, and this happens before the non-integer formatter is reached. Move the range check before the cast to `(int64_t)`.
There was a problem hiding this comment.
🟡 Medium
Line 394 in 9bc1fc6
When val is a Proxy whose target (or its prototype chain) has a toJSON property, json_apply_tojson reads it via lkp_interned/lkp_proto and js_prop_load instead of going through the Proxy's get trap, so the trap is bypassed entirely: JSON.stringify can invoke a callable that the Proxy explicitly hid, or skip the trap's replacement function. The same raw lookups bypass accessor dispatch, so an own or inherited toJSON getter (e.g. get toJSON() { ... }) is never invoked and its return value — and any error it throws — is silently lost, causing JSON.stringify to serialize the original value instead of the getter's transformed result. Consider obtaining toJSON via the engine's normal property-get path (js_get or equivalent) so Proxy traps and accessor dispatch run as expected.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/modules/json.c around line 394:
When `val` is a Proxy whose target (or its prototype chain) has a `toJSON` property, `json_apply_tojson` reads it via `lkp_interned`/`lkp_proto` and `js_prop_load` instead of going through the Proxy's `get` trap, so the trap is bypassed entirely: `JSON.stringify` can invoke a callable that the Proxy explicitly hid, or skip the trap's replacement function. The same raw lookups bypass accessor dispatch, so an own or inherited `toJSON` getter (e.g. `get toJSON() { ... }`) is never invoked and its return value — and any error it throws — is silently lost, causing `JSON.stringify` to serialize the original value instead of the getter's transformed result. Consider obtaining `toJSON` via the engine's normal property-get path (`js_get` or equivalent) so Proxy traps and accessor dispatch run as expected.
| if (swap_entry) swap_entry->slot = slot; | ||
| if (swapped_from) *swapped_from = last; | ||
| } | ||
| if (slot != last) |
There was a problem hiding this comment.
🟠 High src/shapes.c:474
After ant_shape_remove_slot shifts shape properties left with memmove, deleting a non-final property corrupts the pairing between shape keys and object values: the caller (obj_remove_prop_slot) still overwrites only the deleted value slot with the last value, so after deleting a from {a:1,b:2,c:3} the shape says slot 0 is b but value slot 0 holds 3. The old swap-based code updated exactly one index entry, so the value array and the shape index stayed consistent; the new shift invalidates every entry whose slot exceeds the deleted one, but the corresponding value array is still updated as a single swap. The shape-side and value-side removal strategies must match — either shift both arrays and decrement all affected index slots, or swap both arrays and update only the swapped entry.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/shapes.c around line 474:
After `ant_shape_remove_slot` shifts shape properties left with `memmove`, deleting a non-final property corrupts the pairing between shape keys and object values: the caller (`obj_remove_prop_slot`) still overwrites only the deleted value slot with the last value, so after deleting `a` from `{a:1,b:2,c:3}` the shape says slot 0 is `b` but value slot 0 holds `3`. The old swap-based code updated exactly one index entry, so the value array and the shape index stayed consistent; the new shift invalidates every entry whose slot exceeds the deleted one, but the corresponding value array is still updated as a single swap. The shape-side and value-side removal strategies must match — either shift both arrays and decrement all affected index slots, or swap both arrays and update only the swapped entry.
| static bool json_out_quoted(ant_t *js, json_out_t *o, ant_value_t value) { | ||
| size_t byte_len = 0; | ||
| char *str = js_getstr(js, value, &byte_len); | ||
| if (!str) return json_out_write(o, "\"\"", 2); |
There was a problem hiding this comment.
🟡 Medium modules/json.c:278
json_out_quoted returns json_out_write(o, "\"\"", 2) (the valid JSON empty string) when js_getstr returns NULL, so a nonempty string value is silently serialized as "" instead of failing. Since NULL indicates a conversion or allocation failure elsewhere, this turns an OOM/error into silent data corruption. Set o->oom and return false instead of writing "".
| if (!str) return json_out_write(o, "\"\"", 2); | |
| if (!str) { o->oom = true; return false; } |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/modules/json.c around line 278:
`json_out_quoted` returns `json_out_write(o, "\"\"", 2)` (the valid JSON empty string) when `js_getstr` returns `NULL`, so a nonempty string value is silently serialized as `""` instead of failing. Since `NULL` indicates a conversion or allocation failure elsewhere, this turns an OOM/error into silent data corruption. Set `o->oom` and return `false` instead of writing `""`.
| if (nargs < 3) return; | ||
|
|
||
| ant_value_t space = args[2]; | ||
| if (is_special_object(space)) { |
There was a problem hiding this comment.
🟡 Medium modules/json.c:901
json_set_indent calls js_to_primitive on any object-valued space argument, but per the JSON.stringify spec only boxed Number and boxed String objects should be coerced; all other objects must be ignored. As a result, passing an ordinary object as space unexpectedly runs its valueOf/toString/Symbol.toPrimitive hooks — which can throw (silently swallowed) or otherwise interfere — instead of leaving indentation unset. Consider checking for number/string object subtypes before calling js_to_primitive, and skip coercion for any other object type.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/modules/json.c around line 901:
`json_set_indent` calls `js_to_primitive` on any object-valued `space` argument, but per the `JSON.stringify` spec only boxed Number and boxed String objects should be coerced; all other objects must be ignored. As a result, passing an ordinary object as `space` unexpectedly runs its `valueOf`/`toString`/`Symbol.toPrimitive` hooks — which can throw (silently swallowed) or otherwise interfere — instead of leaving indentation unset. Consider checking for number/string object subtypes before calling `js_to_primitive`, and skip coercion for any other object type.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
No description provided.