Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "src: implement query callbacks for vm" #22911

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 0 additions & 42 deletions doc/api/vm.md
Original file line number Diff line number Diff line change
Expand Up @@ -944,48 +944,6 @@ within which it can operate. The process of creating the V8 Context and
associating it with the `sandbox` object is what this document refers to as
"contextifying" the `sandbox`.

## vm module and Proxy object

Leveraging a `Proxy` object as the sandbox of a VM context could result in a
very powerful runtime environment that intercepts all accesses to the global
object. However, there are some restrictions in the JavaScript engine that one
needs to be aware of to prevent unexpected results. In particular, providing a
`Proxy` object with a `get` handler could disallow any access to the original
global properties of the new VM context, as the `get` hook does not distinguish
between the `undefined` value and "requested property is not present" –
the latter of which would ordinarily trigger a lookup on the context global
object.

Included below is a sample for how to work around this restriction. It
initializes the sandbox as a `Proxy` object without any hooks, only to add them
after the relevant properties have been saved.

```js
'use strict';
const { createContext, runInContext } = require('vm');

function createProxySandbox(handlers) {
// Create a VM context with a Proxy object with no hooks specified.
const sandbox = {};
const proxyHandlers = {};
const contextifiedProxy = createContext(new Proxy(sandbox, proxyHandlers));

// Save the initial globals onto our sandbox object.
const contextThis = runInContext('this', contextifiedProxy);
for (const prop of Reflect.ownKeys(contextThis)) {
const descriptor = Object.getOwnPropertyDescriptor(contextThis, prop);
Object.defineProperty(sandbox, prop, descriptor);
}

// Now that `sandbox` contains all the initial global properties, assign the
// provided handlers to the handlers we used to create the Proxy.
Object.assign(proxyHandlers, handlers);

// Return the created contextified Proxy object.
return contextifiedProxy;
}
```

[`Error`]: errors.html#errors_class_error
[`URL`]: url.html#url_class_url
[`eval()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/eval
Expand Down
42 changes: 2 additions & 40 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -143,21 +143,19 @@ Local<Context> ContextifyContext::CreateV8Context(

NamedPropertyHandlerConfiguration config(PropertyGetterCallback,
PropertySetterCallback,
PropertyQueryCallback,
PropertyDescriptorCallback,
PropertyDeleterCallback,
PropertyEnumeratorCallback,
PropertyDefinerCallback,
PropertyDescriptorCallback,
CreateDataWrapper(env));

IndexedPropertyHandlerConfiguration indexed_config(
IndexedPropertyGetterCallback,
IndexedPropertySetterCallback,
IndexedPropertyQueryCallback,
IndexedPropertyDescriptorCallback,
IndexedPropertyDeleterCallback,
PropertyEnumeratorCallback,
IndexedPropertyDefinerCallback,
IndexedPropertyDescriptorCallback,
CreateDataWrapper(env));

object_template->SetHandler(config);
Expand Down Expand Up @@ -393,28 +391,6 @@ void ContextifyContext::PropertySetterCallback(
ctx->sandbox()->Set(property, value);
}

// static
void ContextifyContext::PropertyQueryCallback(
Local<Name> property,
const PropertyCallbackInfo<Integer>& args) {
ContextifyContext* ctx = ContextifyContext::Get(args);

// Still initializing
if (ctx->context_.IsEmpty())
return;

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

Local<Object> sandbox = ctx->sandbox();

PropertyAttribute attributes;
if (sandbox->HasOwnProperty(context, property).FromMaybe(false) &&
sandbox->GetPropertyAttributes(context, property).To(&attributes)) {
args.GetReturnValue().Set(attributes);
}
}


// static
void ContextifyContext::PropertyDescriptorCallback(
Local<Name> property,
Expand Down Expand Up @@ -560,20 +536,6 @@ void ContextifyContext::IndexedPropertySetterCallback(
Uint32ToName(ctx->context(), index), value, args);
}

// static
void ContextifyContext::IndexedPropertyQueryCallback(
uint32_t index,
const PropertyCallbackInfo<Integer>& args) {
ContextifyContext* ctx = ContextifyContext::Get(args);

// Still initializing
if (ctx->context_.IsEmpty())
return;

ContextifyContext::PropertyQueryCallback(
Uint32ToName(ctx->context(), index), args);
}

// static
void ContextifyContext::IndexedPropertyDescriptorCallback(
uint32_t index,
Expand Down
6 changes: 0 additions & 6 deletions src/node_contextify.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,6 @@ class ContextifyContext {
v8::Local<v8::Name> property,
v8::Local<v8::Value> value,
const v8::PropertyCallbackInfo<v8::Value>& args);
static void PropertyQueryCallback(
v8::Local<v8::Name> property,
const v8::PropertyCallbackInfo<v8::Integer>& args);
static void PropertyDescriptorCallback(
v8::Local<v8::Name> property,
const v8::PropertyCallbackInfo<v8::Value>& args);
Expand All @@ -91,9 +88,6 @@ class ContextifyContext {
uint32_t index,
v8::Local<v8::Value> value,
const v8::PropertyCallbackInfo<v8::Value>& args);
static void IndexedPropertyQueryCallback(
uint32_t index,
const v8::PropertyCallbackInfo<v8::Integer>& args);
static void IndexedPropertyDescriptorCallback(
uint32_t index,
const v8::PropertyCallbackInfo<v8::Value>& args);
Expand Down
83 changes: 0 additions & 83 deletions test/parallel/test-vm-proxy.js

This file was deleted.