From d9d31e8d513b870b605d3149228c1b0edb8701bf Mon Sep 17 00:00:00 2001 From: Benjamin Chen Date: Tue, 18 Sep 2018 01:28:41 -0400 Subject: [PATCH] vm: allow `cachedData` to also be TypedArray|DataView PR-URL: https://github.com/nodejs/node/pull/22921 Refs: https://github.com/nodejs/node/issues/1826 Refs: https://github.com/nodejs/node/pull/22921#issuecomment-422350213 Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Denys Otrishko Reviewed-By: Refael Ackermann --- doc/api/vm.md | 13 +++++++------ lib/vm.js | 15 +++++++++------ src/node_contextify.cc | 14 +++++++------- test/parallel/test-vm-basic.js | 8 +++++--- test/parallel/test-vm-cached-data.js | 16 +++++++++------- 5 files changed, 37 insertions(+), 29 deletions(-) diff --git a/doc/api/vm.md b/doc/api/vm.md index e8c4d1a6cbf2f2..ca9124705c61ae 100644 --- a/doc/api/vm.md +++ b/doc/api/vm.md @@ -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 @@ -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 diff --git a/lib/vm.js b/lib/vm.js index 02b3f0bce423bc..1456edf7eeddeb 100644 --- a/lib/vm.js +++ b/lib/vm.js @@ -33,7 +33,7 @@ const { ERR_OUT_OF_RANGE, ERR_VM_MODULE_NOT_MODULE, } = require('internal/errors').codes; -const { isModuleNamespaceObject, isUint8Array } = require('util').types; +const { isModuleNamespaceObject, isArrayBufferView } = require('util').types; const { validateUint32 } = require('internal/validators'); const kParsingContext = Symbol('script parsing context'); @@ -64,9 +64,12 @@ class Script extends ContextifyScript { } 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 (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', @@ -356,10 +359,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 ); } diff --git a/src/node_contextify.cc b/src/node_contextify.cc index a1903d9bd9b7e0..603dbcbf26c7db 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -32,6 +32,7 @@ namespace contextify { using v8::Array; using v8::ArrayBuffer; +using v8::ArrayBufferView; using v8::Boolean; using v8::Context; using v8::EscapableHandleScope; @@ -63,7 +64,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; @@ -628,7 +628,7 @@ void ContextifyScript::New(const FunctionCallbackInfo& args) { Local line_offset; Local column_offset; - Local cached_data_buf; + Local cached_data_buf; bool produce_cached_data = false; Local parsing_context = context; @@ -641,8 +641,8 @@ void ContextifyScript::New(const FunctionCallbackInfo& args) { CHECK(args[3]->IsNumber()); column_offset = args[3].As(); if (!args[4]->IsUndefined()) { - CHECK(args[4]->IsUint8Array()); - cached_data_buf = args[4].As(); + CHECK(args[4]->IsArrayBufferView()); + cached_data_buf = args[4].As(); } CHECK(args[5]->IsBoolean()); produce_cached_data = args[5]->IsTrue(); @@ -993,10 +993,10 @@ void ContextifyContext::CompileFunction( Local column_offset = args[3].As(); // Argument 5: cached data (optional) - Local cached_data_buf; + Local cached_data_buf; if (!args[4]->IsUndefined()) { - CHECK(args[4]->IsUint8Array()); - cached_data_buf = args[4].As(); + CHECK(args[4]->IsArrayBufferView()); + cached_data_buf = args[4].As(); } // Argument 6: produce cache data diff --git a/test/parallel/test-vm-basic.js b/test/parallel/test-vm-basic.js index 11d64f54ab83e6..29e2a8b5251934 100644 --- a/test/parallel/test-vm-basic.js +++ b/test/parallel/test-vm-basic.js @@ -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` }); } diff --git a/test/parallel/test-vm-cached-data.js b/test/parallel/test-vm-cached-data.js index e3947d1ae6c166..1b14999cdbc2f7 100644 --- a/test/parallel/test-vm-cached-data.js +++ b/test/parallel/test-vm-cached-data.js @@ -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(); @@ -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/ });