Skip to content

Commit 8989c76

Browse files
committed
Revert "src: implement query callbacks for vm"
This reverts commit 85c356c from PR #22390. See the discussion in the (proposed) fix at #22836. Refs: #22836 Refs: #22390 Fixes: #22723 PR-URL: #22911 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: John-David Dalton <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
1 parent dcc0c2c commit 8989c76

File tree

4 files changed

+2
-171
lines changed

4 files changed

+2
-171
lines changed

doc/api/vm.md

-42
Original file line numberDiff line numberDiff line change
@@ -944,48 +944,6 @@ within which it can operate. The process of creating the V8 Context and
944944
associating it with the `sandbox` object is what this document refers to as
945945
"contextifying" the `sandbox`.
946946

947-
## vm module and Proxy object
948-
949-
Leveraging a `Proxy` object as the sandbox of a VM context could result in a
950-
very powerful runtime environment that intercepts all accesses to the global
951-
object. However, there are some restrictions in the JavaScript engine that one
952-
needs to be aware of to prevent unexpected results. In particular, providing a
953-
`Proxy` object with a `get` handler could disallow any access to the original
954-
global properties of the new VM context, as the `get` hook does not distinguish
955-
between the `undefined` value and "requested property is not present" &ndash;
956-
the latter of which would ordinarily trigger a lookup on the context global
957-
object.
958-
959-
Included below is a sample for how to work around this restriction. It
960-
initializes the sandbox as a `Proxy` object without any hooks, only to add them
961-
after the relevant properties have been saved.
962-
963-
```js
964-
'use strict';
965-
const { createContext, runInContext } = require('vm');
966-
967-
function createProxySandbox(handlers) {
968-
// Create a VM context with a Proxy object with no hooks specified.
969-
const sandbox = {};
970-
const proxyHandlers = {};
971-
const contextifiedProxy = createContext(new Proxy(sandbox, proxyHandlers));
972-
973-
// Save the initial globals onto our sandbox object.
974-
const contextThis = runInContext('this', contextifiedProxy);
975-
for (const prop of Reflect.ownKeys(contextThis)) {
976-
const descriptor = Object.getOwnPropertyDescriptor(contextThis, prop);
977-
Object.defineProperty(sandbox, prop, descriptor);
978-
}
979-
980-
// Now that `sandbox` contains all the initial global properties, assign the
981-
// provided handlers to the handlers we used to create the Proxy.
982-
Object.assign(proxyHandlers, handlers);
983-
984-
// Return the created contextified Proxy object.
985-
return contextifiedProxy;
986-
}
987-
```
988-
989947
[`Error`]: errors.html#errors_class_error
990948
[`URL`]: url.html#url_class_url
991949
[`eval()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/eval

src/node_contextify.cc

+2-40
Original file line numberDiff line numberDiff line change
@@ -143,21 +143,19 @@ Local<Context> ContextifyContext::CreateV8Context(
143143

144144
NamedPropertyHandlerConfiguration config(PropertyGetterCallback,
145145
PropertySetterCallback,
146-
PropertyQueryCallback,
146+
PropertyDescriptorCallback,
147147
PropertyDeleterCallback,
148148
PropertyEnumeratorCallback,
149149
PropertyDefinerCallback,
150-
PropertyDescriptorCallback,
151150
CreateDataWrapper(env));
152151

153152
IndexedPropertyHandlerConfiguration indexed_config(
154153
IndexedPropertyGetterCallback,
155154
IndexedPropertySetterCallback,
156-
IndexedPropertyQueryCallback,
155+
IndexedPropertyDescriptorCallback,
157156
IndexedPropertyDeleterCallback,
158157
PropertyEnumeratorCallback,
159158
IndexedPropertyDefinerCallback,
160-
IndexedPropertyDescriptorCallback,
161159
CreateDataWrapper(env));
162160

163161
object_template->SetHandler(config);
@@ -393,28 +391,6 @@ void ContextifyContext::PropertySetterCallback(
393391
ctx->sandbox()->Set(property, value);
394392
}
395393

396-
// static
397-
void ContextifyContext::PropertyQueryCallback(
398-
Local<Name> property,
399-
const PropertyCallbackInfo<Integer>& args) {
400-
ContextifyContext* ctx = ContextifyContext::Get(args);
401-
402-
// Still initializing
403-
if (ctx->context_.IsEmpty())
404-
return;
405-
406-
Local<Context> context = ctx->context();
407-
408-
Local<Object> sandbox = ctx->sandbox();
409-
410-
PropertyAttribute attributes;
411-
if (sandbox->HasOwnProperty(context, property).FromMaybe(false) &&
412-
sandbox->GetPropertyAttributes(context, property).To(&attributes)) {
413-
args.GetReturnValue().Set(attributes);
414-
}
415-
}
416-
417-
418394
// static
419395
void ContextifyContext::PropertyDescriptorCallback(
420396
Local<Name> property,
@@ -560,20 +536,6 @@ void ContextifyContext::IndexedPropertySetterCallback(
560536
Uint32ToName(ctx->context(), index), value, args);
561537
}
562538

563-
// static
564-
void ContextifyContext::IndexedPropertyQueryCallback(
565-
uint32_t index,
566-
const PropertyCallbackInfo<Integer>& args) {
567-
ContextifyContext* ctx = ContextifyContext::Get(args);
568-
569-
// Still initializing
570-
if (ctx->context_.IsEmpty())
571-
return;
572-
573-
ContextifyContext::PropertyQueryCallback(
574-
Uint32ToName(ctx->context(), index), args);
575-
}
576-
577539
// static
578540
void ContextifyContext::IndexedPropertyDescriptorCallback(
579541
uint32_t index,

src/node_contextify.h

-6
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,6 @@ class ContextifyContext {
6969
v8::Local<v8::Name> property,
7070
v8::Local<v8::Value> value,
7171
const v8::PropertyCallbackInfo<v8::Value>& args);
72-
static void PropertyQueryCallback(
73-
v8::Local<v8::Name> property,
74-
const v8::PropertyCallbackInfo<v8::Integer>& args);
7572
static void PropertyDescriptorCallback(
7673
v8::Local<v8::Name> property,
7774
const v8::PropertyCallbackInfo<v8::Value>& args);
@@ -91,9 +88,6 @@ class ContextifyContext {
9188
uint32_t index,
9289
v8::Local<v8::Value> value,
9390
const v8::PropertyCallbackInfo<v8::Value>& args);
94-
static void IndexedPropertyQueryCallback(
95-
uint32_t index,
96-
const v8::PropertyCallbackInfo<v8::Integer>& args);
9791
static void IndexedPropertyDescriptorCallback(
9892
uint32_t index,
9993
const v8::PropertyCallbackInfo<v8::Value>& args);

test/parallel/test-vm-proxy.js

-83
This file was deleted.

0 commit comments

Comments
 (0)