Skip to content

Commit

Permalink
vm: allow cachedData to also be TypedArray|DataView
Browse files Browse the repository at this point in the history
PR-URL: nodejs#22921
Refs: nodejs#1826
Refs: nodejs#22921 (comment)
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
  • Loading branch information
BeniCheni authored and refack committed Nov 2, 2018
1 parent 74ba482 commit 65fe999
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 29 deletions.
13 changes: 7 additions & 6 deletions doc/api/vm.md
Original file line number Diff line number Diff line change
Expand Up @@ -434,10 +434,10 @@ changes:
in stack traces produced by this script.
* `columnOffset` {number} Specifies the column number offset that is displayed
in stack traces produced by this script.
* `cachedData` {Buffer} Provides an optional `Buffer` with V8's code cache
data for the supplied source. When supplied, the `cachedDataRejected` value
will be set to either `true` or `false` depending on acceptance of the data
by V8.
* `cachedData` {Buffer|TypedArray|DataView} Provides an optional `Buffer` or
`TypedArray`, or `DataView` with V8's code cache data for the supplied
source. When supplied, the `cachedDataRejected` value will be set to
either `true` or `false` depending on acceptance of the data by V8.
* `produceCachedData` {boolean} When `true` and no `cachedData` is present, V8
will attempt to produce code cache data for `code`. Upon success, a
`Buffer` with V8's code cache data will be produced and stored in the
Expand Down Expand Up @@ -669,8 +669,9 @@ added: v10.10.0
in stack traces produced by this script. **Default:** `0`.
* `columnOffset` {number} Specifies the column number offset that is displayed
in stack traces produced by this script. **Default:** `0`.
* `cachedData` {Buffer} Provides an optional `Buffer` with V8's code cache
data for the supplied source.
* `cachedData` {Buffer|TypedArray|DataView} Provides an optional `Buffer` or
`TypedArray`, or `DataView` with V8's code cache data for the supplied
source.
* `produceCachedData` {boolean} Specifies whether to produce new cache data.
**Default:** `false`.
* `parsingContext` {Object} The [contextified][] sandbox in which the said
Expand Down
15 changes: 9 additions & 6 deletions lib/vm.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const {
ERR_INVALID_ARG_TYPE,
ERR_VM_MODULE_NOT_MODULE,
} = require('internal/errors').codes;
const { isModuleNamespaceObject, isUint8Array } = require('util').types;
const { isModuleNamespaceObject, isArrayBufferView } = require('util').types;
const { validateInt32, validateUint32 } = require('internal/validators');
const kParsingContext = Symbol('script parsing context');

