Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

vm: move options checks from C++ to JS #19398

Closed
wants to merge 1 commit into from
Closed

vm: move options checks from C++ to JS #19398

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions doc/api/vm.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,9 @@ console.log(globalVar);
<!-- YAML
added: v0.3.1
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/19398
description: The `sandbox` option can no longer be a function.
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/19016
description: The `codeGeneration` option is supported now.
Expand Down
13 changes: 3 additions & 10 deletions lib/internal/bootstrap/loaders.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions lib/internal/vm/module.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,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 {
Expand Down
221 changes: 145 additions & 76 deletions lib/vm.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to take the opportunity to do stricter verifications of the *Offset and produceCachedData options. Good idea?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go ahead!

[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')
Expand All @@ -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,
Expand All @@ -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;
}

Expand All @@ -200,6 +265,7 @@ function sigintHandlersWrap(fn, thisArg, argsArray) {
}

function runInContext(code, contextifiedSandbox, options) {
validateContext(contextifiedSandbox);
if (typeof options === 'string') {
options = {
filename: options,
Expand All @@ -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);
}

Expand Down
1 change: 0 additions & 1 deletion src/env.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,6 @@ struct PackageConfig {
V(port_string, "port") \
V(preference_string, "preference") \
V(priority_string, "priority") \
V(produce_cached_data_string, "produceCachedData") \
V(promise_string, "promise") \
V(pubkey_string, "pubkey") \
V(query_string, "query") \
Expand Down
Loading