Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Change JS_IsArray to not transparently detect proxies #938

Closed
bnoordhuis opened this issue Mar 1, 2025 · 0 comments · Fixed by #939
Closed

Change JS_IsArray to not transparently detect proxies #938

bnoordhuis opened this issue Mar 1, 2025 · 0 comments · Fixed by #939

Comments

@bnoordhuis
Copy link
Contributor

JS_IsArray currently has this prototype:

/* return -1 if exception (proxy case) or true/false */ 
int JS_IsArray(JSContext *ctx, JSValue val)

That's jarringly different from JS_IsObject/JS_IsPromise/etc. and returning a ternary instead of a boolean like the other functions is a bug magnet.

The reason it takes a JSContext pointer as its first argument is because it transparently handles proxies and guards against stack overflow in case the proxy's target object loops back on itself.

I don't think it should try to handle proxies because that's inconsistent. Neither JS_IsObject or JS_IsPromise do that, for example.

My proposal:

  1. definitely remove proxy handling from JS_IsArray

  2. optionally add a JS_IsProxy function and maybe JS_GetProxyTarget + JS_GetProxyHandler?

bnoordhuis added a commit to bnoordhuis/quickjs that referenced this issue Mar 1, 2025
This commit also turns JS_IsArray into a simple predicate function.
It no longer punches automatically through proxies because that can
raise exceptions and is inconsistent with the other predicate functions.

Fixes: quickjs-ng#938
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant