Skip to content

fix: pass flags to v8__ObjectTemplate__SetIndexedPropertyHandler FFI#1994

Draft
popematt wants to merge 1 commit into
denoland:mainfrom
popematt:main
Draft

fix: pass flags to v8__ObjectTemplate__SetIndexedPropertyHandler FFI#1994
popematt wants to merge 1 commit into
denoland:mainfrom
popematt:main

Conversation

@popematt
Copy link
Copy Markdown

The FFI declaration for SetIndexedPropertyHandler omitted the flags parameter that the C++ binding accepts, causing any flags set on IndexedPropertyHandlerConfiguration (e.g., NON_MASKING) to be silently discarded. I think it also results in UB because we're calling a C/C++ function with fewer args than expected... and the Rust compiler cannot detect because we correctly called the binding function. The problem is that the extern "C" binding function does not match the actual signature of the C++ function.

The named handler's declaration already passes flags correctly—this aligns the indexed handler to match.

Added a test that sets NON_MASKING on an indexed handler and verifies real indexed elements take priority over the interceptor.

For reference, here is the C++ function that we're trying to match:

rusty_v8/src/binding.cc

Lines 1505 to 1517 in 5d0e31e

void v8__ObjectTemplate__SetIndexedPropertyHandler(
const v8::ObjectTemplate& self, v8::IndexedPropertyGetterCallbackV2 getter,
v8::IndexedPropertySetterCallbackV2 setter,
v8::IndexedPropertyQueryCallbackV2 query,
v8::IndexedPropertyDeleterCallbackV2 deleter,
v8::IndexedPropertyEnumeratorCallback enumerator,
v8::IndexedPropertyDefinerCallbackV2 definer,
v8::IndexedPropertyDescriptorCallbackV2 descriptor,
const v8::Value* data_or_null, v8::PropertyHandlerFlags flags) {
ptr_to_local(&self)->SetHandler(v8::IndexedPropertyHandlerConfiguration(
getter, setter, query, deleter, enumerator, definer, descriptor,
ptr_to_local(data_or_null), flags));
}

The FFI declaration for `SetIndexedPropertyHandler` omitted the `flags`
parameter that the C++ binding accepts, causing any flags set on
`IndexedPropertyHandlerConfiguration` (e.g., `NON_MASKING`) to be
silently discarded. The named handler's declaration already passes flags
correctly—this aligns the indexed handler to match.

Added a test that sets `NON_MASKING` on an indexed handler and verifies
real indexed elements take priority over the interceptor.
@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants