Skip to content

Commit cdc6331

Browse files
victorgomesvictorgomes
committed
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. # Conflicts: # src/node_contextify.cc
1 parent 6b056bf commit cdc6331

File tree

4 files changed

+119
-102
lines changed

4 files changed

+119
-102
lines changed

‎src/node_contextify.cc‎

Lines changed: 60 additions & 53 deletions
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;
@@ -455,14 +456,13 @@ bool ContextifyContext::IsStillInitializing(const ContextifyContext* ctx) {
455456
}
456457

457458
// static
458-
void ContextifyContext::PropertyGetterCallback(
459-
Local<Name> property,
460-
const PropertyCallbackInfo<Value>& args) {
459+
Intercepted ContextifyContext::PropertyGetterCallback(
460+
Local<Name> property, const PropertyCallbackInfo<Value>& args) {
461461
Environment* env = Environment::GetCurrent(args);
462462
ContextifyContext* ctx = ContextifyContext::Get(args);
463463

464464
// Still initializing
465-
if (IsStillInitializing(ctx)) return;
465+
if (IsStillInitializing(ctx)) return Intercepted::kNo;
466466

467467
Local<Context> context = ctx->context();
468468
Local<Object> sandbox = ctx->sandbox();
@@ -484,18 +484,20 @@ void ContextifyContext::PropertyGetterCallback(
484484
rv = ctx->global_proxy();
485485

486486
args.GetReturnValue().Set(rv);
487+
return Intercepted::kYes;
487488
}
489+
return Intercepted::kNo;
488490
}
489491

490492
// static
491-
void ContextifyContext::PropertySetterCallback(
493+
Intercepted ContextifyContext::PropertySetterCallback(
492494
Local<Name> property,
493495
Local<Value> value,
494-
const PropertyCallbackInfo<Value>& args) {
496+
const PropertyCallbackInfo<void>& args) {
495497
ContextifyContext* ctx = ContextifyContext::Get(args);
496498

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

500502
Local<Context> context = ctx->context();
501503
PropertyAttribute attributes = PropertyAttribute::None;
@@ -513,8 +515,7 @@ void ContextifyContext::PropertySetterCallback(
513515
(static_cast<int>(attributes) &
514516
static_cast<int>(PropertyAttribute::ReadOnly));
515517

516-
if (read_only)
517-
return;
518+
if (read_only) return Intercepted::kNo;
518519

519520
// true for x = 5
520521
// false for this.x = 5
@@ -534,10 +535,11 @@ void ContextifyContext::PropertySetterCallback(
534535
bool is_declared = is_declared_on_global_proxy || is_declared_on_sandbox;
535536
if (!is_declared && args.ShouldThrowOnError() && is_contextual_store &&
536537
!is_function)
537-
return;
538+
return Intercepted::kNo;
538539

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

542544
Local<Value> desc;
543545
if (is_declared_on_sandbox &&
@@ -551,19 +553,21 @@ void ContextifyContext::PropertySetterCallback(
551553
// We have to specify the return value for any contextual or get/set
552554
// property
553555
if (desc_obj->HasOwnProperty(context, env->get_string()).FromMaybe(false) ||
554-
desc_obj->HasOwnProperty(context, env->set_string()).FromMaybe(false))
556+
desc_obj->HasOwnProperty(context, env->set_string()).FromMaybe(false)) {
555557
args.GetReturnValue().Set(value);
558+
return Intercepted::kYes;
559+
}
556560
}
561+
return Intercepted::kNo;
557562
}
558563

559564
// static
560-
void ContextifyContext::PropertyDescriptorCallback(
561-
Local<Name> property,
562-
const PropertyCallbackInfo<Value>& args) {
565+
Intercepted ContextifyContext::PropertyDescriptorCallback(
566+
Local<Name> property, const PropertyCallbackInfo<Value>& args) {
563567
ContextifyContext* ctx = ContextifyContext::Get(args);
564568

565569
// Still initializing
566-
if (IsStillInitializing(ctx)) return;
570+
if (IsStillInitializing(ctx)) return Intercepted::kNo;
567571

568572
Local<Context> context = ctx->context();
569573

@@ -573,19 +577,21 @@ void ContextifyContext::PropertyDescriptorCallback(
573577
Local<Value> desc;
574578
if (sandbox->GetOwnPropertyDescriptor(context, property).ToLocal(&desc)) {
575579
args.GetReturnValue().Set(desc);
580+
return Intercepted::kYes;
576581
}
577582
}
583+
return Intercepted::kNo;
578584
}
579585

580586
// static
581-
void ContextifyContext::PropertyDefinerCallback(
587+
Intercepted ContextifyContext::PropertyDefinerCallback(
582588
Local<Name> property,
583589
const PropertyDescriptor& desc,
584-
const PropertyCallbackInfo<Value>& args) {
590+
const PropertyCallbackInfo<void>& args) {
585591
ContextifyContext* ctx = ContextifyContext::Get(args);
586592

587593
// Still initializing
588-
if (IsStillInitializing(ctx)) return;
594+
if (IsStillInitializing(ctx)) return Intercepted::kNo;
589595

590596
Local<Context> context = ctx->context();
591597
Isolate* isolate = context->GetIsolate();
@@ -604,7 +610,7 @@ void ContextifyContext::PropertyDefinerCallback(
604610
// If the property is set on the global as neither writable nor
605611
// configurable, don't change it on the global or sandbox.
606612
if (is_declared && read_only && dont_delete) {
607-
return;
613+
return Intercepted::kNo;
608614
}
609615

610616
Local<Object> sandbox = ctx->sandbox();
@@ -627,6 +633,9 @@ void ContextifyContext::PropertyDefinerCallback(
627633
desc.has_set() ? desc.set() : Undefined(isolate).As<Value>());
628634

629635
define_prop_on_sandbox(&desc_for_sandbox);
636+
// TODO(https://github.com/nodejs/node/issues/52634): this should return
637+
// kYes to behave according to the expected semantics.
638+
return Intercepted::kNo;
630639
} else {
631640
Local<Value> value =
632641
desc.has_value() ? desc.value() : Undefined(isolate).As<Value>();
@@ -638,26 +647,29 @@ void ContextifyContext::PropertyDefinerCallback(
638647
PropertyDescriptor desc_for_sandbox(value);
639648
define_prop_on_sandbox(&desc_for_sandbox);
640649
}
650+
// TODO(https://github.com/nodejs/node/issues/52634): this should return
651+
// kYes to behave according to the expected semantics.
652+
return Intercepted::kNo;
641653
}
654+
return Intercepted::kNo;
642655
}
643656

644657
// static
645-
void ContextifyContext::PropertyDeleterCallback(
646-
Local<Name> property,
647-
const PropertyCallbackInfo<Boolean>& args) {
658+
Intercepted ContextifyContext::PropertyDeleterCallback(
659+
Local<Name> property, const PropertyCallbackInfo<Boolean>& args) {
648660
ContextifyContext* ctx = ContextifyContext::Get(args);
649661

650662
// Still initializing
651-
if (IsStillInitializing(ctx)) return;
663+
if (IsStillInitializing(ctx)) return Intercepted::kNo;
652664

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

655-
if (success.FromMaybe(false))
656-
return;
667+
if (success.FromMaybe(false)) return Intercepted::kNo;
657668

658669
// Delete failed on the sandbox, intercept and do not delete on
659670
// the global object.
660671
args.GetReturnValue().Set(false);
672+
return Intercepted::kYes;
661673
}
662674

663675
// static
@@ -677,76 +689,71 @@ void ContextifyContext::PropertyEnumeratorCallback(
677689
}
678690

679691
// static
680-
void ContextifyContext::IndexedPropertyGetterCallback(
681-
uint32_t index,
682-
const PropertyCallbackInfo<Value>& args) {
692+
Intercepted ContextifyContext::IndexedPropertyGetterCallback(
693+
uint32_t index, const PropertyCallbackInfo<Value>& args) {
683694
ContextifyContext* ctx = ContextifyContext::Get(args);
684695

685696
// Still initializing
686-
if (IsStillInitializing(ctx)) return;
697+
if (IsStillInitializing(ctx)) return Intercepted::kNo;
687698

688-
ContextifyContext::PropertyGetterCallback(
699+
return ContextifyContext::PropertyGetterCallback(
689700
Uint32ToName(ctx->context(), index), args);
690701
}
691702

692-
693-
void ContextifyContext::IndexedPropertySetterCallback(
703+
Intercepted ContextifyContext::IndexedPropertySetterCallback(
694704
uint32_t index,
695705
Local<Value> value,
696-
const PropertyCallbackInfo<Value>& args) {
706+
const PropertyCallbackInfo<void>& args) {
697707
ContextifyContext* ctx = ContextifyContext::Get(args);
698708

699709
// Still initializing
700-
if (IsStillInitializing(ctx)) return;
710+
if (IsStillInitializing(ctx)) return Intercepted::kNo;
701711

702-
ContextifyContext::PropertySetterCallback(
712+
return ContextifyContext::PropertySetterCallback(
703713
Uint32ToName(ctx->context(), index), value, args);
704714
}
705715

706716
// static
707-
void ContextifyContext::IndexedPropertyDescriptorCallback(
708-
uint32_t index,
709-
const PropertyCallbackInfo<Value>& args) {
717+
Intercepted ContextifyContext::IndexedPropertyDescriptorCallback(
718+
uint32_t index, const PropertyCallbackInfo<Value>& args) {
710719
ContextifyContext* ctx = ContextifyContext::Get(args);
711720

712721
// Still initializing
713-
if (IsStillInitializing(ctx)) return;
722+
if (IsStillInitializing(ctx)) return Intercepted::kNo;
714723

715-
ContextifyContext::PropertyDescriptorCallback(
724+
return ContextifyContext::PropertyDescriptorCallback(
716725
Uint32ToName(ctx->context(), index), args);
717726
}
718727

719-
720-
void ContextifyContext::IndexedPropertyDefinerCallback(
728+
Intercepted ContextifyContext::IndexedPropertyDefinerCallback(
721729
uint32_t index,
722730
const PropertyDescriptor& desc,
723-
const PropertyCallbackInfo<Value>& args) {
731+
const PropertyCallbackInfo<void>& args) {
724732
ContextifyContext* ctx = ContextifyContext::Get(args);
725733

726734
// Still initializing
727-
if (IsStillInitializing(ctx)) return;
735+
if (IsStillInitializing(ctx)) return Intercepted::kNo;
728736

729-
ContextifyContext::PropertyDefinerCallback(
737+
return ContextifyContext::PropertyDefinerCallback(
730738
Uint32ToName(ctx->context(), index), desc, args);
731739
}
732740

733741
// static
734-
void ContextifyContext::IndexedPropertyDeleterCallback(
735-
uint32_t index,
736-
const PropertyCallbackInfo<Boolean>& args) {
742+
Intercepted ContextifyContext::IndexedPropertyDeleterCallback(
743+
uint32_t index, const PropertyCallbackInfo<Boolean>& args) {
737744
ContextifyContext* ctx = ContextifyContext::Get(args);
738745

739746
// Still initializing
740-
if (IsStillInitializing(ctx)) return;
747+
if (IsStillInitializing(ctx)) return Intercepted::kNo;
741748

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

744-
if (success.FromMaybe(false))
745-
return;
751+
if (success.FromMaybe(false)) return Intercepted::kNo;
746752

747753
// Delete failed on the sandbox, intercept and do not delete on
748754
// the global object.
749755
args.GetReturnValue().Set(false);
756+
return Intercepted::kYes;
750757
}
751758

752759
void ContextifyScript::CreatePerIsolateProperties(

‎src/node_contextify.h‎

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -96,42 +96,39 @@ class ContextifyContext : public BaseObject {
9696
const errors::TryCatchScope& try_catch);
9797
static void WeakCallback(
9898
const v8::WeakCallbackInfo<ContextifyContext>& data);
99-
static void PropertyGetterCallback(
99+
static v8::Intercepted PropertyGetterCallback(
100100
v8::Local<v8::Name> property,
101101
const v8::PropertyCallbackInfo<v8::Value>& args);
102-
static void PropertySetterCallback(
102+
static v8::Intercepted PropertySetterCallback(
103103
v8::Local<v8::Name> property,
104104
v8::Local<v8::Value> value,
105-
const v8::PropertyCallbackInfo<v8::Value>& args);
106-
static void PropertyDescriptorCallback(
105+
const v8::PropertyCallbackInfo<void>& args);
106+
static v8::Intercepted PropertyDescriptorCallback(
107107
v8::Local<v8::Name> property,
108108
const v8::PropertyCallbackInfo<v8::Value>& args);
109-
static void PropertyDefinerCallback(
109+
static v8::Intercepted PropertyDefinerCallback(
110110
v8::Local<v8::Name> property,
111111
const v8::PropertyDescriptor& desc,
112-
const v8::PropertyCallbackInfo<v8::Value>& args);
113-
static void PropertyDeleterCallback(
112+
const v8::PropertyCallbackInfo<void>& args);
113+
static v8::Intercepted PropertyDeleterCallback(
114114
v8::Local<v8::Name> property,
115115
const v8::PropertyCallbackInfo<v8::Boolean>& args);
116116
static void PropertyEnumeratorCallback(
117117
const v8::PropertyCallbackInfo<v8::Array>& args);
118-
static void IndexedPropertyGetterCallback(
119-
uint32_t index,
120-
const v8::PropertyCallbackInfo<v8::Value>& args);
121-
static void IndexedPropertySetterCallback(
118+
static v8::Intercepted IndexedPropertyGetterCallback(
119+
uint32_t index, const v8::PropertyCallbackInfo<v8::Value>& args);
120+
static v8::Intercepted IndexedPropertySetterCallback(
122121
uint32_t index,
123122
v8::Local<v8::Value> value,
124-
const v8::PropertyCallbackInfo<v8::Value>& args);
125-
static void IndexedPropertyDescriptorCallback(
126-
uint32_t index,
127-
const v8::PropertyCallbackInfo<v8::Value>& args);
128-
static void IndexedPropertyDefinerCallback(
123+
const v8::PropertyCallbackInfo<void>& args);
124+
static v8::Intercepted IndexedPropertyDescriptorCallback(
125+
uint32_t index, const v8::PropertyCallbackInfo<v8::Value>& args);
126+
static v8::Intercepted IndexedPropertyDefinerCallback(
129127
uint32_t index,
130128
const v8::PropertyDescriptor& desc,
131-
const v8::PropertyCallbackInfo<v8::Value>& args);
132-
static void IndexedPropertyDeleterCallback(
133-
uint32_t index,
134-
const v8::PropertyCallbackInfo<v8::Boolean>& args);
129+
const v8::PropertyCallbackInfo<void>& args);
130+
static v8::Intercepted IndexedPropertyDeleterCallback(
131+
uint32_t index, const v8::PropertyCallbackInfo<v8::Boolean>& args);
135132

136133
v8::Global<v8::Context> context_;
137134
std::unique_ptr<v8::MicrotaskQueue> microtask_queue_;

0 commit comments

Comments
 (0)