Skip to content

Commit 4aa6997

Browse files
isheludkoisheludko
authored
Migrate to a new V8 interceptors Api (#180)
The new callback should return v8::Intercepted::kYes/kNo to indicate whether the operation was intercepted. This replaces the old approach where the callback had to leave the return value unset or set it to an empty handle to indicate that the the request wasn't intercepted. See https://crrev.com/c/5465509 and https://crrev.com/c/5465513.
1 parent fc89994 commit 4aa6997

File tree

4 files changed

+119
-102
lines changed

4 files changed

+119
-102
lines changed

‎src/node_contextify.cc

+60-53
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ using v8::FunctionTemplate;
4949
using v8::HandleScope;
5050
using v8::IndexedPropertyHandlerConfiguration;
5151
using v8::Int32;
52+
using v8::Intercepted;
5253
using v8::Isolate;
5354
using v8::Just;
5455
using v8::Local;
@@ -459,13 +460,12 @@ bool ContextifyContext::IsStillInitializing(const ContextifyContext* ctx) {
459460
}
460461

461462
// static
462-
void ContextifyContext::PropertyGetterCallback(
463-
Local<Name> property,
464-
const PropertyCallbackInfo<Value>& args) {
463+
Intercepted ContextifyContext::PropertyGetterCallback(
464+
Local<Name> property, const PropertyCallbackInfo<Value>& args) {
465465
ContextifyContext* ctx = ContextifyContext::Get(args);
466466

467467
// Still initializing
468-
if (IsStillInitializing(ctx)) return;
468+
if (IsStillInitializing(ctx)) return Intercepted::kNo;
469469

470470
Local<Context> context = ctx->context();
471471
Local<Object> sandbox = ctx->sandbox();
@@ -482,18 +482,20 @@ void ContextifyContext::PropertyGetterCallback(
482482
rv = ctx->global_proxy();
483483

484484
args.GetReturnValue().Set(rv);
485+
return Intercepted::kYes;
485486
}
487+
return Intercepted::kNo;
486488
}
487489

488490
// static
489-
void ContextifyContext::PropertySetterCallback(
491+
Intercepted ContextifyContext::PropertySetterCallback(
490492
Local<Name> property,
491493
Local<Value> value,
492-
const PropertyCallbackInfo<Value>& args) {
494+
const PropertyCallbackInfo<void>& args) {
493495
ContextifyContext* ctx = ContextifyContext::Get(args);
494496

495497
// Still initializing
496-
if (IsStillInitializing(ctx)) return;
498+
if (IsStillInitializing(ctx)) return Intercepted::kNo;
497499

498500
Local<Context> context = ctx->context();
499501
PropertyAttribute attributes = PropertyAttribute::None;
@@ -511,8 +513,7 @@ void ContextifyContext::PropertySetterCallback(
511513
(static_cast<int>(attributes) &
512514
static_cast<int>(PropertyAttribute::ReadOnly));
513515

514-
if (read_only)
515-
return;
516+
if (read_only) return Intercepted::kNo;
516517

517518
// true for x = 5
518519
// false for this.x = 5
@@ -532,10 +533,11 @@ void ContextifyContext::PropertySetterCallback(
532533
bool is_declared = is_declared_on_global_proxy || is_declared_on_sandbox;
533534
if (!is_declared && args.ShouldThrowOnError() && is_contextual_store &&
534535
!is_function)
535-
return;
536+
return Intercepted::kNo;
536537

537-
if (!is_declared && property->IsSymbol()) return;
538-
if (ctx->sandbox()->Set(context, property, value).IsNothing()) return;
538+
if (!is_declared && property->IsSymbol()) return Intercepted::kNo;
539+
if (ctx->sandbox()->Set(context, property, value).IsNothing())
540+
return Intercepted::kNo;
539541

540542
Local<Value> desc;
541543
if (is_declared_on_sandbox &&
@@ -549,19 +551,21 @@ void ContextifyContext::PropertySetterCallback(
549551
// We have to specify the return value for any contextual or get/set
550552
// property
551553
if (desc_obj->HasOwnProperty(context, env->get_string()).FromMaybe(false) ||
552-
desc_obj->HasOwnProperty(context, env->set_string()).FromMaybe(false))
554+
desc_obj->HasOwnProperty(context, env->set_string()).FromMaybe(false)) {
553555
args.GetReturnValue().Set(value);
556+
return Intercepted::kYes;
557+
}
554558
}
559+
return Intercepted::kNo;
555560
}
556561

557562
// static
558-
void ContextifyContext::PropertyDescriptorCallback(
559-
Local<Name> property,
560-
const PropertyCallbackInfo<Value>& args) {
563+
Intercepted ContextifyContext::PropertyDescriptorCallback(
564+
Local<Name> property, const PropertyCallbackInfo<Value>& args) {
561565
ContextifyContext* ctx = ContextifyContext::Get(args);
562566

563567
// Still initializing
564-
if (IsStillInitializing(ctx)) return;
568+
if (IsStillInitializing(ctx)) return Intercepted::kNo;
565569

566570
Local<Context> context = ctx->context();
567571

@@ -571,19 +575,21 @@ void ContextifyContext::PropertyDescriptorCallback(
571575
Local<Value> desc;
572576
if (sandbox->GetOwnPropertyDescriptor(context, property).ToLocal(&desc)) {
573577
args.GetReturnValue().Set(desc);
578+
return Intercepted::kYes;
574579
}
575580
}
581+
return Intercepted::kNo;
576582
}
577583

578584
// static
579-
void ContextifyContext::PropertyDefinerCallback(
585+
Intercepted ContextifyContext::PropertyDefinerCallback(
580586
Local<Name> property,
581587
const PropertyDescriptor& desc,
582-
const PropertyCallbackInfo<Value>& args) {
588+
const PropertyCallbackInfo<void>& args) {
583589
ContextifyContext* ctx = ContextifyContext::Get(args);
584590

585591
// Still initializing
586-
if (IsStillInitializing(ctx)) return;
592+
if (IsStillInitializing(ctx)) return Intercepted::kNo;
587593

588594
Local<Context> context = ctx->context();
589595
Isolate* isolate = context->GetIsolate();
@@ -602,7 +608,7 @@ void ContextifyContext::PropertyDefinerCallback(
602608
// If the property is set on the global as neither writable nor
603609
// configurable, don't change it on the global or sandbox.
604610
if (is_declared && read_only && dont_delete) {
605-
return;
611+
return Intercepted::kNo;
606612
}
607613

608614
Local<Object> sandbox = ctx->sandbox();
@@ -625,6 +631,9 @@ void ContextifyContext::PropertyDefinerCallback(
625631
desc.has_set() ? desc.set() : Undefined(isolate).As<Value>());
626632

627633
define_prop_on_sandbox(&desc_for_sandbox);
634+
// TODO(https://github.com/nodejs/node/issues/52634): this should return
635+
// kYes to behave according to the expected semantics.
636+
return Intercepted::kNo;
628637
} else {
629638
Local<Value> value =
630639
desc.has_value() ? desc.value() : Undefined(isolate).As<Value>();
@@ -636,26 +645,29 @@ void ContextifyContext::PropertyDefinerCallback(
636645
PropertyDescriptor desc_for_sandbox(value);
637646
define_prop_on_sandbox(&desc_for_sandbox);
638647
}
648+
// TODO(https://github.com/nodejs/node/issues/52634): this should return
649+
// kYes to behave according to the expected semantics.
650+
return Intercepted::kNo;
639651
}
652+
return Intercepted::kNo;
640653
}
641654

642655
// static
643-
void ContextifyContext::PropertyDeleterCallback(
644-
Local<Name> property,
645-
const PropertyCallbackInfo<Boolean>& args) {
656+
Intercepted ContextifyContext::PropertyDeleterCallback(
657+
Local<Name> property, const PropertyCallbackInfo<Boolean>& args) {
646658
ContextifyContext* ctx = ContextifyContext::Get(args);
647659

648660
// Still initializing
649-
if (IsStillInitializing(ctx)) return;
661+
if (IsStillInitializing(ctx)) return Intercepted::kNo;
650662

651663
Maybe<bool> success = ctx->sandbox()->Delete(ctx->context(), property);
652664

653-
if (success.FromMaybe(false))
654-
return;
665+
if (success.FromMaybe(false)) return Intercepted::kNo;
655666

656667
// Delete failed on the sandbox, intercept and do not delete on
657668
// the global object.
658669
args.GetReturnValue().Set(false);
670+
return Intercepted::kYes;
659671
}
660672

661673
// static
@@ -675,76 +687,71 @@ void ContextifyContext::PropertyEnumeratorCallback(
675687
}
676688

677689
// static
678-
void ContextifyContext::IndexedPropertyGetterCallback(
679-
uint32_t index,
680-
const PropertyCallbackInfo<Value>& args) {
690+
Intercepted ContextifyContext::IndexedPropertyGetterCallback(
691+
uint32_t index, const PropertyCallbackInfo<Value>& args) {
681692
ContextifyContext* ctx = ContextifyContext::Get(args);
682693

683694
// Still initializing
684-
if (IsStillInitializing(ctx)) return;
695+
if (IsStillInitializing(ctx)) return Intercepted::kNo;
685696

686-
ContextifyContext::PropertyGetterCallback(
697+
return ContextifyContext::PropertyGetterCallback(
687698
Uint32ToName(ctx->context(), index), args);
688699
}
689700

690-
691-
void ContextifyContext::IndexedPropertySetterCallback(
701+
Intercepted ContextifyContext::IndexedPropertySetterCallback(
692702
uint32_t index,
693703
Local<Value> value,
694-
const PropertyCallbackInfo<Value>& args) {
704+
const PropertyCallbackInfo<void>& args) {
695705
ContextifyContext* ctx = ContextifyContext::Get(args);
696706

697707
// Still initializing
698-
if (IsStillInitializing(ctx)) return;
708+
if (IsStillInitializing(ctx)) return Intercepted::kNo;
699709

700-
ContextifyContext::PropertySetterCallback(
710+
return ContextifyContext::PropertySetterCallback(
701711
Uint32ToName(ctx->context(), index), value, args);
702712
}
703713

704714
// static
705-
void ContextifyContext::IndexedPropertyDescriptorCallback(
706-
uint32_t index,
707-
const PropertyCallbackInfo<Value>& args) {
715+
Intercepted ContextifyContext::IndexedPropertyDescriptorCallback(
716+
uint32_t index, const PropertyCallbackInfo<Value>& args) {
708717
ContextifyContext* ctx = ContextifyContext::Get(args);
709718

710719
// Still initializing
711-
if (IsStillInitializing(ctx)) return;
720+
if (IsStillInitializing(ctx)) return Intercepted::kNo;
712721

713-
ContextifyContext::PropertyDescriptorCallback(
722+
return ContextifyContext::PropertyDescriptorCallback(
714723
Uint32ToName(ctx->context(), index), args);
715724
}
716725

717-
718-
void ContextifyContext::IndexedPropertyDefinerCallback(
726+
Intercepted ContextifyContext::IndexedPropertyDefinerCallback(
719727
uint32_t index,
720728
const PropertyDescriptor& desc,
721-
const PropertyCallbackInfo<Value>& args) {
729+
const PropertyCallbackInfo<void>& args) {
722730
ContextifyContext* ctx = ContextifyContext::Get(args);
723731

724732
// Still initializing
725-
if (IsStillInitializing(ctx)) return;
733+
if (IsStillInitializing(ctx)) return Intercepted::kNo;
726734

727-
ContextifyContext::PropertyDefinerCallback(
735+
return ContextifyContext::PropertyDefinerCallback(
728736
Uint32ToName(ctx->context(), index), desc, args);
729737
}
730738

731739
// static
732-
void ContextifyContext::IndexedPropertyDeleterCallback(
733-
uint32_t index,
734-
const PropertyCallbackInfo<Boolean>& args) {
740+
Intercepted ContextifyContext::IndexedPropertyDeleterCallback(
741+
uint32_t index, const PropertyCallbackInfo<Boolean>& args) {
735742
ContextifyContext* ctx = ContextifyContext::Get(args);
736743

737744
// Still initializing
738-
if (IsStillInitializing(ctx)) return;
745+
if (IsStillInitializing(ctx)) return Intercepted::kNo;
739746

740747
Maybe<bool> success = ctx->sandbox()->Delete(ctx->context(), index);
741748

742-
if (success.FromMaybe(false))
743-
return;
749+
if (success.FromMaybe(false)) return Intercepted::kNo;
744750

745751
// Delete failed on the sandbox, intercept and do not delete on
746752
// the global object.
747753
args.GetReturnValue().Set(false);
754+
return Intercepted::kYes;
748755
}
749756

750757
void ContextifyScript::CreatePerIsolateProperties(

‎src/node_contextify.h

+17-20
Original file line numberDiff line numberDiff line change
@@ -111,42 +111,39 @@ class ContextifyContext : public BaseObject {
111111
v8::Local<v8::String> code);
112112
static void WeakCallback(
113113
const v8::WeakCallbackInfo<ContextifyContext>& data);
114-
static void PropertyGetterCallback(
114+
static v8::Intercepted PropertyGetterCallback(
115115
v8::Local<v8::Name> property,
116116
const v8::PropertyCallbackInfo<v8::Value>& args);
117-
static void PropertySetterCallback(
117+
static v8::Intercepted PropertySetterCallback(
118118
v8::Local<v8::Name> property,
119119
v8::Local<v8::Value> value,
120-
const v8::PropertyCallbackInfo<v8::Value>& args);
121-
static void PropertyDescriptorCallback(
120+
const v8::PropertyCallbackInfo<void>& args);
121+
static v8::Intercepted PropertyDescriptorCallback(
122122
v8::Local<v8::Name> property,
123123
const v8::PropertyCallbackInfo<v8::Value>& args);
124-
static void PropertyDefinerCallback(
124+
static v8::Intercepted PropertyDefinerCallback(
125125
v8::Local<v8::Name> property,
126126
const v8::PropertyDescriptor& desc,
127-
const v8::PropertyCallbackInfo<v8::Value>& args);
128-
static void PropertyDeleterCallback(
127+
const v8::PropertyCallbackInfo<void>& args);
128+
static v8::Intercepted PropertyDeleterCallback(
129129
v8::Local<v8::Name> property,
130130
const v8::PropertyCallbackInfo<v8::Boolean>& args);
131131
static void PropertyEnumeratorCallback(
132132
const v8::PropertyCallbackInfo<v8::Array>& args);
133-
static void IndexedPropertyGetterCallback(
134-
uint32_t index,
135-
const v8::PropertyCallbackInfo<v8::Value>& args);
136-
static void IndexedPropertySetterCallback(
133+
static v8::Intercepted IndexedPropertyGetterCallback(
134+
uint32_t index, const v8::PropertyCallbackInfo<v8::Value>& args);
135+
static v8::Intercepted IndexedPropertySetterCallback(
137136
uint32_t index,
138137
v8::Local<v8::Value> value,
139-
const v8::PropertyCallbackInfo<v8::Value>& args);
140-
static void IndexedPropertyDescriptorCallback(
141-
uint32_t index,
142-
const v8::PropertyCallbackInfo<v8::Value>& args);
143-
static void IndexedPropertyDefinerCallback(
138+
const v8::PropertyCallbackInfo<void>& args);
139+
static v8::Intercepted IndexedPropertyDescriptorCallback(
140+
uint32_t index, const v8::PropertyCallbackInfo<v8::Value>& args);
141+
static v8::Intercepted IndexedPropertyDefinerCallback(
144142
uint32_t index,
145143
const v8::PropertyDescriptor& desc,
146-
const v8::PropertyCallbackInfo<v8::Value>& args);
147-
static void IndexedPropertyDeleterCallback(
148-
uint32_t index,
149-
const v8::PropertyCallbackInfo<v8::Boolean>& args);
144+
const v8::PropertyCallbackInfo<void>& args);
145+
static v8::Intercepted IndexedPropertyDeleterCallback(
146+
uint32_t index, const v8::PropertyCallbackInfo<v8::Boolean>& args);
150147

151148
v8::Global<v8::Context> context_;
152149
std::unique_ptr<v8::MicrotaskQueue> microtask_queue_;

0 commit comments

Comments
 (0)