From 0733f4ac8ded670499177925edd5fe956133408a Mon Sep 17 00:00:00 2001 From: Benjamin Chen Date: Tue, 18 Sep 2018 01:28:41 -0400 Subject: [PATCH 01/10] vm: update source, related basic & cached data tests for ArrayBuffer In vm module, accept ArrayBuffer in place of Uint8Array in the source code & related test code in "test-vm-basic.js" & "test-vm-cached-data.js", per the "vm" item in the checklist of the comment in #1826. Refs: https://github.com/nodejs/node/issues/1826#issuecomment-410487110 --- lib/vm.js | 15 +++++++++------ test/parallel/test-vm-basic.js | 8 +++++--- test/parallel/test-vm-cached-data.js | 2 +- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/lib/vm.js b/lib/vm.js index 1bb948fa550531..6e735bca4a76d9 100644 --- a/lib/vm.js +++ b/lib/vm.js @@ -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'); @@ -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', @@ -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 ); } diff --git a/test/parallel/test-vm-basic.js b/test/parallel/test-vm-basic.js index 54b7c45ff8043a..df0c7df1062c14 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..11fd36a380696e 100644 --- a/test/parallel/test-vm-cached-data.js +++ b/test/parallel/test-vm-cached-data.js @@ -91,5 +91,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/ }); From 802935255ffc9a7598403aa6a584056bed944692 Mon Sep 17 00:00:00 2001 From: Benjamin Chen Date: Wed, 26 Sep 2018 00:05:50 -0400 Subject: [PATCH 02/10] vm: update contextify source Use ArrayBufferView/IsArrayBufferView in places of Uint8Array/IsUint8Array, in the node_contextify.cc source code. Refs: https://github.com/nodejs/node/pull/22921#issuecomment-422350213 --- src/node_contextify.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 3fbc616fda0284..79943f1ad6a7a4 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -33,6 +33,7 @@ namespace contextify { using v8::Array; using v8::ArrayBuffer; +using v8::ArrayBufferView; using v8::Boolean; using v8::Context; using v8::EscapableHandleScope; @@ -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; @@ -629,7 +629,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; @@ -642,8 +642,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(); @@ -994,10 +994,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 From 2010cfd1a6c2d01c59b0840b5b3b066aa9d6e502 Mon Sep 17 00:00:00 2001 From: Beni von Cheni Date: Wed, 3 Oct 2018 11:04:41 -0400 Subject: [PATCH 03/10] doc: update vm.md for "cachedData" w. TypedArray|DataView MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update the "cachedData" description with support of TypedArray or DataView support in “new vm.Script” & “vm.compileFunction”. --- doc/api/vm.md | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/doc/api/vm.md b/doc/api/vm.md index c4b7e3f9b381ed..ae4daec7166eb6 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 From 08d177f87e55372c7112458f36ea37aab4a75533 Mon Sep 17 00:00:00 2001 From: Beni von Cheni Date: Sun, 21 Oct 2018 11:01:28 -0400 Subject: [PATCH 04/10] test: add array buffer view tests in test-vm-cached-data.js --- test/parallel/test-vm-cached-data.js | 32 +++++++++++++++------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/test/parallel/test-vm-cached-data.js b/test/parallel/test-vm-cached-data.js index 11fd36a380696e..ff6ecaa674ac25 100644 --- a/test/parallel/test-vm-cached-data.js +++ b/test/parallel/test-vm-cached-data.js @@ -41,12 +41,13 @@ 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 parsedData of common.getArrayBufferViews(data)) { + const script = new vm.Script(source, { + cachedData: parsedData + }); + assert(!script.cachedDataRejected); + assert.strictEqual(script.runInThisContext()(), 'original'); + } } testProduceConsume(); @@ -61,15 +62,16 @@ function testRejectInvalid() { const source = getSource('invalid'); const data = produce(source); - - // It should reject invalid code cache - const script = new vm.Script(getSource('invalid_1'), { - cachedData: data - }); - assert(script.cachedDataRejected); - assert.strictEqual(script.runInThisContext()(), 'invalid_1'); + + for (const parsedData of common.getArrayBufferViews(data)) { + // It should reject invalid code cache + const script = new vm.Script(getSource('invalid_1'), { + cachedData: parsedData + }); + assert(script.cachedDataRejected); + assert.strictEqual(script.runInThisContext()(), 'invalid_1'); + } } -testRejectInvalid(); function testRejectSlice() { const source = getSource('slice'); @@ -81,7 +83,7 @@ function testRejectSlice() { }); assert(script.cachedDataRejected); } -testRejectSlice(); +// testRejectSlice(); // It should throw on non-Buffer cachedData common.expectsError(() => { From bc53ec597ac0de32d488f71355c70ea862a45c27 Mon Sep 17 00:00:00 2001 From: Beni von Cheni Date: Sun, 21 Oct 2018 23:15:04 -0400 Subject: [PATCH 05/10] test: update tests in test-vm-cached-data.js --- test/parallel/test-vm-cached-data.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/parallel/test-vm-cached-data.js b/test/parallel/test-vm-cached-data.js index ff6ecaa674ac25..d0a7d99350a159 100644 --- a/test/parallel/test-vm-cached-data.js +++ b/test/parallel/test-vm-cached-data.js @@ -42,6 +42,7 @@ function testProduceConsume() { const data = produce(source); for (const parsedData of common.getArrayBufferViews(data)) { + // It should consume code cache const script = new vm.Script(source, { cachedData: parsedData }); @@ -62,16 +63,15 @@ function testRejectInvalid() { const source = getSource('invalid'); const data = produce(source); - - for (const parsedData of common.getArrayBufferViews(data)) { - // It should reject invalid code cache - const script = new vm.Script(getSource('invalid_1'), { - cachedData: parsedData - }); - assert(script.cachedDataRejected); - assert.strictEqual(script.runInThisContext()(), 'invalid_1'); - } + + // It should reject invalid code cache + const script = new vm.Script(getSource('invalid_1'), { + cachedData: data + }); + assert(script.cachedDataRejected); + assert.strictEqual(script.runInThisContext()(), 'invalid_1'); } +testRejectInvalid(); function testRejectSlice() { const source = getSource('slice'); @@ -83,7 +83,7 @@ function testRejectSlice() { }); assert(script.cachedDataRejected); } -// testRejectSlice(); +testRejectSlice(); // It should throw on non-Buffer cachedData common.expectsError(() => { From 18b86aa7cbd4d735bbcb213e39417cbbfc26c70b Mon Sep 17 00:00:00 2001 From: Beni von Cheni Date: Sun, 21 Oct 2018 23:27:14 -0400 Subject: [PATCH 06/10] test: update tests in test-vm-cached-data.js --- test/parallel/test-vm-cached-data.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-vm-cached-data.js b/test/parallel/test-vm-cached-data.js index d0a7d99350a159..347d5049a92631 100644 --- a/test/parallel/test-vm-cached-data.js +++ b/test/parallel/test-vm-cached-data.js @@ -42,7 +42,7 @@ function testProduceConsume() { const data = produce(source); for (const parsedData of common.getArrayBufferViews(data)) { - // It should consume code cache + // It should consume code cache const script = new vm.Script(source, { cachedData: parsedData }); From b86151e95c12a75f585a8411c3c5f9df9b0c7f6a Mon Sep 17 00:00:00 2001 From: Beni von Cheni Date: Tue, 23 Oct 2018 10:39:02 -0400 Subject: [PATCH 07/10] test: update "parsedData" namespace in test-vm-cached-data.js --- test/parallel/test-vm-cached-data.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/test/parallel/test-vm-cached-data.js b/test/parallel/test-vm-cached-data.js index 347d5049a92631..034b026d5ff556 100644 --- a/test/parallel/test-vm-cached-data.js +++ b/test/parallel/test-vm-cached-data.js @@ -41,11 +41,9 @@ function testProduceConsume() { const data = produce(source); - for (const parsedData of common.getArrayBufferViews(data)) { - // It should consume code cache - const script = new vm.Script(source, { - cachedData: parsedData - }); + 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'); } From 862c63dbda7885db120092cff421ced99cc39cc2 Mon Sep 17 00:00:00 2001 From: Beni von Cheni Date: Wed, 24 Oct 2018 10:13:03 -0400 Subject: [PATCH 08/10] test: update object code format in vm cached data test --- test/parallel/test-vm-cached-data.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-vm-cached-data.js b/test/parallel/test-vm-cached-data.js index 034b026d5ff556..1b14999cdbc2f7 100644 --- a/test/parallel/test-vm-cached-data.js +++ b/test/parallel/test-vm-cached-data.js @@ -43,7 +43,9 @@ function testProduceConsume() { for (const cachedData of common.getArrayBufferViews(data)) { // It should consume code cache - const script = new vm.Script(source, { cachedData }); + const script = new vm.Script(source, { + cachedData + }); assert(!script.cachedDataRejected); assert.strictEqual(script.runInThisContext()(), 'original'); } From fac59e7a6c872e17c9fa7946175a5aa71f0b5a68 Mon Sep 17 00:00:00 2001 From: Beni von Cheni Date: Wed, 24 Oct 2018 12:33:18 -0400 Subject: [PATCH 09/10] test: use "cachedData" object key-value format in vm cached data test --- test/parallel/test-vm-cached-data.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-vm-cached-data.js b/test/parallel/test-vm-cached-data.js index 1b14999cdbc2f7..68735ddd21b4cd 100644 --- a/test/parallel/test-vm-cached-data.js +++ b/test/parallel/test-vm-cached-data.js @@ -44,7 +44,7 @@ function testProduceConsume() { for (const cachedData of common.getArrayBufferViews(data)) { // It should consume code cache const script = new vm.Script(source, { - cachedData + cachedData: cachedData }); assert(!script.cachedDataRejected); assert.strictEqual(script.runInThisContext()(), 'original'); From 0f20acaafe12efabfef709ed33b7339c049f7542 Mon Sep 17 00:00:00 2001 From: Beni von Cheni Date: Sat, 27 Oct 2018 10:45:17 -0400 Subject: [PATCH 10/10] test: use short-hand object for "cachedData" in vm cached data test --- test/parallel/test-vm-cached-data.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-vm-cached-data.js b/test/parallel/test-vm-cached-data.js index 68735ddd21b4cd..1b14999cdbc2f7 100644 --- a/test/parallel/test-vm-cached-data.js +++ b/test/parallel/test-vm-cached-data.js @@ -44,7 +44,7 @@ function testProduceConsume() { for (const cachedData of common.getArrayBufferViews(data)) { // It should consume code cache const script = new vm.Script(source, { - cachedData: cachedData + cachedData }); assert(!script.cachedDataRejected); assert.strictEqual(script.runInThisContext()(), 'original');