Skip to content

Commit fd36576

Browse files
domenicisaacs
domenic
authored and
isaacs
committed
vm: update API to use options argument
Passing a filename is still supported in place of certain options arguments, for backward-compatibility, but timeout and display-errors are not translated since those were undocumented. Also managed to eliminate an extra stack trace line by not calling through the `createScript` export. Added a few message tests to show how `displayErrors` works.
1 parent de7d698 commit fd36576

17 files changed

Lines changed: 331 additions & 88 deletions

‎lib/module.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,7 @@ Module.prototype._compile = function(content, filename) {
412412
sandbox.global = sandbox;
413413
sandbox.root = root;
414414

415-
return runInNewContext(content, sandbox, filename);
415+
return runInNewContext(content, sandbox, { filename: filename });
416416
}
417417

418418
debug('load root module');
@@ -423,13 +423,13 @@ Module.prototype._compile = function(content, filename) {
423423
global.__dirname = dirname;
424424
global.module = self;
425425

426-
return runInThisContext(content, filename);
426+
return runInThisContext(content, { filename: filename });
427427
}
428428

429429
// create wrapper function
430430
var wrapper = Module.wrap(content);
431431

432-
var compiledWrapper = runInThisContext(wrapper, filename);
432+
var compiledWrapper = runInThisContext(wrapper, { filename: filename });
433433
if (global.v8debug) {
434434
if (!resolvedArgv) {
435435
// we enter the repl if we're not given a filename argument.

‎lib/repl.js‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,15 @@ function REPLServer(prompt, stream, eval_, useGlobal, ignoreUndefined) {
113113
var err, result;
114114
try {
115115
if (self.useGlobal) {
116-
result = vm.runInThisContext(code, file, 0, false);
116+
result = vm.runInThisContext(code, {
117+
filename: file,
118+
displayErrors: false
119+
});
117120
} else {
118-
result = vm.runInContext(code, context, file, 0, false);
121+
result = vm.runInContext(code, context, {
122+
filename: file,
123+
displayErrors: false
124+
});
119125
}
120126
} catch (e) {
121127
err = e;

‎lib/vm.js‎

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -24,48 +24,50 @@ var Script = binding.ContextifyScript;
2424
var util = require('util');
2525

2626
// The binding provides a few useful primitives:
27-
// - ContextifyScript(code, [filename]), with methods:
28-
// - runInThisContext()
29-
// - runInContext(sandbox, [timeout])
27+
// - ContextifyScript(code, { filename = "evalmachine.anonymous",
28+
// displayErrors = true } = {})
29+
// with methods:
30+
// - runInThisContext({ displayErrors = true } = {})
31+
// - runInContext(sandbox, { displayErrors = true, timeout = undefined } = {})
3032
// - makeContext(sandbox)
3133
// - isContext(sandbox)
3234
// From this we build the entire documented API.
3335

34-
Script.prototype.runInNewContext = function(initSandbox, timeout, disp) {
35-
var context = exports.createContext(initSandbox);
36-
return this.runInContext(context, timeout, disp);
36+
Script.prototype.runInNewContext = function(sandbox, options) {
37+
var context = exports.createContext(sandbox);
38+
return this.runInContext(context, options);
3739
};
3840

3941
exports.Script = Script;
4042

41-
exports.createScript = function(code, filename, disp) {
42-
return new Script(code, filename, disp);
43+
exports.createScript = function(code, options) {
44+
return new Script(code, options);
4345
};
4446

45-
exports.createContext = function(initSandbox) {
46-
if (util.isUndefined(initSandbox)) {
47-
initSandbox = {};
48-
} else if (binding.isContext(initSandbox)) {
49-
return initSandbox;
47+
exports.createContext = function(sandbox) {
48+
if (util.isUndefined(sandbox)) {
49+
sandbox = {};
50+
} else if (binding.isContext(sandbox)) {
51+
return sandbox;
5052
}
5153

52-
binding.makeContext(initSandbox);
53-
return initSandbox;
54+
binding.makeContext(sandbox);
55+
return sandbox;
5456
};
5557

56-
exports.runInContext = function(code, sandbox, filename, timeout, disp) {
57-
var script = exports.createScript(code, filename, disp);
58-
return script.runInContext(sandbox, timeout, disp);
58+
exports.runInContext = function(code, contextifiedSandbox, options) {
59+
var script = new Script(code, options);
60+
return script.runInContext(contextifiedSandbox, options);
5961
};
6062

61-
exports.runInNewContext = function(code, sandbox, filename, timeout, disp) {
62-
var script = exports.createScript(code, filename, disp);
63-
return script.runInNewContext(sandbox, timeout, disp);
63+
exports.runInNewContext = function(code, sandbox, options) {
64+
var script = new Script(code, options);
65+
return script.runInNewContext(sandbox, options);
6466
};
6567

66-
exports.runInThisContext = function(code, filename, timeout, disp) {
67-
var script = exports.createScript(code, filename, disp);
68-
return script.runInThisContext(timeout, disp);
68+
exports.runInThisContext = function(code, options) {
69+
var script = new Script(code, options);
70+
return script.runInThisContext(options);
6971
};
7072

7173
exports.isContext = binding.isContext;

‎src/node.js‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -382,8 +382,8 @@
382382
'global.__dirname = __dirname;\n' +
383383
'global.require = require;\n' +
384384
'return require("vm").runInThisContext(' +
385-
JSON.stringify(body) + ', ' +
386-
JSON.stringify(name) + ');\n';
385+
JSON.stringify(body) + ', { filename: ' +
386+
JSON.stringify(name) + ' });\n';
387387
}
388388
var result = module._compile(script, name + '-wrapper');
389389
if (process._print_eval) console.log(result);
@@ -675,8 +675,8 @@
675675
// node binary, so they can be loaded faster.
676676

677677
var ContextifyScript = process.binding('contextify').ContextifyScript;
678-
function runInThisContext(code, filename) {
679-
var script = new ContextifyScript(code, filename);
678+
function runInThisContext(code, options) {
679+
var script = new ContextifyScript(code, options);
680680
return script.runInThisContext();
681681
}
682682

@@ -739,7 +739,7 @@
739739
var source = NativeModule.getSource(this.id);
740740
source = NativeModule.wrap(source);
741741

742-
var fn = runInThisContext(source, this.filename);
742+
var fn = runInThisContext(source, { filename: this.filename });
743743
fn(this.exports, NativeModule.require, this, this.filename);
744744

745745
this.loaded = true;

‎src/node_contextify.cc‎

Lines changed: 68 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ class ContextifyScript : ObjectWrap {
321321
}
322322

323323

324-
// args: code, [filename]
324+
// args: code, [options]
325325
static void New(const FunctionCallbackInfo<Value>& args) {
326326
HandleScope scope(node_isolate);
327327

@@ -331,20 +331,25 @@ class ContextifyScript : ObjectWrap {
331331

332332
ContextifyScript *contextify_script = new ContextifyScript();
333333
contextify_script->Wrap(args.Holder());
334+
335+
TryCatch try_catch;
334336
Local<String> code = args[0]->ToString();
335337
Local<String> filename = GetFilenameArg(args, 1);
336-
bool display_exception = GetDisplayArg(args, 2);
338+
bool display_errors = GetDisplayErrorsArg(args, 1);
339+
if (try_catch.HasCaught()) {
340+
try_catch.ReThrow();
341+
return;
342+
}
337343

338344
Local<Context> context = Context::GetCurrent();
339345
Context::Scope context_scope(context);
340346

341-
TryCatch try_catch;
342-
343347
Local<Script> v8_script = Script::New(code, filename);
344348

345349
if (v8_script.IsEmpty()) {
346-
if (display_exception)
350+
if (display_errors) {
347351
DisplayExceptionLine(try_catch.Message());
352+
}
348353
try_catch.ReThrow();
349354
return;
350355
}
@@ -358,42 +363,40 @@ class ContextifyScript : ObjectWrap {
358363
}
359364

360365

361-
// args: [timeout]
366+
// args: [options]
362367
static void RunInThisContext(const FunctionCallbackInfo<Value>& args) {
363368
HandleScope scope(node_isolate);
364369

365370
// Assemble arguments
366371
TryCatch try_catch;
367372
uint64_t timeout = GetTimeoutArg(args, 0);
373+
bool display_errors = GetDisplayErrorsArg(args, 0);
368374
if (try_catch.HasCaught()) {
369375
try_catch.ReThrow();
370376
return;
371377
}
372378

373-
bool display_exception = GetDisplayArg(args, 1);
374-
375379
// Do the eval within this context
376-
EvalMachine(timeout, display_exception, args, try_catch);
380+
EvalMachine(timeout, display_errors, args, try_catch);
377381
}
378382

379-
// args: sandbox, [timeout]
383+
// args: sandbox, [options]
380384
static void RunInContext(const FunctionCallbackInfo<Value>& args) {
381385
HandleScope scope(node_isolate);
382386

383387
// Assemble arguments
384388
TryCatch try_catch;
385389
if (!args[0]->IsObject()) {
386-
return ThrowTypeError("sandbox argument must be an object.");
390+
return ThrowTypeError("contextifiedSandbox argument must be an object.");
387391
}
388392
Local<Object> sandbox = args[0].As<Object>();
389-
uint64_t timeout = GetTimeoutArg(args, 1);
393+
int64_t timeout = GetTimeoutArg(args, 1);
394+
bool display_errors = GetDisplayErrorsArg(args, 1);
390395
if (try_catch.HasCaught()) {
391396
try_catch.ReThrow();
392397
return;
393398
}
394399

395-
bool display_exception = GetDisplayArg(args, 2);
396-
397400
// Get the context from the sandbox
398401
Local<Context> context =
399402
ContextifyContext::V8ContextFromContextifiedSandbox(sandbox);
@@ -404,43 +407,76 @@ class ContextifyScript : ObjectWrap {
404407

405408
// Do the eval within the context
406409
Context::Scope context_scope(context);
407-
EvalMachine(timeout, display_exception, args, try_catch);
410+
EvalMachine(timeout, display_errors, args, try_catch);
408411
}
409412

410413
static int64_t GetTimeoutArg(const FunctionCallbackInfo<Value>& args,
411414
const int i) {
412-
if (args[i]->IsUndefined()) {
413-
return 0;
415+
if (args[i]->IsUndefined() || args[i]->IsString()) {
416+
return -1;
417+
}
418+
if (!args[i]->IsObject()) {
419+
ThrowTypeError("options must be an object");
420+
return -1;
421+
}
422+
423+
Local<String> key = FIXED_ONE_BYTE_STRING(node_isolate, "timeout");
424+
Local<Value> value = args[i].As<Object>()->Get(key);
425+
if (value->IsUndefined()) {
426+
return -1;
414427
}
428+
int64_t timeout = value->IntegerValue();
415429

416-
int64_t timeout = args[i]->IntegerValue();
417-
if (timeout < 0) {
430+
if (timeout <= 0) {
418431
ThrowRangeError("timeout must be a positive number");
432+
return -1;
419433
}
420434
return timeout;
421435
}
422436

423-
static bool GetDisplayArg(const FunctionCallbackInfo<Value>& args,
424-
const int i) {
425-
bool display_exception = true;
426437

427-
if (args[i]->IsBoolean())
428-
display_exception = args[i]->BooleanValue();
438+
static bool GetDisplayErrorsArg(const FunctionCallbackInfo<Value>& args,
439+
const int i) {
440+
if (args[i]->IsUndefined() || args[i]->IsString()) {
441+
return true;
442+
}
443+
if (!args[i]->IsObject()) {
444+
ThrowTypeError("options must be an object");
445+
return false;
446+
}
429447

430-
return display_exception;
448+
Local<String> key = FIXED_ONE_BYTE_STRING(node_isolate, "displayErrors");
449+
Local<Value> value = args[i].As<Object>()->Get(key);
450+
451+
return value->IsUndefined() ? true : value->BooleanValue();
431452
}
432453

433454

434455
static Local<String> GetFilenameArg(const FunctionCallbackInfo<Value>& args,
435456
const int i) {
436-
return !args[i]->IsUndefined()
437-
? args[i]->ToString()
438-
: FIXED_ONE_BYTE_STRING(node_isolate, "evalmachine.<anonymous>");
457+
Local<String> defaultFilename =
458+
FIXED_ONE_BYTE_STRING(node_isolate, "evalmachine.<anonymous>");
459+
460+
if (args[i]->IsUndefined()) {
461+
return defaultFilename;
462+
}
463+
if (args[i]->IsString()) {
464+
return args[i].As<String>();
465+
}
466+
if (!args[i]->IsObject()) {
467+
ThrowTypeError("options must be an object");
468+
return Local<String>();
469+
}
470+
471+
Local<String> key = FIXED_ONE_BYTE_STRING(node_isolate, "filename");
472+
Local<Value> value = args[i].As<Object>()->Get(key);
473+
474+
return value->IsUndefined() ? defaultFilename : value->ToString();
439475
}
440476

441477

442478
static void EvalMachine(const int64_t timeout,
443-
const bool display_exception,
479+
const bool display_errors,
444480
const FunctionCallbackInfo<Value>& args,
445481
TryCatch& try_catch) {
446482
if (!ContextifyScript::InstanceOf(args.This())) {
@@ -452,15 +488,9 @@ class ContextifyScript : ObjectWrap {
452488
ObjectWrap::Unwrap<ContextifyScript>(args.This());
453489
Local<Script> script = PersistentToLocal(node_isolate,
454490
wrapped_script->script_);
455-
if (script.IsEmpty()) {
456-
if (display_exception)
457-
DisplayExceptionLine(try_catch.Message());
458-
try_catch.ReThrow();
459-
return;
460-
}
461491

462492
Local<Value> result;
463-
if (timeout) {
493+
if (timeout != -1) {
464494
Watchdog wd(timeout);
465495
result = script->Run();
466496
} else {
@@ -474,8 +504,9 @@ class ContextifyScript : ObjectWrap {
474504

475505
if (result.IsEmpty()) {
476506
// Error occurred during execution of the script.
477-
if (display_exception)
507+
if (display_errors) {
478508
DisplayExceptionLine(try_catch.Message());
509+
}
479510
try_catch.ReThrow();
480511
return;
481512
}

‎test/message/eval_messages.out‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
with(this){__filename}
55
^^^^
66
SyntaxError: Strict mode code may not include a with statement
7-
at Object.exports.createScript (vm.js:*)
87
at Object.exports.runInThisContext (vm.js:*)
98
at Object.<anonymous> ([eval]-wrapper:*:*)
109
at Module._compile (module.js:*:*)

‎test/message/stdin_messages.out‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
with(this){__filename}
55
^^^^
66
SyntaxError: Strict mode code may not include a with statement
7-
at Object.exports.createScript (vm.js:*)
87
at Object.exports.runInThisContext (vm.js:*)
98
at Object.<anonymous> ([stdin]-wrapper:*:*)
109
at Module._compile (module.js:*:*)

0 commit comments

Comments
 (0)