Skip to content

Commit 58b2482

Browse files
XmiliaHXmiliaH
authored
Merge pull request #286 from XmiliaH/fix-279
Fix false positive async errors
2 parents 6f1e2c1 + 2503d3d commit 58b2482

File tree

4 files changed

+125
-100
lines changed

4 files changed

+125
-100
lines changed

‎lib/contextify.js

+1
Original file line numberDiff line numberDiff line change
@@ -983,5 +983,6 @@ exportsMap.Contextify = Contextify;
983983
exportsMap.Decontextify = Decontextify;
984984
exportsMap.Buffer = LocalBuffer;
985985
exportsMap.sandbox = Decontextify.value(global);
986+
exportsMap.Function = Function;
986987

987988
return exportsMap;

‎lib/fixasync.js

+39-69
Original file line numberDiff line numberDiff line change
@@ -1,109 +1,79 @@
11
'use strict';
22

33
// eslint-disable-next-line no-invalid-this, no-shadow
4-
const {GeneratorFunction, AsyncFunction, AsyncGeneratorFunction, global, Contextify, host} = this;
4+
const {GeneratorFunction, AsyncFunction, AsyncGeneratorFunction, global, internal, host, hook} = this;
5+
const {Contextify, Decontextify} = internal;
56
// eslint-disable-next-line no-shadow
6-
const {Function, eval: eval_, Promise, Object, Reflect, RegExp, VMError} = global;
7-
const {setPrototypeOf, getOwnPropertyDescriptor, defineProperty} = Object;
7+
const {Function, eval: eval_, Promise, Object, Reflect} = global;
8+
const {getOwnPropertyDescriptor, defineProperty, assign} = Object;
89
const {apply: rApply, construct: rConstruct} = Reflect;
9-
const {test} = RegExp.prototype;
1010

