diff --git a/lib/internal/modules/esm/create_dynamic_module.js b/lib/internal/modules/esm/create_dynamic_module.js index 7e9777af51ee2b..e0c97263631432 100644 --- a/lib/internal/modules/esm/create_dynamic_module.js +++ b/lib/internal/modules/esm/create_dynamic_module.js @@ -32,7 +32,7 @@ const createDynamicModule = (exports, url = '', evaluate) => { }));`; const reflectiveModule = new ModuleWrap(src, `cjs-facade:${url}`); reflectiveModule.instantiate(); - const { setExecutor, reflect } = reflectiveModule.evaluate()(); + const { setExecutor, reflect } = reflectiveModule.evaluate(-1, false)(); // public exposed ESM const reexports = ` import { diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index d948252829ddbf..8dbeef78e63a1d 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -103,7 +103,7 @@ class ModuleJob { async run() { const module = await this.instantiate(); try { - module.evaluate(); + module.evaluate(-1, false); } catch (e) { e.stack; this.hadError = true; diff --git a/lib/internal/vm/module.js b/lib/internal/vm/module.js index a64ccfe3b2d553..7284c8bd619901 100644 --- a/lib/internal/vm/module.js +++ b/lib/internal/vm/module.js @@ -3,9 +3,10 @@ const { internalBinding } = require('internal/bootstrap/loaders'); const { emitExperimentalWarning } = require('internal/util'); const { URL } = require('internal/url'); -const { kParsingContext, isContext } = process.binding('contextify'); +const { isContext } = process.binding('contextify'); const { ERR_INVALID_ARG_TYPE, + ERR_OUT_OF_RANGE, ERR_VM_MODULE_ALREADY_LINKED, ERR_VM_MODULE_DIFFERENT_CONTEXT, ERR_VM_MODULE_LINKING_ERRORED, @@ -55,23 +56,26 @@ class Module { if (typeof src !== 'string') throw new ERR_INVALID_ARG_TYPE('src', 'string', src); if (typeof options !== 'object' || options === null) - throw new ERR_INVALID_ARG_TYPE('options', 'object', options); + throw new ERR_INVALID_ARG_TYPE('options', 'Object', options); - let context; - if (options.context !== undefined) { - if (typeof options.context !== 'object' || options.context === null) { - throw new ERR_INVALID_ARG_TYPE('options.context', 'object', - options.context); + const { + context, + lineOffset = 0, + columnOffset = 0, + initializeImportMeta + } = options; + + if (context !== undefined) { + if (typeof context !== 'object' || context === null) { + throw new ERR_INVALID_ARG_TYPE('options.context', 'Object', context); } - if (isContext(options.context)) { - context = options.context; - } else { - throw new ERR_INVALID_ARG_TYPE('options.context', - 'vm.Context', options.context); + if (!isContext(context)) { + throw new ERR_INVALID_ARG_TYPE('options.context', 'vm.Context', + context); } } - let url = options.url; + let { url } = options; if (url !== undefined) { if (typeof url !== 'string') { throw new ERR_INVALID_ARG_TYPE('options.url', 'string', url); @@ -88,22 +92,19 @@ class Module { perContextModuleId.set(context, 1); } - if (options.initializeImportMeta !== undefined) { - if (typeof options.initializeImportMeta === 'function') { - initImportMetaMap.set(this, options.initializeImportMeta); + validateInteger(lineOffset, 'options.lineOffset'); + validateInteger(columnOffset, 'options.columnOffset'); + + if (initializeImportMeta !== undefined) { + if (typeof initializeImportMeta === 'function') { + initImportMetaMap.set(this, initializeImportMeta); } else { throw new ERR_INVALID_ARG_TYPE( - 'options.initializeImportMeta', 'function', - options.initializeImportMeta); + 'options.initializeImportMeta', 'function', initializeImportMeta); } } - const wrap = new ModuleWrap(src, url, { - [kParsingContext]: context, - lineOffset: options.lineOffset, - columnOffset: options.columnOffset - }); - + const wrap = new ModuleWrap(src, url, context, lineOffset, columnOffset); wrapMap.set(this, wrap); linkingStatusMap.set(this, 'unlinked'); wrapToModuleMap.set(wrap, this); @@ -194,7 +195,25 @@ class Module { wrap.instantiate(); } - async evaluate(options) { + async evaluate(options = {}) { + if (typeof options !== 'object' || options === null) { + throw new ERR_INVALID_ARG_TYPE('options', 'Object', options); + } + + let timeout = options.timeout; + if (timeout === undefined) { + timeout = -1; + } else if (!Number.isInteger(timeout) || timeout <= 0) { + throw new ERR_INVALID_ARG_TYPE('options.timeout', 'a positive integer', + timeout); + } + + const { breakOnSigint = false } = options; + if (typeof breakOnSigint !== 'boolean') { + throw new ERR_INVALID_ARG_TYPE('options.breakOnSigint', 'boolean', + breakOnSigint); + } + const wrap = wrapMap.get(this); const status = wrap.getStatus(); if (status !== kInstantiated && @@ -204,7 +223,7 @@ class Module { 'must be one of instantiated, evaluated, and errored' ); } - const result = wrap.evaluate(options); + const result = wrap.evaluate(timeout, breakOnSigint); return { result, __proto__: null }; } @@ -224,6 +243,15 @@ class Module { } } +function validateInteger(prop, propName) { + if (!Number.isInteger(prop)) { + throw new ERR_INVALID_ARG_TYPE(propName, 'integer', prop); + } + if ((prop >> 0) !== prop) { + throw new ERR_OUT_OF_RANGE(propName, '32-bit integer', prop); + } +} + module.exports = { Module, initImportMetaMap, diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 617bae8b60e3e0..7bf7b9dd08c062 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -13,6 +13,7 @@ namespace node { namespace loader { +using node::contextify::ContextifyContext; using node::url::URL; using node::url::URL_FLAGS_FAILED; using v8::Array; @@ -77,54 +78,58 @@ ModuleWrap* ModuleWrap::GetFromModule(Environment* env, void ModuleWrap::New(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); + Isolate* isolate = env->isolate(); - Isolate* isolate = args.GetIsolate(); + CHECK(args.IsConstructCall()); + Local that = args.This(); - if (!args.IsConstructCall()) { - env->ThrowError("constructor must be called using new"); - return; - } - - if (!args[0]->IsString()) { - env->ThrowError("first argument is not a string"); - return; - } + const int argc = args.Length(); + CHECK_GE(argc, 2); + CHECK(args[0]->IsString()); Local source_text = args[0].As(); - if (!args[1]->IsString()) { - env->ThrowError("second argument is not a string"); - return; - } - + CHECK(args[1]->IsString()); Local url = args[1].As(); - Local that = args.This(); + Local context; + Local line_offset; + Local column_offset; - Environment::ShouldNotAbortOnUncaughtScope no_abort_scope(env); - TryCatch try_catch(isolate); - - Local options = args[2]; - MaybeLocal line_offset = contextify::GetLineOffsetArg(env, options); - MaybeLocal column_offset = - contextify::GetColumnOffsetArg(env, options); - MaybeLocal maybe_context = contextify::GetContextArg(env, options); + if (argc == 5) { + // new ModuleWrap(source, url, context?, lineOffset, columnOffset) + if (args[2]->IsUndefined()) { + context = that->CreationContext(); + } else { + CHECK(args[2]->IsObject()); + ContextifyContext* sandbox = + ContextifyContext::ContextFromContextifiedSandbox( + env, args[2].As()); + CHECK_NE(sandbox, nullptr); + context = sandbox->context(); + } + CHECK(args[3]->IsNumber()); + line_offset = args[3].As(); - if (try_catch.HasCaught()) { - no_abort_scope.Close(); - try_catch.ReThrow(); - return; + CHECK(args[4]->IsNumber()); + column_offset = args[4].As(); + } else { + // new ModuleWrap(source, url) + context = that->CreationContext(); + line_offset = Integer::New(isolate, 0); + column_offset = Integer::New(isolate, 0); } - Local context = maybe_context.FromMaybe(that->CreationContext()); + Environment::ShouldNotAbortOnUncaughtScope no_abort_scope(env); + TryCatch try_catch(isolate); Local module; // compile { ScriptOrigin origin(url, - line_offset.ToLocalChecked(), // line offset - column_offset.ToLocalChecked(), // column offset + line_offset, // line offset + column_offset, // column offset False(isolate), // is cross origin Local(), // script id Local(), // source map URL @@ -161,10 +166,9 @@ void ModuleWrap::New(const FunctionCallbackInfo& args) { void ModuleWrap::Link(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); Isolate* isolate = args.GetIsolate(); - if (!args[0]->IsFunction()) { - env->ThrowError("first argument is not a function"); - return; - } + + CHECK_EQ(args.Length(), 1); + CHECK(args[0]->IsFunction()); Local that = args.This(); @@ -239,27 +243,23 @@ void ModuleWrap::Instantiate(const FunctionCallbackInfo& args) { void ModuleWrap::Evaluate(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - Isolate* isolate = args.GetIsolate(); + Isolate* isolate = env->isolate(); ModuleWrap* obj; ASSIGN_OR_RETURN_UNWRAP(&obj, args.This()); Local context = obj->context_.Get(isolate); Local module = obj->module_.Get(isolate); - Environment::ShouldNotAbortOnUncaughtScope no_abort_scope(env); - TryCatch try_catch(isolate); - Maybe maybe_timeout = - contextify::GetTimeoutArg(env, args[0]); - Maybe maybe_break_on_sigint = - contextify::GetBreakOnSigintArg(env, args[0]); + // module.evaluate(timeout, breakOnSigint) + CHECK_EQ(args.Length(), 2); - if (try_catch.HasCaught()) { - no_abort_scope.Close(); - try_catch.ReThrow(); - return; - } + CHECK(args[0]->IsNumber()); + int64_t timeout = args[0]->IntegerValue(env->context()).FromJust(); - int64_t timeout = maybe_timeout.ToChecked(); - bool break_on_sigint = maybe_break_on_sigint.ToChecked(); + CHECK(args[1]->IsBoolean()); + bool break_on_sigint = args[1]->IsTrue(); + + Environment::ShouldNotAbortOnUncaughtScope no_abort_scope(env); + TryCatch try_catch(isolate); bool timed_out = false; bool received_signal = false; @@ -665,26 +665,14 @@ Maybe Resolve(Environment* env, void ModuleWrap::Resolve(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - if (args.IsConstructCall()) { - env->ThrowError("resolve() must not be called as a constructor"); - return; - } - if (args.Length() != 2) { - env->ThrowError("resolve must have exactly 2 arguments (string, string)"); - return; - } + // module.resolve(specifier, url) + CHECK_EQ(args.Length(), 2); - if (!args[0]->IsString()) { - env->ThrowError("first argument is not a string"); - return; - } + CHECK(args[0]->IsString()); Utf8Value specifier_utf8(env->isolate(), args[0]); std::string specifier_std(*specifier_utf8, specifier_utf8.length()); - if (!args[1]->IsString()) { - env->ThrowError("second argument is not a string"); - return; - } + CHECK(args[1]->IsString()); Utf8Value url_utf8(env->isolate(), args[1]); URL url(*url_utf8, url_utf8.length()); @@ -748,11 +736,9 @@ void ModuleWrap::SetImportModuleDynamicallyCallback( Isolate* iso = args.GetIsolate(); Environment* env = Environment::GetCurrent(args); HandleScope handle_scope(iso); - if (!args[0]->IsFunction()) { - env->ThrowError("first argument is not a function"); - return; - } + CHECK_EQ(args.Length(), 1); + CHECK(args[0]->IsFunction()); Local import_callback = args[0].As(); env->set_host_import_module_dynamically_callback(import_callback); @@ -781,11 +767,9 @@ void ModuleWrap::SetInitializeImportMetaObjectCallback( const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); Isolate* isolate = env->isolate(); - if (!args[0]->IsFunction()) { - env->ThrowError("first argument is not a function"); - return; - } + CHECK_EQ(args.Length(), 1); + CHECK(args[0]->IsFunction()); Local import_meta_callback = args[0].As(); env->set_host_initialize_import_meta_object_callback(import_meta_callback); diff --git a/src/node_contextify.cc b/src/node_contextify.cc index c71e9b57896836..ce235765018775 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -47,7 +47,6 @@ using v8::Maybe; using v8::MaybeLocal; using v8::Name; using v8::NamedPropertyHandlerConfiguration; -using v8::Nothing; using v8::Object; using v8::ObjectTemplate; using v8::PropertyAttribute; @@ -586,132 +585,6 @@ void ContextifyContext::IndexedPropertyDeleterCallback( args.GetReturnValue().Set(false); } -Maybe GetBreakOnSigintArg(Environment* env, - Local options) { - if (options->IsUndefined() || options->IsString()) { - return Just(false); - } - if (!options->IsObject()) { - env->ThrowTypeError("options must be an object"); - return Nothing(); - } - - Local key = FIXED_ONE_BYTE_STRING(env->isolate(), "breakOnSigint"); - MaybeLocal maybe_value = - options.As()->Get(env->context(), key); - if (maybe_value.IsEmpty()) - return Nothing(); - - Local value = maybe_value.ToLocalChecked(); - return Just(value->IsTrue()); -} - -Maybe GetTimeoutArg(Environment* env, Local options) { - if (options->IsUndefined() || options->IsString()) { - return Just(-1); - } - if (!options->IsObject()) { - env->ThrowTypeError("options must be an object"); - return Nothing(); - } - - MaybeLocal maybe_value = - options.As()->Get(env->context(), env->timeout_string()); - if (maybe_value.IsEmpty()) - return Nothing(); - - Local value = maybe_value.ToLocalChecked(); - if (value->IsUndefined()) { - return Just(-1); - } - - Maybe timeout = value->IntegerValue(env->context()); - - if (timeout.IsJust() && timeout.ToChecked() <= 0) { - env->ThrowRangeError("timeout must be a positive number"); - return Nothing(); - } - - return timeout; -} - -MaybeLocal GetLineOffsetArg(Environment* env, - Local options) { - Local defaultLineOffset = Integer::New(env->isolate(), 0); - - if (!options->IsObject()) { - return defaultLineOffset; - } - - Local key = FIXED_ONE_BYTE_STRING(env->isolate(), "lineOffset"); - MaybeLocal maybe_value = - options.As()->Get(env->context(), key); - if (maybe_value.IsEmpty()) - return MaybeLocal(); - - Local value = maybe_value.ToLocalChecked(); - if (value->IsUndefined()) - return defaultLineOffset; - - return value->ToInteger(env->context()); -} - -MaybeLocal GetColumnOffsetArg(Environment* env, - Local options) { - Local defaultColumnOffset = Integer::New(env->isolate(), 0); - - if (!options->IsObject()) { - return defaultColumnOffset; - } - - Local key = FIXED_ONE_BYTE_STRING(env->isolate(), "columnOffset"); - MaybeLocal maybe_value = - options.As()->Get(env->context(), key); - if (maybe_value.IsEmpty()) - return MaybeLocal(); - - Local value = maybe_value.ToLocalChecked(); - if (value->IsUndefined()) - return defaultColumnOffset; - - return value->ToInteger(env->context()); -} - -MaybeLocal GetContextArg(Environment* env, - Local options) { - if (!options->IsObject()) - return MaybeLocal(); - - MaybeLocal maybe_value = - options.As()->Get(env->context(), - env->vm_parsing_context_symbol()); - Local value; - if (!maybe_value.ToLocal(&value)) - return MaybeLocal(); - - if (!value->IsObject()) { - if (!value->IsNullOrUndefined()) { - env->ThrowTypeError( - "contextifiedSandbox argument must be an object."); - } - return MaybeLocal(); - } - - ContextifyContext* sandbox = - ContextifyContext::ContextFromContextifiedSandbox( - env, value.As()); - if (!sandbox) { - env->ThrowTypeError( - "sandbox argument must have been converted to a context."); - return MaybeLocal(); - } - - Local context = sandbox->context(); - if (context.IsEmpty()) - return MaybeLocal(); - return context; -} - class ContextifyScript : public BaseObject { private: Persistent script_; diff --git a/src/node_contextify.h b/src/node_contextify.h index 1075bc5c6865a9..565b8ef856ea49 100644 --- a/src/node_contextify.h +++ b/src/node_contextify.h @@ -98,17 +98,6 @@ class ContextifyContext { const v8::PropertyCallbackInfo& args); }; -v8::Maybe GetBreakOnSigintArg( - Environment* env, v8::Local options); -v8::Maybe GetTimeoutArg( - Environment* env, v8::Local options); -v8::MaybeLocal GetLineOffsetArg( - Environment* env, v8::Local options); -v8::MaybeLocal GetColumnOffsetArg( - Environment* env, v8::Local options); -v8::MaybeLocal GetContextArg( - Environment* env, v8::Local options); - } // namespace contextify } // namespace node diff --git a/test/parallel/test-internal-module-wrap.js b/test/parallel/test-internal-module-wrap.js index 66a17a0f344701..bb4a648ef7f684 100644 --- a/test/parallel/test-internal-module-wrap.js +++ b/test/parallel/test-internal-module-wrap.js @@ -24,6 +24,6 @@ const bar = new ModuleWrap('export const five = 5', 'bar'); foo.instantiate(); - assert.strictEqual(await foo.evaluate(), 6); + assert.strictEqual(await foo.evaluate(-1, false), 6); assert.strictEqual(foo.namespace().five, 5); })();