From 4725ac61c885b798fd3e1f8416bb1f6b8d0b6af4 Mon Sep 17 00:00:00 2001 From: Gus Caplan Date: Thu, 28 Nov 2019 15:36:27 -0800 Subject: [PATCH 1/2] vm: introduce vm.Context Fixes #855 Fixes #31658 Fixes #31808 --- doc/api/vm.md | 229 +++++++++++------- doc/api/worker_threads.md | 7 +- lib/vm.js | 41 +++- src/node_contextify.cc | 124 ++++++---- test/parallel/test-new-vm-context.js | 42 ++++ test/parallel/test-vm-basic.js | 2 +- test/parallel/test-vm-codegen.js | 16 +- .../test-vm-create-and-run-in-context.js | 14 +- .../parallel/test-vm-function-redefinition.js | 2 +- test/parallel/test-vm-module-basic.js | 33 ++- .../parallel/test-vm-module-dynamic-import.js | 4 +- test/parallel/test-vm-module-errors.js | 5 +- test/parallel/test-vm-strict-assign.js | 4 +- test/parallel/test-vm-strict-mode.js | 5 +- .../parallel/test-worker-message-port-move.js | 9 +- tools/doc/type-parser.js | 1 + 16 files changed, 336 insertions(+), 202 deletions(-) create mode 100644 test/parallel/test-new-vm-context.js diff --git a/doc/api/vm.md b/doc/api/vm.md index bd64b23484eb24..ab2a0684fad81c 100644 --- a/doc/api/vm.md +++ b/doc/api/vm.md @@ -10,36 +10,75 @@ The `vm` module enables compiling and running code within V8 Virtual Machine contexts. **The `vm` module is not a security mechanism. Do not use it to run untrusted code**. -JavaScript code can be compiled and run immediately or -compiled, saved, and run later. +JavaScript code can be compiled and run immediately or compiled, saved, and run +later. -A common use case is to run the code in a different V8 Context. This means -invoked code has a different global object than the invoking code. - -One can provide the context by [_contextifying_][contextified] an -object. The invoked code treats any property in the context like a -global variable. Any changes to global variables caused by the invoked -code are reflected in the context object. +A common use case is to run code within a different environment. Each context +uses a different V8 Context, meaning that it has a different global object than +the rest of the code. One can provide a context by using the [`vm.Context`][] +constructor. ```js const vm = require('vm'); const x = 1; -const context = { x: 2 }; -vm.createContext(context); // Contextify the object. +const context = new vm.Context(); +context.global.x = 2; const code = 'x += 40; var y = 17;'; -// `x` and `y` are global variables in the context. -// Initially, x has the value 2 because that is the value of context.x. -vm.runInContext(code, context); +// `x` and `y` are global variables in the context environment. +// Initially, x has the value 2 because that is the value of context.global.x. +const script = new vm.Script(code); +script.runInContext(context); -console.log(context.x); // 42 -console.log(context.y); // 17 +console.log(context.global.x); // 42 +console.log(context.global.y); // 17 console.log(x); // 1; y is not defined. ``` +## Class: `vm.Context` + + +> Stability: 1 - Experimental + +### Constructor: `new vm.Context([options])` + + +* `options` {Object} + * `name` {string} Human-readable name of the newly created context. + **Default:** `'VM Context i'`, where `i` is an ascending numerical index of + the created context. + * `origin` {string} [Origin][origin] corresponding to the newly created + context for display purposes. The origin should be formatted like a URL, + but with only the scheme, host, and port (if necessary), like the value of + the [`url.origin`][] property of a [`URL`][] object. Most notably, this + string should omit the trailing slash, as that denotes a path. + **Default:** `''`. + * `codeGeneration` {Object} + * `strings` {boolean} If set to false any calls to `eval` or function + constructors (`Function`, `GeneratorFunction`, etc) will throw an + `EvalError`. **Default:** `true`. + * `wasm` {boolean} If set to false any attempt to compile a WebAssembly + module will throw a `WebAssembly.CompileError`. **Default:** `true`. + +Create a new VM Context. This context will have a unique global object and +variable environment. + +### `context.global` + + +* {Object} + +The global object of this context. + ## Class: `vm.Script` -* `contextifiedObject` {Object} A [contextified][] object as returned by the - `vm.createContext()` method. +* `context` {vm.Context} A [`vm.Context`][] instance. * `options` {Object} * `displayErrors` {boolean} When `true`, if an [`Error`][] occurs while compiling the `code`, the line of code causing the error is attached @@ -152,30 +190,28 @@ changes: * Returns: {any} the result of the very last statement executed in the script. Runs the compiled code contained by the `vm.Script` object within the given -`contextifiedObject` and returns the result. Running code does not have access -to local scope. +`context` and returns the result. Running code does not have access to local +scope. The following example compiles code that increments a global variable, sets the value of another global variable, then execute the code multiple times. -The globals are contained in the `context` object. +The globals are contained in the `context`'s global object. ```js const util = require('util'); const vm = require('vm'); -const context = { - animal: 'cat', - count: 2 -}; +const context = new vm.Context(); +context.global.animal = 'cat'; +context.global.count = 2; const script = new vm.Script('count += 1; name = "kitty";'); -vm.createContext(context); for (let i = 0; i < 10; ++i) { script.runInContext(context); } -console.log(context); +console.log(context.global); // Prints: { animal: 'cat', count: 12, name: 'kitty' } ``` @@ -226,6 +262,9 @@ changes: module will throw a `WebAssembly.CompileError`. **Default:** `true`. * Returns: {any} the result of the very last statement executed in the script. +**Warning!** This API exhibits quite a few performance and correctness issues. +If possible, use the [`vm.Context`][] API instead. + First contextifies the given `contextObject`, runs the compiled code contained by the `vm.Script` object within the created context, and returns the result. Running code does not have access to local scope. @@ -327,7 +366,8 @@ support is planned. ```js const vm = require('vm'); -const contextifiedObject = vm.createContext({ secret: 42 }); +const context = new vm.Context(); +context.global.secret = 42; (async () => { // Step 1 @@ -335,7 +375,7 @@ const contextifiedObject = vm.createContext({ secret: 42 }); // Create a Module by constructing a new `vm.SourceTextModule` object. This // parses the provided source text, throwing a `SyntaxError` if anything goes // wrong. By default, a Module is created in the top context. But here, we - // specify `contextifiedObject` as the context this Module belongs to. + // specify `context` as the context this Module belongs to. // // Here, we attempt to obtain the default export from the module "foo", and // put it into local binding "secret". @@ -343,7 +383,7 @@ const contextifiedObject = vm.createContext({ secret: 42 }); const bar = new vm.SourceTextModule(` import s from 'foo'; s; - `, { context: contextifiedObject }); + `, { context }); // Step 2 // @@ -372,11 +412,11 @@ const contextifiedObject = vm.createContext({ secret: 42 }); if (specifier === 'foo') { return new vm.SourceTextModule(` // The "secret" variable refers to the global variable we added to - // "contextifiedObject" when creating the context. + // "context" when creating the context. export default secret; `, { context: referencingModule.context }); - // Using `contextifiedObject` instead of `referencingModule.context` + // Using `context` instead of `referencingModule.context` // here would work as well. } throw new Error(`Unable to resolve dependency: ${specifier}`); @@ -568,8 +608,8 @@ defined in the ECMAScript specification. `TypedArray`, or `DataView` with V8's code cache data for the supplied source. The `code` must be the same as the module from which this `cachedData` was created. - * `context` {Object} The [contextified][] object as returned by the - `vm.createContext()` method, to compile and evaluate this `Module` in. + * `context` {vm.Context} A [`vm.Context`][] instance, to compile and evaluate + this `Module` in. * `lineOffset` {integer} Specifies the line number offset that is displayed in stack traces produced by this `Module`. **Default:** `0`. * `columnOffset` {integer} Specifies the column number offset that is @@ -590,13 +630,13 @@ defined in the ECMAScript specification. Creates a new `SourceTextModule` instance. Properties assigned to the `import.meta` object that are objects may -allow the module to access information outside the specified `context`. Use -`vm.runInContext()` to create objects in a specific context. +allow the module to access information outside the specified `context`. ```js const vm = require('vm'); -const contextifiedObject = vm.createContext({ secret: 42 }); +const context = new vm.Context(); +context.global.secret = 42; (async () => { const module = new vm.SourceTextModule( @@ -606,7 +646,7 @@ const contextifiedObject = vm.createContext({ secret: 42 }); // Note: this object is created in the top context. As such, // Object.getPrototypeOf(import.meta.prop) points to the // Object.prototype in the top context rather than that in - // the contextified object. + // `context`. meta.prop = {}; } }); @@ -619,7 +659,7 @@ const contextifiedObject = vm.createContext({ secret: 42 }); // To fix this problem, replace // meta.prop = {}; // above with - // meta.prop = vm.runInContext('{}', contextifiedObject); + // meta.prop = vm.runInContext('{}', context); })(); ``` @@ -685,14 +725,13 @@ added: v13.0.0 * `identifier` {string} String used in stack traces. **Default:** `'vm:module(i)'` where `i` is a context-specific ascending index. - * `context` {Object} The [contextified][] object as returned by the - `vm.createContext()` method, to compile and evaluate this `Module` in. + * `context` {vm.Context} A [`vm.Context`][] instance, to compile and evaluate + this `Module` in. Creates a new `SyntheticModule` instance. Objects assigned to the exports of this instance may allow importers of -the module to access information outside the specified `context`. Use -`vm.runInContext()` to create objects in a specific context. +the module to access information outside the specified `context`. ### `syntheticModule.setExport(name, value)` * `code` {string} The JavaScript code to compile and run. -* `contextifiedObject` {Object} The [contextified][] object that will be used - as the `global` when the `code` is compiled and run. +* `context` {vm.Context} The [`vm.Context`][] that will be used when the `code` + is compiled and run. * `options` {Object|string} * `filename` {string} Specifies the filename used in stack traces produced by this script. **Default:** `'evalmachine.'`. @@ -887,9 +929,8 @@ changes: * Returns: {any} the result of the very last statement executed in the script. The `vm.runInContext()` method compiles `code`, runs it within the context of -the `contextifiedObject`, then returns the result. Running code does not have -access to the local scope. The `contextifiedObject` object *must* have been -previously [contextified][] using the [`vm.createContext()`][] method. +the `context`, then returns the result. Running code does not have access to +the local scope. If `options` is a string, then it specifies the filename. @@ -897,16 +938,15 @@ The following example compiles and executes different scripts using a single [contextified][] object: ```js -const util = require('util'); const vm = require('vm'); -const contextObject = { globalVar: 1 }; -vm.createContext(contextObject); +const context = new vm.Context(); +context.global.globalVar = 1; for (let i = 0; i < 10; ++i) { - vm.runInContext('globalVar *= 2;', contextObject); + vm.runInContext('globalVar *= 2;', context); } -console.log(contextObject); +console.log(context.global); // Prints: { globalVar: 1024 } ``` @@ -982,15 +1022,18 @@ changes: issues with namespaces that contain `then` function exports. * Returns: {any} the result of the very last statement executed in the script. +**Warning!** This API exhibits quite a few performance and correctness issues. +If possible, use the [`vm.Context`][] API instead. + The `vm.runInNewContext()` first contextifies the given `contextObject` (or creates a new `contextObject` if passed as `undefined`), compiles the `code`, -runs it within the created context, then returns the result. Running code -does not have access to the local scope. +runs it within the context of the created context, then returns the result. +Running code does not have access to the local scope. If `options` is a string, then it specifies the filename. The following example compiles and executes code that increments a global -variable and sets a new one. These globals are contained in the `contextObject`. +variable and sets a new one. These globals are contained in the context. ```js const util = require('util'); @@ -1129,38 +1172,37 @@ According to the [V8 Embedder's Guide][]: > JavaScript applications to run in a single instance of V8. You must explicitly > specify the context in which you want any JavaScript code to be run. -When the method `vm.createContext()` is called, the `contextObject` argument -(or a newly-created object if `contextObject` is `undefined`) is associated -internally with a new instance of a V8 Context. This V8 Context provides the -`code` run using the `vm` module's methods with an isolated global environment -within which it can operate. The process of creating the V8 Context and -associating it with the `contextObject` is what this document refers to as -"contextifying" the object. +When the method `vm.createContext()` is called, the `contextObject` object that +is passed in (or a newly created object if `contextObject` is `undefined`) is +associated internally with a new instance of a V8 Context. This V8 Context +provides the `code` run using the `vm` module's methods with an isolated global +environment within which it can operate. The process of creating the V8 Context +and associating it with the `contextObject` object is what this document refers +to as "contextifying" the `contextObject`. ## Timeout limitations when using `process.nextTick()`, Promises, and `queueMicrotask()` Because of the internal mechanics of how the `process.nextTick()` queue and the microtask queue that underlies Promises are implemented within V8 and Node.js, it is possible for code running within a context to "escape" the -`timeout` set using `vm.runInContext()`, `vm.runInNewContext()`, and -`vm.runInThisContext()`. +`timeout` set using `vm.runInContext()` and `vm.runInThisContext()`. -For example, the following code executed by `vm.runInNewContext()` with a -timeout of 5 milliseconds schedules an infinite loop to run after a promise -resolves. The scheduled loop is never interrupted by the timeout: +For example, the following code executed by `vm.runInContext()` with a timeout +of 5 milliseconds schedules an infinite loop to run after a promise resolves. +The scheduled loop is never interrupted by the timeout: ```js const vm = require('vm'); -function loop() { - while (1) console.log(Date.now()); -} +const context = new vm.Context(); + +context.global.loop = () => { + while (1) { + console.log(Date.now()); + } +}; -vm.runInNewContext( - 'Promise.resolve().then(loop);', - { loop, console }, - { timeout: 5 } -); +vm.runInContext('Promise.resolve().then(loop)', context, { timeout: 5 }); ``` This issue also occurs when the `loop()` call is scheduled using @@ -1174,11 +1216,12 @@ queues. [`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 -[`script.runInContext()`]: #vm_script_runincontext_contextifiedobject_options +[`script.runInContext()`]: #vm_script_runincontext_context_options [`script.runInThisContext()`]: #vm_script_runinthiscontext_options [`url.origin`]: url.html#url_url_origin +[`vm.Context`]: #vm_class_vm_context [`vm.createContext()`]: #vm_vm_createcontext_contextobject_options -[`vm.runInContext()`]: #vm_vm_runincontext_code_contextifiedobject_options +[`vm.runInContext()`]: #vm_vm_runincontext_code_context_options [`vm.runInThisContext()`]: #vm_vm_runinthiscontext_code_options [Cyclic Module Record]: https://tc39.es/ecma262/#sec-cyclic-module-records [ECMAScript Module Loader]: esm.html#esm_ecmascript_modules diff --git a/doc/api/worker_threads.md b/doc/api/worker_threads.md index c56411318da0d9..31e54796d73e89 100644 --- a/doc/api/worker_threads.md +++ b/doc/api/worker_threads.md @@ -76,14 +76,13 @@ if (isMainThread) { } ``` -## `worker.moveMessagePortToContext(port, contextifiedSandbox)` +## `worker.moveMessagePortToContext(port, context)` * `port` {MessagePort} The message port which will be transferred. -* `contextifiedSandbox` {Object} A [contextified][] object as returned by the - `vm.createContext()` method. +* `context` {vm.Context} A [`vm.Context`][] instance. * Returns: {MessagePort} @@ -768,6 +767,7 @@ active handle in the event system. If the worker is already `unref()`ed calling [`trace_events`]: tracing.html [`v8.getHeapSnapshot()`]: v8.html#v8_v8_getheapsnapshot [`vm`]: vm.html +[`vm.Context`]: vm.html#vm_class_vm_context [`worker.on('message')`]: #worker_threads_event_message_1 [`worker.postMessage()`]: #worker_threads_worker_postmessage_value_transferlist [`worker.SHARE_ENV`]: #worker_threads_worker_share_env @@ -780,5 +780,4 @@ active handle in the event system. If the worker is already `unref()`ed calling [Web Workers]: https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API [browser `MessagePort`]: https://developer.mozilla.org/en-US/docs/Web/API/MessagePort [child processes]: child_process.html -[contextified]: vm.html#vm_what_does_it_mean_to_contextify_an_object [v8.serdes]: v8.html#v8_serialization_api diff --git a/lib/vm.js b/lib/vm.js index 1d0715af528c73..ea4da08a7229ee 100644 --- a/lib/vm.js +++ b/lib/vm.js @@ -47,7 +47,10 @@ const { validateBuffer, validateObject, } = require('internal/validators'); -const { kVmBreakFirstLineSymbol } = require('internal/util'); +const { + kVmBreakFirstLineSymbol, + emitExperimentalWarning, +} = require('internal/util'); const kParsingContext = Symbol('script parsing context'); class Script extends ContextifyScript { @@ -206,13 +209,11 @@ function isContext(object) { } let defaultContextNameIndex = 1; -function createContext(contextObject = {}, options = {}) { - if (isContext(contextObject)) { - return contextObject; +function validateContextOptions(options) { + if (typeof options !== 'object' || options === null) { + throw new ERR_INVALID_ARG_TYPE('options', 'Object', options); } - validateObject(options, 'options'); - const { name = `VM Context ${defaultContextNameIndex++}`, origin, @@ -233,10 +234,35 @@ function createContext(contextObject = {}, options = {}) { validateBoolean(wasm, 'options.codeGeneration.wasm'); } - makeContext(contextObject, name, origin, strings, wasm); + return { name, origin, strings, wasm }; +} + +function createContext(contextObject = {}, options = {}) { + if (isContext(contextObject)) { + return contextObject; + } + + const { name, origin, strings, wasm } = validateContextOptions(options); + + makeContext(contextObject, name, origin, strings, wasm, undefined); return contextObject; } +class Context { + #global; + + constructor(options = {}) { + emitExperimentalWarning('vm.Context'); + + const { name, origin, strings, wasm } = validateContextOptions(options); + this.#global = makeContext(undefined, name, origin, strings, wasm, this); + } + + get global() { + return this.#global; + } +} + function createScript(code, options) { return new Script(code, options); } @@ -357,6 +383,7 @@ function compileFunction(code, params, options = {}) { module.exports = { + Context, Script, createContext, createScript, diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 46a1d7c8ef0691..b334618a70c7eb 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -151,40 +151,44 @@ MaybeLocal ContextifyContext::CreateV8Context( Local sandbox_obj, const ContextOptions& options) { EscapableHandleScope scope(env->isolate()); - Local function_template = - FunctionTemplate::New(env->isolate()); - - function_template->SetClassName(sandbox_obj->GetConstructorName()); - - Local object_template = - function_template->InstanceTemplate(); - Local data_wrapper; - if (!CreateDataWrapper(env).ToLocal(&data_wrapper)) - return MaybeLocal(); - - NamedPropertyHandlerConfiguration config( - PropertyGetterCallback, - PropertySetterCallback, - PropertyDescriptorCallback, - PropertyDeleterCallback, - PropertyEnumeratorCallback, - PropertyDefinerCallback, - data_wrapper, - PropertyHandlerFlags::kHasNoSideEffect); - - IndexedPropertyHandlerConfiguration indexed_config( - IndexedPropertyGetterCallback, - IndexedPropertySetterCallback, - IndexedPropertyDescriptorCallback, - IndexedPropertyDeleterCallback, - PropertyEnumeratorCallback, - IndexedPropertyDefinerCallback, - data_wrapper, - PropertyHandlerFlags::kHasNoSideEffect); - - object_template->SetHandler(config); - object_template->SetHandler(indexed_config); + Local object_template; + + if (!sandbox_obj.IsEmpty()) { + Local function_template = + FunctionTemplate::New(env->isolate()); + + function_template->SetClassName(sandbox_obj->GetConstructorName()); + + object_template = function_template->InstanceTemplate(); + + Local data_wrapper; + if (!CreateDataWrapper(env).ToLocal(&data_wrapper)) + return MaybeLocal(); + + NamedPropertyHandlerConfiguration config( + PropertyGetterCallback, + PropertySetterCallback, + PropertyDescriptorCallback, + PropertyDeleterCallback, + PropertyEnumeratorCallback, + PropertyDefinerCallback, + data_wrapper, + PropertyHandlerFlags::kHasNoSideEffect); + + IndexedPropertyHandlerConfiguration indexed_config( + IndexedPropertyGetterCallback, + IndexedPropertySetterCallback, + IndexedPropertyDescriptorCallback, + IndexedPropertyDeleterCallback, + PropertyEnumeratorCallback, + IndexedPropertyDefinerCallback, + data_wrapper, + PropertyHandlerFlags::kHasNoSideEffect); + + object_template->SetHandler(config); + object_template->SetHandler(indexed_config); + } Local ctx = NewContext(env->isolate(), object_template); @@ -194,16 +198,18 @@ MaybeLocal ContextifyContext::CreateV8Context( ctx->SetSecurityToken(env->context()->GetSecurityToken()); - // We need to tie the lifetime of the sandbox object with the lifetime of - // newly created context. We do this by making them hold references to each - // other. The context can directly hold a reference to the sandbox as an - // embedder data field. However, we cannot hold a reference to a v8::Context - // directly in an Object, we instead hold onto the new context's global - // object instead (which then has a reference to the context). - ctx->SetEmbedderData(ContextEmbedderIndex::kSandboxObject, sandbox_obj); - sandbox_obj->SetPrivate(env->context(), - env->contextify_global_private_symbol(), - ctx->Global()); + if (!sandbox_obj.IsEmpty()) { + // We need to tie the lifetime of the sandbox object with the lifetime of + // newly created context. We do this by making them hold references to each + // other. The context can directly hold a reference to the sandbox as an + // embedder data field. However, we cannot hold a reference to a v8::Context + // directly in an Object, we instead hold onto the new context's global + // object instead (which then has a reference to the context). + ctx->SetEmbedderData(ContextEmbedderIndex::kSandboxObject, sandbox_obj); + sandbox_obj->SetPrivate(env->context(), + env->contextify_global_private_symbol(), + ctx->Global()); + } Utf8Value name_val(env->isolate(), options.name); ctx->AllowCodeGenerationFromStrings(options.allow_code_gen_strings->IsTrue()); @@ -236,19 +242,23 @@ void ContextifyContext::Init(Environment* env, Local target) { } -// makeContext(sandbox, name, origin, strings, wasm); +// makeContext(sandbox, name, origin, strings, wasm, instance); void ContextifyContext::MakeContext(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - CHECK_EQ(args.Length(), 5); - CHECK(args[0]->IsObject()); - Local sandbox = args[0].As(); + CHECK_EQ(args.Length(), 6); - // Don't allow contextifying a sandbox multiple times. - CHECK( - !sandbox->HasPrivate( - env->context(), - env->contextify_context_private_symbol()).FromJust()); + Local sandbox; + if (args[5]->IsUndefined()) { + CHECK(args[0]->IsObject()); + sandbox = args[0].As(); + + // Don't allow contextifying a sandbox multiple times. + CHECK( + !sandbox->HasPrivate( + env->context(), + env->contextify_context_private_symbol()).FromJust()); + } ContextOptions options; @@ -278,7 +288,15 @@ void ContextifyContext::MakeContext(const FunctionCallbackInfo& args) { if (context_ptr->context().IsEmpty()) return; - sandbox->SetPrivate( + Local instance; + if (args[5]->IsUndefined()) { + instance = sandbox; + } else { + CHECK(args[5]->IsObject()); + args.GetReturnValue().Set(context_ptr->global_proxy()); + instance = args[5].As(); + } + instance->SetPrivate( env->context(), env->contextify_context_private_symbol(), External::New(env->isolate(), context_ptr.release())); diff --git a/test/parallel/test-new-vm-context.js b/test/parallel/test-new-vm-context.js new file mode 100644 index 00000000000000..fbdf72d7821cf0 --- /dev/null +++ b/test/parallel/test-new-vm-context.js @@ -0,0 +1,42 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const { Context, Script } = require('vm'); + +[ + true, 0, null, '', 'hi', + Symbol('symbol'), + { name: 0 }, + { origin: 0 }, + { codeGeneration: 0 }, + { codeGeneration: { strings: 0 } }, + { codeGeneration: { wasm: 0 } }, +].forEach((v) => { + assert.throws(() => { + new Context(v); + }, { + code: 'ERR_INVALID_ARG_TYPE', + }); +}); + +{ + const ctx = new Context(); + ctx.global.a = 1; + const script = new Script('this'); + assert.strictEqual(script.runInContext(ctx), ctx.global); +} + +// https://github.com/nodejs/node/issues/31808 +{ + const ctx = new Context(); + Object.defineProperty(ctx.global, 'x', { + enumerable: true, + configurable: true, + get: common.mustNotCall(), + set: common.mustNotCall(), + }); + const script = new Script('function x() {}'); + script.runInContext(ctx); + assert.strictEqual(typeof ctx.global.x, 'function'); +} diff --git a/test/parallel/test-vm-basic.js b/test/parallel/test-vm-basic.js index c6dadce9ad0263..8ba313ee0922f7 100644 --- a/test/parallel/test-vm-basic.js +++ b/test/parallel/test-vm-basic.js @@ -84,7 +84,7 @@ const vm = require('vm'); { const script = 'throw new Error("boom")'; const filename = 'test-boom-error'; - const context = vm.createContext(); + const context = new vm.Context(); function checkErr(err) { return err.stack.startsWith('test-boom-error:1'); diff --git a/test/parallel/test-vm-codegen.js b/test/parallel/test-vm-codegen.js index 90b37c741a1c68..1564bf210efc2c 100644 --- a/test/parallel/test-vm-codegen.js +++ b/test/parallel/test-vm-codegen.js @@ -3,13 +3,14 @@ require('../common'); const assert = require('assert'); -const { createContext, runInContext, runInNewContext } = require('vm'); +const { Context, runInContext, runInNewContext } = require('vm'); const WASM_BYTES = Buffer.from( [0x00, 0x61, 0x73, 0x6d, 0x01, 0x00, 0x00, 0x00]); { - const ctx = createContext({ WASM_BYTES }); + const ctx = new Context(); + ctx.global.WASM_BYTES = WASM_BYTES; const test = 'eval(""); new WebAssembly.Module(WASM_BYTES);'; runInContext(test, ctx); @@ -19,7 +20,7 @@ const WASM_BYTES = Buffer.from( } { - const ctx = createContext({}, { + const ctx = new Context({ codeGeneration: { strings: false, }, @@ -32,11 +33,12 @@ const WASM_BYTES = Buffer.from( } { - const ctx = createContext({ WASM_BYTES }, { + const ctx = new Context({ codeGeneration: { wasm: false, }, }); + ctx.global.WASM_BYTES = WASM_BYTES; const CompileError = runInContext('WebAssembly.CompileError', ctx); assert.throws(() => { @@ -65,7 +67,7 @@ assert.throws(() => { }); assert.throws(() => { - createContext({}, { + new Context({ codeGeneration: { strings: 0, }, @@ -85,7 +87,7 @@ assert.throws(() => { }); assert.throws(() => { - createContext({}, { + new Context({ codeGeneration: 1, }); }, { @@ -93,7 +95,7 @@ assert.throws(() => { }); assert.throws(() => { - createContext({}, { + new Context({ codeGeneration: null, }); }, { diff --git a/test/parallel/test-vm-create-and-run-in-context.js b/test/parallel/test-vm-create-and-run-in-context.js index bd746cf2df7080..6fcae23e263480 100644 --- a/test/parallel/test-vm-create-and-run-in-context.js +++ b/test/parallel/test-vm-create-and-run-in-context.js @@ -27,19 +27,21 @@ const assert = require('assert'); const vm = require('vm'); // Run in a new empty context -let context = vm.createContext(); +let context = new vm.Context(); let result = vm.runInContext('"passed";', context); assert.strictEqual(result, 'passed'); // Create a new pre-populated context -context = vm.createContext({ 'foo': 'bar', 'thing': 'lala' }); -assert.strictEqual(context.foo, 'bar'); -assert.strictEqual(context.thing, 'lala'); +context = new vm.Context(); +context.global.foo = 'bar'; +context.global.thing = 'lala'; +assert.strictEqual(context.global.foo, 'bar'); +assert.strictEqual(context.global.thing, 'lala'); // Test updating context result = vm.runInContext('var foo = 3;', context); -assert.strictEqual(context.foo, 3); -assert.strictEqual(context.thing, 'lala'); +assert.strictEqual(context.global.foo, 3); +assert.strictEqual(context.global.thing, 'lala'); // https://github.com/nodejs/node/issues/5768 // Run in contextified sandbox without referencing the context diff --git a/test/parallel/test-vm-function-redefinition.js b/test/parallel/test-vm-function-redefinition.js index fa1ddde389244e..76c65c309114dc 100644 --- a/test/parallel/test-vm-function-redefinition.js +++ b/test/parallel/test-vm-function-redefinition.js @@ -3,7 +3,7 @@ require('../common'); const assert = require('assert'); const vm = require('vm'); -const context = vm.createContext(); +const context = new vm.Context(); vm.runInContext('function test() { return 0; }', context); vm.runInContext('function test() { return 1; }', context); diff --git a/test/parallel/test-vm-module-basic.js b/test/parallel/test-vm-module-basic.js index 86e13f8b12bfa8..c67bf37b42f48a 100644 --- a/test/parallel/test-vm-module-basic.js +++ b/test/parallel/test-vm-module-basic.js @@ -8,16 +8,15 @@ const { Module, SourceTextModule, SyntheticModule, - createContext + Context, } = require('vm'); const util = require('util'); (async function test1() { - const context = createContext({ - foo: 'bar', - baz: undefined, - typeofProcess: undefined, - }); + const context = new Context(); + context.global.foo = 'bar'; + context.global.baz = undefined; + context.global.typeofProcess = undefined; const m = new SourceTextModule( 'baz = foo; typeofProcess = typeof process; typeof Object;', { context } @@ -28,12 +27,10 @@ const util = require('util'); const result = await m.evaluate(); assert.strictEqual(m.status, 'evaluated'); assert.strictEqual(Object.getPrototypeOf(result), null); - assert.deepStrictEqual(context, { - foo: 'bar', - baz: 'bar', - typeofProcess: 'undefined' - }); assert.strictEqual(result.result, 'function'); + assert.strictEqual(context.global.foo, 'bar'); + assert.strictEqual(context.global.baz, 'bar'); + assert.strictEqual(context.global.typeofProcess, 'undefined'); }()); (async () => { @@ -56,8 +53,8 @@ const util = require('util'); // Check the generated identifier for each module (async () => { - const context1 = createContext({ }); - const context2 = createContext({ }); + const context1 = new Context(); + const context2 = new Context(); const m1 = new SourceTextModule('1', { context: context1 }); assert.strictEqual(m1.identifier, 'vm:module(0)'); @@ -69,7 +66,8 @@ const util = require('util'); // Check inspection of the instance { - const context = createContext({ foo: 'bar' }); + const context = new Context(); + context.global.foo = 'bar'; const m = new SourceTextModule('1', { context }); assert.strictEqual( @@ -77,7 +75,7 @@ const util = require('util'); `SourceTextModule { status: 'unlinked', identifier: 'vm:module(0)', - context: { foo: 'bar' } + context: Context {} }` ); @@ -90,7 +88,8 @@ const util = require('util'); } { - const context = createContext({ foo: 'bar' }); + const context = new Context(); + context.global.foo = 'bar'; const m = new SyntheticModule([], () => {}, { context }); assert.strictEqual( @@ -98,7 +97,7 @@ const util = require('util'); `SyntheticModule { status: 'unlinked', identifier: 'vm:module(0)', - context: { foo: 'bar' } + context: Context {} }` ); diff --git a/test/parallel/test-vm-module-dynamic-import.js b/test/parallel/test-vm-module-dynamic-import.js index 70229b3897874b..a38fdd627698d1 100644 --- a/test/parallel/test-vm-module-dynamic-import.js +++ b/test/parallel/test-vm-module-dynamic-import.js @@ -5,10 +5,10 @@ const common = require('../common'); const assert = require('assert'); -const { Script, SourceTextModule, createContext } = require('vm'); +const { Script, SourceTextModule, Context } = require('vm'); async function testNoCallback() { - const m = new SourceTextModule('import("foo")', { context: createContext() }); + const m = new SourceTextModule('import("foo")', { context: new Context() }); await m.link(common.mustNotCall()); const { result } = await m.evaluate(); let threw = false; diff --git a/test/parallel/test-vm-module-errors.js b/test/parallel/test-vm-module-errors.js index 5f36f9339fe41a..f2dbfda6465d3f 100644 --- a/test/parallel/test-vm-module-errors.js +++ b/test/parallel/test-vm-module-errors.js @@ -6,7 +6,7 @@ const common = require('../common'); const assert = require('assert'); -const { SourceTextModule, createContext } = require('vm'); +const { SourceTextModule, Context } = require('vm'); async function createEmptyLinkedModule() { const m = new SourceTextModule(''); @@ -125,7 +125,8 @@ async function checkLinking() { }); await assert.rejects(async () => { - const c = createContext({ a: 1 }); + const c = new Context(); + c.global.a = 1; const foo = new SourceTextModule('', { context: c }); await foo.link(common.mustNotCall()); const bar = new SourceTextModule('import "foo";'); diff --git a/test/parallel/test-vm-strict-assign.js b/test/parallel/test-vm-strict-assign.js index a61db82c639a23..fd3b22d7456578 100644 --- a/test/parallel/test-vm-strict-assign.js +++ b/test/parallel/test-vm-strict-assign.js @@ -5,11 +5,11 @@ const assert = require('assert'); const vm = require('vm'); // https://github.com/nodejs/node/issues/10223 -const ctx = vm.createContext(); +const ctx = new vm.Context(); // Define x with writable = false. vm.runInContext('Object.defineProperty(this, "x", { value: 42 })', ctx); -assert.strictEqual(ctx.x, 42); +assert.strictEqual(ctx.global.x, 42); assert.strictEqual(vm.runInContext('x', ctx), 42); vm.runInContext('x = 0', ctx); // Does not throw but x... diff --git a/test/parallel/test-vm-strict-mode.js b/test/parallel/test-vm-strict-mode.js index b1b233664dab9b..e249539cd0c176 100644 --- a/test/parallel/test-vm-strict-mode.js +++ b/test/parallel/test-vm-strict-mode.js @@ -5,10 +5,11 @@ require('../common'); const assert = require('assert'); const vm = require('vm'); -const ctx = vm.createContext({ x: 42 }); +const ctx = new vm.Context(); +ctx.global.x = 42; // This might look as if x has not been declared, but x is defined on the // sandbox and the assignment should not throw. vm.runInContext('"use strict"; x = 1', ctx); -assert.strictEqual(ctx.x, 1); +assert.strictEqual(ctx.global.x, 1); diff --git a/test/parallel/test-worker-message-port-move.js b/test/parallel/test-worker-message-port-move.js index 7e5d0243fa8b96..2b1f1ba9eb41bf 100644 --- a/test/parallel/test-worker-message-port-move.js +++ b/test/parallel/test-worker-message-port-move.js @@ -7,12 +7,11 @@ const { MessagePort, MessageChannel, moveMessagePortToContext } = require('worker_threads'); -const context = vm.createContext(); +const context = new vm.Context(); const { port1, port2 } = new MessageChannel(); -context.port = moveMessagePortToContext(port1, context); -context.global = context; -Object.assign(context, { - global: context, +Object.assign(context.global, { + global: context.global, + port: moveMessagePortToContext(port1, context), assert, MessagePort, MessageChannel diff --git a/tools/doc/type-parser.js b/tools/doc/type-parser.js index 02b59d37ffd278..1e6b782e60ba19 100644 --- a/tools/doc/type-parser.js +++ b/tools/doc/type-parser.js @@ -147,6 +147,7 @@ const customTypesMap = { 'URL': 'url.html#url_the_whatwg_url_api', 'URLSearchParams': 'url.html#url_class_urlsearchparams', + 'vm.Context': 'vm.html#vm_class_vm_context', 'vm.Module': 'vm.html#vm_class_vm_module', 'vm.SourceTextModule': 'vm.html#vm_class_vm_sourcetextmodule', From 45c5232d9baf133f692fc300437eebac680f12a0 Mon Sep 17 00:00:00 2001 From: Gus Caplan Date: Fri, 10 Jan 2020 23:51:35 -0800 Subject: [PATCH 2/2] tools: apilinks.js skip file if unable to be parsed --- test/fixtures/apilinks/private_fields.js | 9 +++++++++ test/fixtures/apilinks/private_fields.json | 1 + tools/doc/apilinks.js | 13 +++++++++---- 3 files changed, 19 insertions(+), 4 deletions(-) create mode 100644 test/fixtures/apilinks/private_fields.js create mode 100644 test/fixtures/apilinks/private_fields.json diff --git a/test/fixtures/apilinks/private_fields.js b/test/fixtures/apilinks/private_fields.js new file mode 100644 index 00000000000000..3bd17f384b0de3 --- /dev/null +++ b/test/fixtures/apilinks/private_fields.js @@ -0,0 +1,9 @@ +'use strict'; + +// Acorn does not support private fields. +// Verify that apilinks creates empty output +// instead of crashing. + +class Foo { + #a = 1; +} diff --git a/test/fixtures/apilinks/private_fields.json b/test/fixtures/apilinks/private_fields.json new file mode 100644 index 00000000000000..0967ef424bce67 --- /dev/null +++ b/test/fixtures/apilinks/private_fields.json @@ -0,0 +1 @@ +{} diff --git a/tools/doc/apilinks.js b/tools/doc/apilinks.js index 461805dac3a811..e33e96de3a429c 100644 --- a/tools/doc/apilinks.js +++ b/tools/doc/apilinks.js @@ -54,10 +54,15 @@ inputs.forEach((file) => { // Parse source. const source = fs.readFileSync(file, 'utf8'); - const ast = acorn.parse( - source, - { allowReturnOutsideFunction: true, ecmaVersion: 10, locations: true }); - const program = ast.body; + let program; + try { + const ast = acorn.parse( + source, + { allowReturnOutsideFunction: true, ecmaVersion: 10, locations: true }); + program = ast.body; + } catch { + return; + } // Build link const link = `https://github.com/${repo}/blob/${tag}/` +