11-
function rejectAsync() {
12-
throw new VMError('Async not available');
13-
}
14-
15-
const testAsync = setPrototypeOf(/\basync\b/, null);
16-
17-
function checkAsync(source) {
18-
// Filter async functions, await can only be in them.
19-
if (rApply(test, testAsync, [source])) {
20-
throw rejectAsync();
21-
}
22-
return source;
23-
}
24-
25-
const AsyncCheckHandler = {
11+
const FunctionHandler = {
2612
__proto__: null,
2713
apply(target, thiz, args) {
28-
let i;
29-
for (i=0; i<args.length; i++) {
30-
// We want a exception here if args[i] is a Symbol
31-
// since Function does the same thing
32-
args[i] = checkAsync('' + args[i]);
14+
const type = this.type;
15+
args = Decontextify.arguments(args);
16+
try {
17+
args = Contextify.value(hook(type, args));
18+
} catch (e) {
19+
throw Contextify.value(e);
3320
}
34-
return rApply(target, undefined, args);
21+
return rApply(target, thiz, args);
3522
},
3623
construct(target, args, newTarget) {
37-
let i;
38-
for (i=0; i<args.length; i++) {
39-
// We want a exception here if args[i] is a Symbol
40-
// since Function does the same thing
41-
args[i] = checkAsync('' + args[i]);
24+
const type = this.type;
25+
args = Decontextify.arguments(args);
26+
try {
27+
args = Contextify.value(hook(type, args));
28+
} catch (e) {
29+
throw Contextify.value(e);
4230
}
43-
return rConstruct(target, args);
31+
return rConstruct(target, args, newTarget);
4432
}
4533
};
4634

47-
const AsyncEvalHandler = {
48-
__proto__: null,
49-
apply(target, thiz, args) {
50-
if (args.length === 0) return undefined;
51-
const script = args[0];
52-
if (typeof script !== 'string') {
53-
// Eval does the same thing
54-
return script;
55-
}
56-
checkAsync(script);
57-
return eval_(script);
58-
}
59-
};
35+
function makeCheckFunction(type) {
36+
return assign({
37+
__proto__: null,
38+
type
39+
}, FunctionHandler);
40+
}
6041

6142
function override(obj, prop, value) {
6243
const desc = getOwnPropertyDescriptor(obj, prop);
6344
desc.value = value;
6445
defineProperty(obj, prop, desc);
6546
}
6647

67-
const proxiedFunction = new host.Proxy(Function, AsyncCheckHandler);
48+
const proxiedFunction = new host.Proxy(Function, makeCheckFunction('function'));
6849
override(Function.prototype, 'constructor', proxiedFunction);
6950
if (GeneratorFunction) {
7051
Object.setPrototypeOf(GeneratorFunction, proxiedFunction);
71-
override(GeneratorFunction.prototype, 'constructor', new host.Proxy(GeneratorFunction, AsyncCheckHandler));
52+
override(GeneratorFunction.prototype, 'constructor', new host.Proxy(GeneratorFunction, makeCheckFunction('generator_function')));
7253
}
73-
if (AsyncFunction || AsyncGeneratorFunction) {
74-
const AsyncFunctionRejectHandler = {
75-
__proto__: null,
76-
apply: rejectAsync,
77-
construct: rejectAsync
78-
};
79-
if (AsyncFunction) {
80-
Object.setPrototypeOf(AsyncFunction, proxiedFunction);
81-
override(AsyncFunction.prototype, 'constructor', new host.Proxy(AsyncFunction, AsyncFunctionRejectHandler));
82-
}
83-
if (AsyncGeneratorFunction) {
84-
Object.setPrototypeOf(AsyncGeneratorFunction, proxiedFunction);
85-
override(AsyncGeneratorFunction.prototype, 'constructor', new host.Proxy(AsyncGeneratorFunction, AsyncFunctionRejectHandler));
86-
}
54+
if (AsyncFunction) {
55+
Object.setPrototypeOf(AsyncFunction, proxiedFunction);
56+
override(AsyncFunction.prototype, 'constructor', new host.Proxy(AsyncFunction, makeCheckFunction('async_function')));
57+
}
58+
if (AsyncGeneratorFunction) {
59+
Object.setPrototypeOf(AsyncGeneratorFunction, proxiedFunction);
60+
override(AsyncGeneratorFunction.prototype, 'constructor', new host.Proxy(AsyncGeneratorFunction, makeCheckFunction('async_generator_function')));
8761
}
8862

89-
global.Function = Function.prototype.constructor;
90-
global.eval = new host.Proxy(eval_, AsyncEvalHandler);
63+
global.Function = proxiedFunction;
64+
global.eval = new host.Proxy(eval_, makeCheckFunction('eval'));
9165

9266
if (Promise) {
93-
const AsyncRejectHandler = {
94-
__proto__: null,
95-
apply: rejectAsync
96-
};
9767

98-
Promise.prototype.then = new host.Proxy(Promise.prototype.then, AsyncRejectHandler);
68+
Promise.prototype.then = new host.Proxy(Promise.prototype.then, makeCheckFunction('promise_then'));
9969
Contextify.connect(host.Promise.prototype.then, Promise.prototype.then);
10070

10171
if (Promise.prototype.finally) {
102-
Promise.prototype.finally = new host.Proxy(Promise.prototype.finally, AsyncRejectHandler);
72+
Promise.prototype.finally = new host.Proxy(Promise.prototype.finally, makeCheckFunction('promise_finally'));
10373
Contextify.connect(host.Promise.prototype.finally, Promise.prototype.finally);
10474
}
10575
if (Promise.prototype.catch) {
106-
Promise.prototype.catch = new host.Proxy(Promise.prototype.catch, AsyncRejectHandler);
76+
Promise.prototype.catch = new host.Proxy(Promise.prototype.catch, makeCheckFunction('promise_catch'));
10777
Contextify.connect(host.Promise.prototype.catch, Promise.prototype.catch);
10878
}
10979

‎lib/main.js

+70-19
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ const CACHE = {
6060
timeoutScript: null,
6161
contextifyScript: loadAndCompileScript(`${__dirname}/contextify.js`, '(function(require, host) { ', '\n})'),
6262
sandboxScript: null,
63-
fixAsyncScript: null,
63+
hookScript: null,
6464
getGlobalScript: null,
6565
getGeneratorFunctionScript: null,
6666
getAsyncFunctionScript: null,
@@ -468,6 +468,43 @@ function doWithTimeout(fn, timeout) {
468468
}
469469
}
470470

471+
function makeCheckAsync(internal) {
472+
return (hook, args) => {
473+
if (hook === 'function' || hook === 'generator_function' || hook === 'eval' || hook === 'run') {
474+
const funcConstructor = internal.Function;
475+
if (hook === 'eval') {
476+
const script = args[0];
477+
args = [script];
478+
if (typeof(script) !== 'string') return args;
479+
} else {
480+
// Next line throws on Symbol, this is the same behavior as function constructor calls
481+
args = args.map(arg => `${arg}`);
482+
}
483+
if (args.findIndex(arg => /\basync\b/.test(arg)) === -1) return args;
484+
const asyncMapped = args.map(arg => arg.replace(/async/g, 'a\\u0073ync'));
485+
try {
486+
// Note: funcConstructor is a Sandbox object, however, asyncMapped are only strings.
487+
funcConstructor(...asyncMapped);
488+
} catch (u) {
489+
// u is a sandbox object
490+
// Some random syntax error or error because of async.
491+
492+
// First report real syntax errors
493+
try {
494+
// Note: funcConstructor is a Sandbox object, however, args are only strings.
495+
funcConstructor(...args);
496+
} catch (e) {
497+
throw internal.Decontextify.value(e);
498+
}
499+
// Then async error
500+
throw new VMError('Async not available');
501+
}
502+
return args;
503+
}
504+
throw new VMError('Async not available');
505+
};
506+
}
507+
471508
/**
472509
* Class VM.
473510
*
@@ -578,6 +615,8 @@ class VM extends EventEmitter {
578615
// Create the bridge between the host and the sandbox.
579616
const _internal = CACHE.contextifyScript.runInContext(_context, DEFAULT_RUN_OPTIONS).call(_context, require, HOST);
580617

618+
const hook = fixAsync ? makeCheckAsync(_internal) : null;
619+
581620
// Define the properties of this object.
582621
// Use Object.defineProperties here to be able to
583622
// hide and set properties write only.
@@ -598,12 +637,12 @@ class VM extends EventEmitter {
598637
_context: {value: _context},
599638
_internal: {value: _internal},
600639
_compiler: {value: resolvedCompiler},
601-
_fixAsync: {value: fixAsync}
640+
_hook: {value: hook}
602641
});
603642

604-
if (fixAsync) {
605-
if (!CACHE.fixAsyncScript) {
606-
CACHE.fixAsyncScript = loadAndCompileScript(`${__dirname}/fixasync.js`, '(function() { ', '\n})');
643+
if (hook) {
644+
if (!CACHE.hookScript) {
645+
CACHE.hookScript = loadAndCompileScript(`${__dirname}/fixasync.js`, '(function() { ', '\n})');
607646
CACHE.getGlobalScript = new vm.Script('this', {
608647
filename: 'get_global.js',
609648
displayErrors: false
@@ -629,26 +668,27 @@ class VM extends EventEmitter {
629668
}
630669
const internal = {
631670
__proto__: null,
632-
global: CACHE.getGlobalScript.runInContext(this._context, DEFAULT_RUN_OPTIONS),
633-
Contextify: this._internal.Contextify,
634-
host: HOST
671+
global: CACHE.getGlobalScript.runInContext(_context, DEFAULT_RUN_OPTIONS),
672+
internal: _internal,
673+
host: HOST,
674+
hook
635675
};
636676
if (CACHE.getGeneratorFunctionScript) {
637677
try {
638-
internal.GeneratorFunction = CACHE.getGeneratorFunctionScript.runInContext(this._context, DEFAULT_RUN_OPTIONS);
678+
internal.GeneratorFunction = CACHE.getGeneratorFunctionScript.runInContext(_context, DEFAULT_RUN_OPTIONS);
639679
} catch (ex) {}
640680
}
641681
if (CACHE.getAsyncFunctionScript) {
642682
try {
643-
internal.AsyncFunction = CACHE.getAsyncFunctionScript.runInContext(this._context, DEFAULT_RUN_OPTIONS);
683+
internal.AsyncFunction = CACHE.getAsyncFunctionScript.runInContext(_context, DEFAULT_RUN_OPTIONS);
644684
} catch (ex) {}
645685
}
646686
if (CACHE.getAsyncGeneratorFunctionScript) {
647687
try {
648-
internal.AsyncGeneratorFunction = CACHE.getAsyncGeneratorFunctionScript.runInContext(this._context, DEFAULT_RUN_OPTIONS);
688+
internal.AsyncGeneratorFunction = CACHE.getAsyncGeneratorFunctionScript.runInContext(_context, DEFAULT_RUN_OPTIONS);
649689
} catch (ex) {}
650690
}
651-
CACHE.fixAsyncScript.runInContext(this._context, DEFAULT_RUN_OPTIONS).call(internal);
691+
CACHE.hookScript.runInContext(_context, DEFAULT_RUN_OPTIONS).call(internal);
652692
}
653693

654694
// prepare global sandbox
@@ -748,17 +788,28 @@ class VM extends EventEmitter {
748788
run(code, filename) {
749789
let script;
750790
if (code instanceof VMScript) {
751-
if (this._fixAsync && /\basync\b/.test(code.code)) {
752-
throw new VMError('Async not available');
791+
if (this._hook) {
792+
const scriptCode = code.code;
793+
const changed = this._hook('run', [scriptCode])[0];
794+
if (changed === scriptCode) {
795+
script = code._compileVM();
796+
} else {
797+
script = new vm.Script(changed, {
798+
filename: code.filename,
799+
displayErrors: false
800+
});
801+
}
802+
} else {
803+
script = code._compileVM();
753804
}
754-
script = code._compileVM();
755805
} else {
756-
if (this._fixAsync && /\basync\b/.test(code)) {
757-
throw new VMError('Async not available');
758-
}
759806
const useFileName = filename || 'vm.js';
807+
let scriptCode = this._compiler(code, useFileName);
808+
if (this._hook) {
809+
scriptCode = this._hook('run', [scriptCode])[0];
810+
}
760811
// Compile the script here so that we don't need to create a instance of VMScript.
761-
script = new vm.Script(this._compiler(code, useFileName), {
812+
script = new vm.Script(scriptCode, {
762813
filename: useFileName,
763814
displayErrors: false
764815
});

‎test/vm.js

+15-12
Original file line numberDiff line numberDiff line change
@@ -363,18 +363,21 @@ describe('VM', () => {
363363
});
364364
}
365365

366-
it('async', () => {
367-
const vm2 = new VM({fixAsync: true});
368-
assert.throws(() => vm2.run('(async function(){})'), /Async not available/, '#1');
369-
assert.throws(() => vm2.run('new Function("as"+"ync function(){}")'), /Async not available/, '#2');
370-
assert.throws(() => vm2.run('new (function*(){}).constructor("as"+"ync function(){}")'), /Async not available/, '#3');
371-
assert.throws(() => vm2.run('Promise.resolve().then(function(){})'), /Async not available/, '#4');
372-
if (Promise.prototype.finally) assert.throws(() => vm2.run('Promise.resolve().finally(function(){})'), /Async not available/, '#5');
373-
if (Promise.prototype.catch) assert.throws(() => vm2.run('Promise.resolve().catch(function(){})'), /Async not available/, '#6');
374-
assert.throws(() => vm2.run('eval("as"+"ync function(){}")'), /Async not available/, '#7');
375-
assert.strictEqual(vm2.run('Object.getPrototypeOf((function*(){}).constructor)'), vm2.run('Function'), '#8');
376-
assert.throws(() => vm2.run('Function')('async function(){}'), /Async not available/, '#9');
377-
});
366+
if (NODE_VERSION > 7) {
367+
// Node until 7 had no async, see https://node.green/
368+
it('async', () => {
369+
const vm2 = new VM({fixAsync: true});
370+
assert.throws(() => vm2.run('(async function(){})'), /Async not available/, '#1');
371+
assert.strictEqual(vm2.run('Object.getPrototypeOf((function*(){}).constructor)'), vm2.run('Function'), '#2');
372+
assert.throws(() => vm2.run('new Function("(as"+"ync function(){})")'), /Async not available/, '#3');
373+
assert.throws(() => vm2.run('new (function*(){}).constructor("(as"+"ync function(){})")'), /Async not available/, '#4');
374+
assert.throws(() => vm2.run('Promise.resolve().then(function(){})'), /Async not available/, '#5');
375+
if (Promise.prototype.finally) assert.throws(() => vm2.run('Promise.resolve().finally(function(){})'), /Async not available/, '#6');
376+
if (Promise.prototype.catch) assert.throws(() => vm2.run('Promise.resolve().catch(function(){})'), /Async not available/, '#7');
377+
assert.throws(() => vm2.run('eval("(as"+"ync function(){})")'), /Async not available/, '#8');
378+
assert.throws(() => vm2.run('Function')('(async function(){})'), /Async not available/, '#9');
379+
});
380+
}
378381

379382
it('frozen unconfigurable access', () => {
380383
const vm2 = new VM();

0 commit comments

Comments
 (0)