Closed mmastrac closed 1 week ago
This can be done by not providing ReturnValue
argument to the relevant callbacks. I'll work on that.
After further research, it looks like the return value is ignored for these with the new APIs.
// TODO(ishell): just return v8::Intercepted.
Handle<Object> PropertyCallbackArguments::CallIndexedSetter(
Handle<InterceptorInfo> interceptor, uint32_t index, Handle<Object> value) {
DCHECK(!interceptor->is_named());
Isolate* isolate = this->isolate();
RCS_SCOPE(isolate, RuntimeCallCounterId::kIndexedSetterCallback);
if (interceptor->has_new_callbacks_signature()) {
// New Api relies on the return value to be set to undefined.
// TODO(ishell): do this in the constructor once the old Api is deprecated.
slot_at(kReturnValueIndex).store(ReadOnlyRoots(isolate).undefined_value());
IndexedPropertySetterCallbackV2 f =
ToCData<IndexedPropertySetterCallbackV2>(interceptor->setter());
Handle<InterceptorInfo> has_side_effects;
PREPARE_CALLBACK_INFO_INTERCEPTOR(isolate, f, void, has_side_effects);
auto intercepted = f(index, v8::Utils::ToLocal(value), callback_info);
if (intercepted == v8::Intercepted::kNo) return {};
// Non-empty handle indicates that the request was intercepted.
return isolate->factory()->undefined_value();
} else {
IndexedPropertySetterCallback f =
ToCData<IndexedPropertySetterCallback>(interceptor->setter());
Handle<InterceptorInfo> has_side_effects;
PREPARE_CALLBACK_INFO_INTERCEPTOR(isolate, f, v8::Value, has_side_effects);
f(index, v8::Utils::ToLocal(value), callback_info);
return GetReturnValue<Object>(isolate);
}
}
The indexed and named property setters and definers should only allow setting "the hole" as a return value -- not any other type. We don't currently check this in any way, allowing embedders to footgun themselves pretty easily.
On the C++ side,
PropertyCallbackInfo<void>
only accepts a single kind of return value: an empty handle. This is different from callingSetUndefined()
, which setsundefined
rather thanTheHole
!Our tests currently call
set_undefined
on theReturnValue
which is incorrect. We should either add aset_empty
to ReturnValue, and potentially should consider adding a more strongly-typed return value interface.Deleters and property callbacks are boolean and integers as well, but those are probably easier for embedders to get right.