Skip to content

Commit

Permalink
module: move options checks from C++ to JS
Browse files Browse the repository at this point in the history 
PR-URL: #19822
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
@targos
targos committed Apr 7, 2018
1 parent 0ac6ced commit 77b52fd
Show file tree
Hide file tree
Showing 7 changed files with 114 additions and 240 deletions.
2 changes: 1 addition & 1 deletion lib/internal/modules/esm/create_dynamic_module.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/modules/esm/module_job.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
80 changes: 54 additions & 26 deletions lib/internal/vm/module.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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 &&
Expand All @@ -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 };
}

Expand All @@ -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,
Expand Down
130 changes: 57 additions & 73 deletions src/module_wrap.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -77,54 +78,58 @@ ModuleWrap* ModuleWrap::GetFromModule(Environment* env,

void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Isolate* isolate = env->isolate();

Isolate* isolate = args.GetIsolate();
CHECK(args.IsConstructCall());
Local<Object> 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<String> source_text = args[0].As<String>();

if (!args[1]->IsString()) {
env->ThrowError("second argument is not a string");
return;
}

CHECK(args[1]->IsString());
Local<String> url = args[1].As<String>();

Local<Object> that = args.This();
Local<Context> context;
Local<Integer> line_offset;
Local<Integer> column_offset;

Environment::ShouldNotAbortOnUncaughtScope no_abort_scope(env);
TryCatch try_catch(isolate);

Local<Value> options = args[2];
MaybeLocal<Integer> line_offset = contextify::GetLineOffsetArg(env, options);
MaybeLocal<Integer> column_offset =
contextify::GetColumnOffsetArg(env, options);
MaybeLocal<Context> 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<Object>());
CHECK_NE(sandbox, nullptr);
context = sandbox->context();
}

CHECK(args[3]->IsNumber());
line_offset = args[3].As<Integer>();

if (try_catch.HasCaught()) {
no_abort_scope.Close();
try_catch.ReThrow();
return;
CHECK(args[4]->IsNumber());
column_offset = args[4].As<Integer>();
} else {
// new ModuleWrap(source, url)
context = that->CreationContext();
line_offset = Integer::New(isolate, 0);
column_offset = Integer::New(isolate, 0);
}

Local<Context> context = maybe_context.FromMaybe(that->CreationContext());
Environment::ShouldNotAbortOnUncaughtScope no_abort_scope(env);
TryCatch try_catch(isolate);
Local<Module> 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<Integer>(), // script id
Local<Value>(), // source map URL
Expand Down Expand Up @@ -161,10 +166,9 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
void ModuleWrap::Link(const FunctionCallbackInfo<Value>& 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<Object> that = args.This();

Expand Down Expand Up @@ -239,27 +243,23 @@ void ModuleWrap::Instantiate(const FunctionCallbackInfo<Value>& args) {

void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& 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> context = obj->context_.Get(isolate);
Local<Module> module = obj->module_.Get(isolate);

Environment::ShouldNotAbortOnUncaughtScope no_abort_scope(env);
TryCatch try_catch(isolate);
Maybe<int64_t> maybe_timeout =
contextify::GetTimeoutArg(env, args[0]);
Maybe<bool> 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;
Expand Down Expand Up @@ -665,26 +665,14 @@ Maybe<URL> Resolve(Environment* env,
void ModuleWrap::Resolve(const FunctionCallbackInfo<Value>& 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());

Expand Down Expand Up @@ -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<Function> import_callback = args[0].As<Function>();
env->set_host_import_module_dynamically_callback(import_callback);

Expand Down Expand Up @@ -781,11 +767,9 @@ void ModuleWrap::SetInitializeImportMetaObjectCallback(
const FunctionCallbackInfo<Value>& 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<Function> import_meta_callback = args[0].As<Function>();
env->set_host_initialize_import_meta_object_callback(import_meta_callback);

Expand Down
Loading

0 comments on commit 77b52fd

Please sign in to comment.