Expand Down Expand Up @@ -64,9 +64,12 @@ class Script extends ContextifyScript {
}
validateInt32(lineOffset, 'options.lineOffset');
validateInt32(columnOffset, 'options.columnOffset');
if (cachedData !== undefined && !isUint8Array(cachedData)) {
throw new ERR_INVALID_ARG_TYPE('options.cachedData',
['Buffer', 'Uint8Array'], cachedData);
if (cachedData !== undefined && !isArrayBufferView(cachedData)) {
throw new ERR_INVALID_ARG_TYPE(
'options.cachedData',
['Buffer', 'TypedArray', 'DataView'],
cachedData
);
}
if (typeof produceCachedData !== 'boolean') {
throw new ERR_INVALID_ARG_TYPE('options.produceCachedData', 'boolean',
Expand Down Expand Up @@ -346,10 +349,10 @@ function compileFunction(code, params, options = {}) {
}
validateUint32(columnOffset, 'options.columnOffset');
validateUint32(lineOffset, 'options.lineOffset');
if (cachedData !== undefined && !isUint8Array(cachedData)) {
if (cachedData !== undefined && !isArrayBufferView(cachedData)) {
throw new ERR_INVALID_ARG_TYPE(
'options.cachedData',
'Uint8Array',
['Buffer', 'TypedArray', 'DataView'],
cachedData
);
}
Expand Down
14 changes: 7 additions & 7 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ namespace contextify {

using v8::Array;
using v8::ArrayBuffer;
using v8::ArrayBufferView;
using v8::Boolean;
using v8::Context;
using v8::EscapableHandleScope;
Expand Down Expand Up @@ -64,7 +65,6 @@ using v8::String;
using v8::Symbol;
using v8::TryCatch;
using v8::Uint32;
using v8::Uint8Array;
using v8::UnboundScript;
using v8::Value;
using v8::WeakCallbackInfo;
Expand Down Expand Up @@ -629,7 +629,7 @@ void ContextifyScript::New(const FunctionCallbackInfo<Value>& args) {

Local<Integer> line_offset;
Local<Integer> column_offset;
Local<Uint8Array> cached_data_buf;
Local<ArrayBufferView> cached_data_buf;
bool produce_cached_data = false;
Local<Context> parsing_context = context;

Expand All @@ -642,8 +642,8 @@ void ContextifyScript::New(const FunctionCallbackInfo<Value>& args) {
CHECK(args[3]->IsNumber());
column_offset = args[3].As<Integer>();
if (!args[4]->IsUndefined()) {
CHECK(args[4]->IsUint8Array());
cached_data_buf = args[4].As<Uint8Array>();
CHECK(args[4]->IsArrayBufferView());
cached_data_buf = args[4].As<ArrayBufferView>();
}
CHECK(args[5]->IsBoolean());
produce_cached_data = args[5]->IsTrue();
Expand Down Expand Up @@ -994,10 +994,10 @@ void ContextifyContext::CompileFunction(
Local<Integer> column_offset = args[3].As<Integer>();

// Argument 5: cached data (optional)
Local<Uint8Array> cached_data_buf;
Local<ArrayBufferView> cached_data_buf;
if (!args[4]->IsUndefined()) {
CHECK(args[4]->IsUint8Array());
cached_data_buf = args[4].As<Uint8Array>();
CHECK(args[4]->IsArrayBufferView());
cached_data_buf = args[4].As<ArrayBufferView>();
}

// Argument 6: produce cache data
Expand Down
8 changes: 5 additions & 3 deletions test/parallel/test-vm-basic.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,18 +178,20 @@ const vm = require('vm');
'filename': 'string',
'columnOffset': 'number',
'lineOffset': 'number',
'cachedData': 'Uint8Array',
'cachedData': 'Buffer, TypedArray, or DataView',
'produceCachedData': 'boolean',
};

for (const option in optionTypes) {
const typeErrorMessage = `The "options.${option}" property must be ` +
`${option === 'cachedData' ? 'one of' : 'of'} type`;
common.expectsError(() => {
vm.compileFunction('', undefined, { [option]: null });
}, {
type: TypeError,
code: 'ERR_INVALID_ARG_TYPE',
message: `The "options.${option}" property must be of type ` +
`${optionTypes[option]}. Received type object`
message: typeErrorMessage +
` ${optionTypes[option]}. Received type object`
});
}

Expand Down
16 changes: 9 additions & 7 deletions test/parallel/test-vm-cached-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,14 @@ function testProduceConsume() {

const data = produce(source);

// It should consume code cache
const script = new vm.Script(source, {
cachedData: data
});
assert(!script.cachedDataRejected);
assert.strictEqual(script.runInThisContext()(), 'original');
for (const cachedData of common.getArrayBufferViews(data)) {
// It should consume code cache
const script = new vm.Script(source, {
cachedData
});
assert(!script.cachedDataRejected);
assert.strictEqual(script.runInThisContext()(), 'original');
}
}
testProduceConsume();

Expand Down Expand Up @@ -91,5 +93,5 @@ common.expectsError(() => {
}, {
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: /must be one of type Buffer or Uint8Array/
message: /must be one of type Buffer, TypedArray, or DataView/
});

0 comments on commit 65fe999

Please sign in to comment.