vm: move options checks from C++ to JS

Also introduces stronger type validations for options passed to vm
functions.

PR-URL: https://github.com/nodejs/node/pull/19398
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

3 files changed, 152 insertions, 86 deletions

diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js
index fff82cd0e0..828370f6ea 100644
--- a/lib/internal/bootstrap/loaders.js
+++ b/lib/internal/bootstrap/loaders.js
@@ -88,12 +88,7 @@
     };
   }
 
-  // Minimal sandbox helper
   const ContextifyScript = process.binding('contextify').ContextifyScript;
-  function runInThisContext(code, options) {
-    const script = new ContextifyScript(code, options);
-    return script.runInThisContext();
-  }
 
   // Set up NativeModule
   function NativeModule(id) {
@@ -205,11 +200,9 @@
     this.loading = true;
 
     try {
-      const fn = runInThisContext(source, {
-        filename: this.filename,
-        lineOffset: 0,
-        displayErrors: true
-      });
+      const script = new ContextifyScript(source, this.filename);
+      // Arguments: timeout, displayErrors, breakOnSigint
+      const fn = script.runInThisContext(-1, true, false);
       const requireFn = this.id.startsWith('internal/deps/') ?
         NativeModule.requireForDeps :
         NativeModule.require;
diff --git a/lib/internal/vm/module.js b/lib/internal/vm/module.js
index 9af071ce28..a64ccfe3b2 100644
--- a/lib/internal/vm/module.js
+++ b/lib/internal/vm/module.js
@@ -59,6 +59,10 @@ class Module {
 
     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);
+      }
       if (isContext(options.context)) {
         context = options.context;
       } else {
diff --git a/lib/vm.js b/lib/vm.js
index 47eb1b851f..5a5130d7c9 100644
--- a/lib/vm.js
+++ b/lib/vm.js
@@ -24,63 +24,109 @@
 const {
   ContextifyScript,
   kParsingContext,
-
   makeContext,
   isContext: _isContext,
 } = process.binding('contextify');
 
 const {
   ERR_INVALID_ARG_TYPE,
-  ERR_MISSING_ARGS
+  ERR_OUT_OF_RANGE
 } = require('internal/errors').codes;
-
-// The binding provides a few useful primitives:
-// - Script(code, { filename = "evalmachine.anonymous",
-//                  displayErrors = true } = {})
-//   with methods:
-//   - runInThisContext({ displayErrors = true } = {})
-//   - runInContext(sandbox, { displayErrors = true, timeout = undefined } = {})
-// - makeContext(sandbox)
-// - isContext(sandbox)
-// From this we build the entire documented API.
+const { isUint8Array } = require('internal/util/types');
 
 class Script extends ContextifyScript {
-  constructor(code, options) {
+  constructor(code, options = {}) {
+    code = `${code}`;
+    if (typeof options === 'string') {
+      options = { filename: options };
+    }
+    if (typeof options !== 'object' || options === null) {
+      throw new ERR_INVALID_ARG_TYPE('options', 'Object', options);
+    }
+
+    const {
+      filename = 'evalmachine.<anonymous>',
+      lineOffset = 0,
+      columnOffset = 0,
+      cachedData,
+      produceCachedData = false,
+      [kParsingContext]: parsingContext
+    } = options;
+
+    if (typeof filename !== 'string') {
+      throw new ERR_INVALID_ARG_TYPE('options.filename', 'string', filename);
+    }
+    validateInteger(lineOffset, 'options.lineOffset');
+    validateInteger(columnOffset, 'options.columnOffset');
+    if (cachedData !== undefined && !isUint8Array(cachedData)) {
+      throw new ERR_INVALID_ARG_TYPE('options.cachedData',
+                                     ['Buffer', 'Uint8Array'], cachedData);
+    }
+    if (typeof produceCachedData !== 'boolean') {
+      throw new ERR_INVALID_ARG_TYPE('options.produceCachedData', 'boolean',
+                                     produceCachedData);
+    }
+
     // Calling `ReThrow()` on a native TryCatch does not generate a new
     // abort-on-uncaught-exception check. A dummy try/catch in JS land
     // protects against that.
     try {
-      super(code, options);
+      super(code,
+            filename,
+            lineOffset,
+            columnOffset,
+            cachedData,
+            produceCachedData,
+            parsingContext);
     } catch (e) {
       throw e; /* node-do-not-add-exception-line */
     }
   }
-}
 
-const realRunInThisContext = Script.prototype.runInThisContext;
-const realRunInContext = Script.prototype.runInContext;
+  runInThisContext(options) {
+    const { breakOnSigint, args } = getRunInContextArgs(options);
+    if (breakOnSigint && process.listenerCount('SIGINT') > 0) {
+      return sigintHandlersWrap(super.runInThisContext, this, args);
+    } else {
+      return super.runInThisContext(...args);
+    }
+  }
 
-Script.prototype.runInThisContext = function(options) {
-  if (options && options.breakOnSigint && process.listenerCount('SIGINT') > 0) {
-    return sigintHandlersWrap(realRunInThisContext, this, [options]);
-  } else {
-    return realRunInThisContext.call(this, options);
+  runInContext(contextifiedSandbox, options) {
+    validateContext(contextifiedSandbox);
+    const { breakOnSigint, args } = getRunInContextArgs(options);
+    if (breakOnSigint && process.listenerCount('SIGINT') > 0) {
+      return sigintHandlersWrap(super.runInContext, this,
+                                [contextifiedSandbox, ...args]);
+    } else {
+      return super.runInContext(contextifiedSandbox, ...args);
+    }
   }
-};
 
-Script.prototype.runInContext = function(contextifiedSandbox, options) {
-  if (options && options.breakOnSigint && process.listenerCount('SIGINT') > 0) {
-    return sigintHandlersWrap(realRunInContext, this,
-                              [contextifiedSandbox, options]);
-  } else {
-    return realRunInContext.call(this, contextifiedSandbox, options);
+  runInNewContext(sandbox, options) {
+    const context = createContext(sandbox, getContextOptions(options));
+    return this.runInContext(context, options);
   }
-};
+}
 
-Script.prototype.runInNewContext = function(sandbox, options) {
-  const context = createContext(sandbox, getContextOptions(options));
-  return this.runInContext(context, options);
-};
+function validateContext(sandbox) {
+  if (typeof sandbox !== 'object' || sandbox === null) {
+    throw new ERR_INVALID_ARG_TYPE('contextifiedSandbox', 'Object', sandbox);
+  }
+  if (!_isContext(sandbox)) {
+    throw new ERR_INVALID_ARG_TYPE('contextifiedSandbox', 'vm.Context',
+                                   sandbox);
+  }
+}
+
+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);
+  }
+}
 
 function validateString(prop, propName) {
   if (prop !== undefined && typeof prop !== 'string')
@@ -97,6 +143,39 @@ function validateObject(prop, propName) {
     throw new ERR_INVALID_ARG_TYPE(propName, 'Object', prop);
 }
 
+function getRunInContextArgs(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 {
+    displayErrors = true,
+    breakOnSigint = false
+  } = options;
+
+  if (typeof displayErrors !== 'boolean') {
+    throw new ERR_INVALID_ARG_TYPE('options.displayErrors', 'boolean',
+                                   displayErrors);
+  }
+  if (typeof breakOnSigint !== 'boolean') {
+    throw new ERR_INVALID_ARG_TYPE('options.breakOnSigint', 'boolean',
+                                   breakOnSigint);
+  }
+
+  return {
+    breakOnSigint,
+    args: [timeout, displayErrors, breakOnSigint]
+  };
+}
+
 function getContextOptions(options) {
   if (options) {
     validateObject(options.contextCodeGeneration,
@@ -123,57 +202,43 @@ function getContextOptions(options) {
 }
 
 function isContext(sandbox) {
-  if (arguments.length < 1) {
-    throw new ERR_MISSING_ARGS('sandbox');
+  if (typeof sandbox !== 'object' || sandbox === null) {
+    throw new ERR_INVALID_ARG_TYPE('sandbox', 'Object', sandbox);
   }
-
-  if (typeof sandbox !== 'object' && typeof sandbox !== 'function' ||
-      sandbox === null) {
-    throw new ERR_INVALID_ARG_TYPE('sandbox', 'object', sandbox);
-  }
-
   return _isContext(sandbox);
 }
 
 let defaultContextNameIndex = 1;
-function createContext(sandbox, options) {
-  if (sandbox === undefined) {
-    sandbox = {};
-  } else if (isContext(sandbox)) {
+function createContext(sandbox = {}, options = {}) {
+  if (isContext(sandbox)) {
     return sandbox;
   }
 
-  if (options !== undefined) {
-    if (typeof options !== 'object' || options === null) {
-      throw new ERR_INVALID_ARG_TYPE('options', 'object', options);
-    }
-    validateObject(options.codeGeneration, 'options.codeGeneration');
-    options = {
-      name: options.name,
-      origin: options.origin,
-      codeGeneration: typeof options.codeGeneration === 'object' ? {
-        strings: options.codeGeneration.strings,
-        wasm: options.codeGeneration.wasm,
-      } : undefined,
-    };
-    if (options.codeGeneration !== undefined) {
-      validateBool(options.codeGeneration.strings,
-                   'options.codeGeneration.strings');
-      validateBool(options.codeGeneration.wasm,
-                   'options.codeGeneration.wasm');
-    }
-    if (options.name === undefined) {
-      options.name = `VM Context ${defaultContextNameIndex++}`;
-    } else if (typeof options.name !== 'string') {
-      throw new ERR_INVALID_ARG_TYPE('options.name', 'string', options.name);
-    }
-    validateString(options.origin, 'options.origin');
-  } else {
-    options = {
-      name: `VM Context ${defaultContextNameIndex++}`
-    };
+  if (typeof options !== 'object' || options === null) {
+    throw new ERR_INVALID_ARG_TYPE('options', 'Object', options);
   }
-  makeContext(sandbox, options);
+
+  const {
+    name = `VM Context ${defaultContextNameIndex++}`,
+    origin,
+    codeGeneration
+  } = options;
+
+  if (typeof name !== 'string') {
+    throw new ERR_INVALID_ARG_TYPE('options.name', 'string', options.name);
+  }
+  validateString(origin, 'options.origin');
+  validateObject(codeGeneration, 'options.codeGeneration');
+
+  let strings = true;
+  let wasm = true;
+  if (codeGeneration !== undefined) {
+    ({ strings = true, wasm = true } = codeGeneration);
+    validateBool(strings, 'options.codeGeneration.strings');
+    validateBool(wasm, 'options.codeGeneration.wasm');
+  }
+
+  makeContext(sandbox, name, origin, strings, wasm);
   return sandbox;
 }
 
@@ -200,6 +265,7 @@ function sigintHandlersWrap(fn, thisArg, argsArray) {
 }
 
 function runInContext(code, contextifiedSandbox, options) {
+  validateContext(contextifiedSandbox);
   if (typeof options === 'string') {
     options = {
       filename: options,
@@ -226,6 +292,9 @@ function runInNewContext(code, sandbox, options) {
 }
 
 function runInThisContext(code, options) {
+  if (typeof options === 'string') {
+    options = { filename: options };
+  }
   return createScript(code, options).runInThisContext(options);
 }