FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

vm: unify host-defined option generation in vm.compileFunction · nodejs/node@9f7899e · GitHub

/ node Public

Commit 9f7899e

Browse files
joyeecheung
authored and
richardlau
committed
vm: unify host-defined option generation in vm.compileFunction
Set a default host-defined option for vm.compileFunction so that it's consistent with vm.Script. PR-URL: #50137 Backport-PR-URL: #51004 Refs: #35375 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent 6291c10 commit 9f7899e

4 files changed

Lines changed: 46 additions & 24 deletions

File tree

‎lib/internal/vm.js‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,16 @@
22

33
const {
44
ArrayPrototypeForEach,
5+
Symbol,
56
} = primordials;
67

78
const {
89
compileFunction,
910
isContext: _isContext,
1011
} = internalBinding('contextify');
12+
const {
13+
default_host_defined_options,
14+
} = internalBinding('symbols');
1115
const {
1216
validateArray,
1317
validateBoolean,
@@ -28,12 +32,27 @@ function isContext(object) {
2832
return _isContext(object);
2933
}
3034

35+
function getHostDefinedOptionId(importModuleDynamically, filename) {
36+
if (importModuleDynamically !== undefined) {
37+
// Check that it's either undefined or a function before we pass
38+
// it into the native constructor.
39+
validateFunction(importModuleDynamically,
40+
'options.importModuleDynamically');
41+
}
42+
if (importModuleDynamically === undefined) {
43+
// We need a default host defined options that are the same for all
44+
// scripts not needing custom module callbacks so that the isolate
45+
// compilation cache can be hit.
46+
return default_host_defined_options;
47+
}
48+
return Symbol(filename);
49+
}
50+
3151
function internalCompileFunction(code, params, options) {
3252
validateString(code, 'code');
3353
if (params !== undefined) {
3454
validateStringArray(params, 'params');
3555
}
36-
3756
const {
3857
filename = '',
3958
columnOffset = 0,
@@ -70,6 +89,8 @@ function internalCompileFunction(code, params, options) {
7089
validateObject(extension, name, { __proto__: null, nullable: true });
7190
});
7291

92+
const hostDefinedOptionId =
93+
getHostDefinedOptionId(importModuleDynamically, filename);
7394
const result = compileFunction(
7495
code,
7596
filename,
@@ -80,6 +101,7 @@ function internalCompileFunction(code, params, options) {
80101
parsingContext,
81102
contextExtensions,
82103
params,
104+
hostDefinedOptionId,
83105
);
84106

85107
if (produceCachedData) {
@@ -111,6 +133,7 @@ function internalCompileFunction(code, params, options) {
111133
}
112134

113135
module.exports = {
136+
getHostDefinedOptionId,
114137
internalCompileFunction,
115138
isContext,
116139
};

‎lib/vm.js‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ const {
4242
const {
4343
validateBoolean,
4444
validateBuffer,
45-
validateFunction,
4645
validateInt32,
4746
validateObject,
4847
validateOneOf,
@@ -55,6 +54,7 @@ const {
5554
kVmBreakFirstLineSymbol,
5655
} = require('internal/util');
5756
const {
57+
getHostDefinedOptionId,
5858
internalCompileFunction,
5959
isContext,
6060
} = require('internal/vm');
@@ -87,12 +87,8 @@ class Script extends ContextifyScript {
8787
}
8888
validateBoolean(produceCachedData, 'options.produceCachedData');
8989

90-
if (importModuleDynamically !== undefined) {
91-
// Check that it's either undefined or a function before we pass
92-
// it into the native constructor.
93-
validateFunction(importModuleDynamically,
94-
'options.importModuleDynamically');
95-
}
90+
const hostDefinedOptionId =
91+
getHostDefinedOptionId(importModuleDynamically, filename);
9692
// Calling `ReThrow()` on a native TryCatch does not generate a new
9793
// abort-on-uncaught-exception check. A dummy try/catch in JS land
9894
// protects against that.
@@ -104,7 +100,7 @@ class Script extends ContextifyScript {
104100
cachedData,
105101
produceCachedData,
106102
parsingContext,
107-
importModuleDynamically !== undefined);
103+
hostDefinedOptionId);
108104
} catch (e) {
109105
throw e; /* node-do-not-add-exception-line */
110106
}

‎src/node_contextify.cc‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -787,11 +787,11 @@ void ContextifyScript::New(const FunctionCallbackInfo<Value>& args) {
787787
bool produce_cached_data = false;
788788
Local<Context> parsing_context = context;
789789

790-
bool needs_custom_host_defined_options = false;
790+
Local<Symbol> id_symbol;
791791
if (argc > 2) {
792792
// new ContextifyScript(code, filename, lineOffset, columnOffset,
793793
// cachedData, produceCachedData, parsingContext,
794-
// needsCustomHostDefinedOptions)
794+
// hostDefinedOptionId)
795795
CHECK_EQ(argc, 8);
796796
CHECK(args[2]->IsNumber());
797797
line_offset = args[2].As<Int32>()->Value();
@@ -811,9 +811,8 @@ void ContextifyScript::New(const FunctionCallbackInfo<Value>& args) {
811811
CHECK_NOT_NULL(sandbox);
812812
parsing_context = sandbox->context();
813813
}
814-
if (args[7]->IsTrue()) {
815-
needs_custom_host_defined_options = true;
816-
}
814+
CHECK(args[7]->IsSymbol());
815+
id_symbol = args[7].As<Symbol>();
817816
}
818817

819818
ContextifyScript* contextify_script =
@@ -837,12 +836,6 @@ void ContextifyScript::New(const FunctionCallbackInfo<Value>& args) {
837836

838837
Local<PrimitiveArray> host_defined_options =
839838
PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength);
840-
// We need a default host defined options that's the same for all scripts
841-
// not needing custom module callbacks for so that the isolate compilation
842-
// cache can be hit.
843-
Local<Symbol> id_symbol = needs_custom_host_defined_options
844-
? Symbol::New(isolate, filename)
845-
: env->default_host_defined_options();
846839
host_defined_options->Set(
847840
isolate, loader::HostDefinedOptions::kID, id_symbol);
848841

@@ -1201,6 +1194,10 @@ void ContextifyContext::CompileFunction(
12011194
params_buf = args[8].As<Array>();
12021195
}
12031196

1197+
// Argument 10: host-defined option symbol
1198+
CHECK(args[9]->IsSymbol());
1199+
Local<Symbol> id_symbol = args[9].As<Symbol>();
1200+
12041201
// Read cache from cached data buffer
12051202
ScriptCompiler::CachedData* cached_data = nullptr;
12061203
if (!cached_data_buf.IsEmpty()) {
@@ -1212,7 +1209,6 @@ void ContextifyContext::CompileFunction(
12121209
// Set host_defined_options
12131210
Local<PrimitiveArray> host_defined_options =
12141211
PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength);
1215-
Local<Symbol> id_symbol = Symbol::New(isolate, filename);
12161212
host_defined_options->Set(
12171213
isolate, loader::HostDefinedOptions::kID, id_symbol);
12181214

‎test/parallel/test-vm-no-dynamic-import-callback.js‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
22

3-
require('../common');
4-
const { Script } = require('vm');
3+
const common = require('../common');
4+
const { Script, compileFunction } = require('vm');
55
const assert = require('assert');
66

77
assert.rejects(async () => {
@@ -10,4 +10,11 @@ assert.rejects(async () => {
1010
await imported;
1111
}, {
1212
code: 'ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING'
13-
});
13+
}).then(common.mustCall());
14+
15+
assert.rejects(async () => {
16+
const imported = compileFunction('return import("fs")')();
17+
await imported;
18+
}, {
19+
code: 'ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING'
20+
}).then(common.mustCall());

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL