Skip to content

Commit 34d988f

Browse files
targostargos
committed
vm: move options checks from C++ to JS
Also introduces stronger type validations for options passed to vm functions. PR-URL: #19398 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
1 parent a820f41 commit 34d988f

File tree

13 files changed

+458
-411
lines changed

13 files changed

+458
-411
lines changed

‎doc/api/vm.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -618,6 +618,9 @@ console.log(globalVar);
618618
<!-- YAML
619619
added: v0.3.1
620620
changes:
621+
- version: REPLACEME
622+
pr-url: https://github.com/nodejs/node/pull/19398
623+
description: The `sandbox` option can no longer be a function.
621624
- version: REPLACEME
622625
pr-url: https://github.com/nodejs/node/pull/19016
623626
description: The `codeGeneration` option is supported now.

‎lib/internal/bootstrap/loaders.js‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -88,12 +88,7 @@
8888
};
8989
}
9090

91-
// Minimal sandbox helper
9291
const ContextifyScript = process.binding('contextify').ContextifyScript;
93-
function runInThisContext(code, options) {
94-
const script = new ContextifyScript(code, options);
95-
return script.runInThisContext();
96-
}
9792

9893
// Set up NativeModule
9994
function NativeModule(id) {
@@ -205,11 +200,9 @@
205200
this.loading = true;
206201

207202
try {
208-
const fn = runInThisContext(source, {
209-
filename: this.filename,
210-
lineOffset: 0,
211-
displayErrors: true
212-
});
203+
const script = new ContextifyScript(source, this.filename);
204+
// Arguments: timeout, displayErrors, breakOnSigint
205+
const fn = script.runInThisContext(-1, true, false);
213206
const requireFn = this.id.startsWith('internal/deps/') ?
214207
NativeModule.requireForDeps :
215208
NativeModule.require;

‎lib/internal/vm/module.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ class Module {
5959

6060
let context;
6161
if (options.context !== undefined) {
62+
if (typeof options.context !== 'object' || options.context === null) {
63+
throw new ERR_INVALID_ARG_TYPE('options.context', 'object',
64+
options.context);
65+
}
6266
if (isContext(options.context)) {
6367
context = options.context;
6468
} else {

‎lib/vm.js‎

Lines changed: 145 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -24,63 +24,109 @@
2424
const {
2525
ContextifyScript,
2626
kParsingContext,
27-
2827
makeContext,
2928
isContext: _isContext,
3029
} = process.binding('contextify');
3130

3231
const {
3332
ERR_INVALID_ARG_TYPE,
34-
ERR_MISSING_ARGS
33+
ERR_OUT_OF_RANGE
3534
} = require('internal/errors').codes;
36-
37-
// The binding provides a few useful primitives:
38-
// - Script(code, { filename = "evalmachine.anonymous",
39-
// displayErrors = true } = {})
40-
// with methods:
41-
// - runInThisContext({ displayErrors = true } = {})
42-
// - runInContext(sandbox, { displayErrors = true, timeout = undefined } = {})
43-
// - makeContext(sandbox)
44-
// - isContext(sandbox)
45-
// From this we build the entire documented API.
35+
const { isUint8Array } = require('internal/util/types');
4636

4737
class Script extends ContextifyScript {
48-
constructor(code, options) {
38+
constructor(code, options = {}) {
39+
code = `${code}`;
40+
if (typeof options === 'string') {
41+
options = { filename: options };
42+
}
43+
if (typeof options !== 'object' || options === null) {
44+
throw new ERR_INVALID_ARG_TYPE('options', 'Object', options);
45+
}
46+
47+
const {
48+
filename = 'evalmachine.<anonymous>',
49+
lineOffset = 0,
50+
columnOffset = 0,
51+
cachedData,
52+
produceCachedData = false,
53+
[kParsingContext]: parsingContext
54+
} = options;
55+
56+
if (typeof filename !== 'string') {
57+
throw new ERR_INVALID_ARG_TYPE('options.filename', 'string', filename);
58+
}
59+
validateInteger(lineOffset, 'options.lineOffset');
60+
validateInteger(columnOffset, 'options.columnOffset');
61+
if (cachedData !== undefined && !isUint8Array(cachedData)) {
62+
throw new ERR_INVALID_ARG_TYPE('options.cachedData',
63+
['Buffer', 'Uint8Array'], cachedData);
64+
}
65+
if (typeof produceCachedData !== 'boolean') {
66+
throw new ERR_INVALID_ARG_TYPE('options.produceCachedData', 'boolean',
67+
produceCachedData);
68+
}
69+
4970
// Calling `ReThrow()` on a native TryCatch does not generate a new
5071
// abort-on-uncaught-exception check. A dummy try/catch in JS land
5172
// protects against that.
5273
try {
53-
super(code, options);
74+
super(code,
75+
filename,
76+
lineOffset,
77+
columnOffset,
78+
cachedData,
79+
produceCachedData,
80+
parsingContext);
5481
} catch (e) {
5582
throw e; /* node-do-not-add-exception-line */
5683
}
5784
}
58-
}
5985

60-
const realRunInThisContext = Script.prototype.runInThisContext;
61-
const realRunInContext = Script.prototype.runInContext;
86+
runInThisContext(options) {
87+
const { breakOnSigint, args } = getRunInContextArgs(options);
88+
if (breakOnSigint && process.listenerCount('SIGINT') > 0) {
89+
return sigintHandlersWrap(super.runInThisContext, this, args);
90+
} else {
91+
return super.runInThisContext(...args);
92+
}
93+
}
6294

63-
Script.prototype.runInThisContext = function(options) {
64-
if (options && options.breakOnSigint && process.listenerCount('SIGINT') > 0) {
65-
return sigintHandlersWrap(realRunInThisContext, this, [options]);
66-
} else {
67-
return realRunInThisContext.call(this, options);
95+
runInContext(contextifiedSandbox, options) {
96+
validateContext(contextifiedSandbox);
97+
const { breakOnSigint, args } = getRunInContextArgs(options);
98+
if (breakOnSigint && process.listenerCount('SIGINT') > 0) {
99+
return sigintHandlersWrap(super.runInContext, this,
100+
[contextifiedSandbox, ...args]);
101+
} else {
102+
return super.runInContext(contextifiedSandbox, ...args);
103+
}
68104
}
69-
};
70105

71-
Script.prototype.runInContext = function(contextifiedSandbox, options) {
72-
if (options && options.breakOnSigint && process.listenerCount('SIGINT') > 0) {
73-
return sigintHandlersWrap(realRunInContext, this,
74-
[contextifiedSandbox, options]);
75-
} else {
76-
return realRunInContext.call(this, contextifiedSandbox, options);
106+
runInNewContext(sandbox, options) {
107+
const context = createContext(sandbox, getContextOptions(options));
108+
return this.runInContext(context, options);
77109
}
78-
};
110+
}
79111

80-
Script.prototype.runInNewContext = function(sandbox, options) {
81-
const context = createContext(sandbox, getContextOptions(options));
82-
return this.runInContext(context, options);
83-
};
112+
function validateContext(sandbox) {
113+
if (typeof sandbox !== 'object' || sandbox === null) {
114+
throw new ERR_INVALID_ARG_TYPE('contextifiedSandbox', 'Object', sandbox);
115+
}
116+
if (!_isContext(sandbox)) {
117+
throw new ERR_INVALID_ARG_TYPE('contextifiedSandbox', 'vm.Context',
118+
sandbox);
119+
}
120+
}
121+
122+
function validateInteger(prop, propName) {
123+
if (!Number.isInteger(prop)) {
124+
throw new ERR_INVALID_ARG_TYPE(propName, 'integer', prop);
125+
}
126+
if ((prop >> 0) !== prop) {
127+
throw new ERR_OUT_OF_RANGE(propName, '32-bit integer', prop);
128+
}
129+
}
84130

85131
function validateString(prop, propName) {
86132
if (prop !== undefined && typeof prop !== 'string')
@@ -97,6 +143,39 @@ function validateObject(prop, propName) {
97143
throw new ERR_INVALID_ARG_TYPE(propName, 'Object', prop);
98144
}
99145

146+
function getRunInContextArgs(options = {}) {
147+
if (typeof options !== 'object' || options === null) {
148+
throw new ERR_INVALID_ARG_TYPE('options', 'Object', options);
149+
}
150+
151+
let timeout = options.timeout;
152+
if (timeout === undefined) {
153+
timeout = -1;
154+
} else if (!Number.isInteger(timeout) || timeout <= 0) {
155+
throw new ERR_INVALID_ARG_TYPE('options.timeout', 'a positive integer',
156+
timeout);
157+
}
158+
159+
const {
160+
displayErrors = true,
161+
breakOnSigint = false
162+
} = options;
163+
164+
if (typeof displayErrors !== 'boolean') {
165+
throw new ERR_INVALID_ARG_TYPE('options.displayErrors', 'boolean',
166+
displayErrors);
167+
}
168+
if (typeof breakOnSigint !== 'boolean') {
169+
throw new ERR_INVALID_ARG_TYPE('options.breakOnSigint', 'boolean',
170+
breakOnSigint);
171+
}
172+
173+
return {
174+
breakOnSigint,
175+
args: [timeout, displayErrors, breakOnSigint]
176+
};
177+
}
178+
100179
function getContextOptions(options) {
101180
if (options) {
102181
validateObject(options.contextCodeGeneration,
@@ -123,57 +202,43 @@ function getContextOptions(options) {
123202
}
124203

125204
function isContext(sandbox) {
126-
if (arguments.length < 1) {
127-
throw new ERR_MISSING_ARGS('sandbox');
205+
if (typeof sandbox !== 'object' || sandbox === null) {
206+
throw new ERR_INVALID_ARG_TYPE('sandbox', 'Object', sandbox);
128207
}
129-
130-
if (typeof sandbox !== 'object' && typeof sandbox !== 'function' ||
131-
sandbox === null) {
132-
throw new ERR_INVALID_ARG_TYPE('sandbox', 'object', sandbox);
133-
}
134-
135208
return _isContext(sandbox);
136209
}
137210

138211
let defaultContextNameIndex = 1;
139-
function createContext(sandbox, options) {
140-
if (sandbox === undefined) {
141-
sandbox = {};
142-
} else if (isContext(sandbox)) {
212+
function createContext(sandbox = {}, options = {}) {
213+
if (isContext(sandbox)) {
143214
return sandbox;
144215
}
145216

146-
if (options !== undefined) {
147-
if (typeof options !== 'object' || options === null) {
148-
throw new ERR_INVALID_ARG_TYPE('options', 'object', options);
149-
}
150-
validateObject(options.codeGeneration, 'options.codeGeneration');
151-
options = {
152-
name: options.name,
153-
origin: options.origin,
154-
codeGeneration: typeof options.codeGeneration === 'object' ? {
155-
strings: options.codeGeneration.strings,
156-
wasm: options.codeGeneration.wasm,
157-
} : undefined,
158-
};
159-
if (options.codeGeneration !== undefined) {
160-
validateBool(options.codeGeneration.strings,
161-
'options.codeGeneration.strings');
162-
validateBool(options.codeGeneration.wasm,
163-
'options.codeGeneration.wasm');
164-
}
165-
if (options.name === undefined) {
166-
options.name = `VM Context ${defaultContextNameIndex++}`;
167-
} else if (typeof options.name !== 'string') {
168-
throw new ERR_INVALID_ARG_TYPE('options.name', 'string', options.name);
169-
}
170-
validateString(options.origin, 'options.origin');
171-
} else {
172-
options = {
173-
name: `VM Context ${defaultContextNameIndex++}`
174-
};
217+
if (typeof options !== 'object' || options === null) {
218+
throw new ERR_INVALID_ARG_TYPE('options', 'Object', options);
175219
}
176-
makeContext(sandbox, options);
220+
221+
const {
222+
name = `VM Context ${defaultContextNameIndex++}`,
223+
origin,
224+
codeGeneration
225+
} = options;
226+
227+
if (typeof name !== 'string') {
228+
throw new ERR_INVALID_ARG_TYPE('options.name', 'string', options.name);
229+
}
230+
validateString(origin, 'options.origin');
231+
validateObject(codeGeneration, 'options.codeGeneration');
232+
233+
let strings = true;
234+
let wasm = true;
235+
if (codeGeneration !== undefined) {
236+
({ strings = true, wasm = true } = codeGeneration);
237+
validateBool(strings, 'options.codeGeneration.strings');
238+
validateBool(wasm, 'options.codeGeneration.wasm');
239+
}
240+
241+
makeContext(sandbox, name, origin, strings, wasm);
177242
return sandbox;
178243
}
179244

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

202267
function runInContext(code, contextifiedSandbox, options) {
268+
validateContext(contextifiedSandbox);
203269
if (typeof options === 'string') {
204270
options = {
205271
filename: options,
@@ -226,6 +292,9 @@ function runInNewContext(code, sandbox, options) {
226292
}
227293

228294
function runInThisContext(code, options) {
295+
if (typeof options === 'string') {
296+
options = { filename: options };
297+
}
229298
return createScript(code, options).runInThisContext(options);
230299
}
231300

‎src/env.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,6 @@ struct PackageConfig {
239239
V(port_string, "port") \
240240
V(preference_string, "preference") \
241241
V(priority_string, "priority") \
242-
V(produce_cached_data_string, "produceCachedData") \
243242
V(promise_string, "promise") \
244243
V(pubkey_string, "pubkey") \
245244
V(query_string, "query") \

0 commit comments

Comments
 (0)