Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit fe66e9d

Browse filesBrowse files
joyeecheungrichardlau
joyeecheung
authored and
richardlau
committed
vm: reject in importModuleDynamically without --experimental-vm-modules
Users cannot access any API that can be used to return a module or module namespace in this callback without --experimental-vm-modules anyway, so this would eventually lead to a rejection. This patch rejects in this case with our own error message and use a constant host-defined option for the rejection, so that scripts with the same source can still be compiled using the compilation cache if no `import()` is actually called in the 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 052e095 commit fe66e9d
Copy full SHA for fe66e9d

File tree

Expand file treeCollapse file tree

7 files changed

+81
-7
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

7 files changed

+81
-7
lines changed
Open diff view settings
Collapse file

‎doc/api/errors.md‎

Copy file name to clipboardExpand all lines: doc/api/errors.md
+6Lines changed: 6 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -2985,6 +2985,12 @@ An attempt was made to use something that was already closed.
29852985
While using the Performance Timing API (`perf_hooks`), no valid performance
29862986
entry types are found.
29872987

2988+
<a id="ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG"></a>
2989+
2990+
### `ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`
2991+
2992+
A dynamic import callback was invoked without `--experimental-vm-modules`.
2993+
29882994
<a id="ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING"></a>
29892995

29902996
### `ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`
Collapse file

‎doc/api/vm.md‎

Copy file name to clipboardExpand all lines: doc/api/vm.md
+19-5Lines changed: 19 additions & 5 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,9 @@ changes:
9898
when `import()` is called. If this option is not specified, calls to
9999
`import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][].
100100
This option is part of the experimental modules API. We do not recommend
101-
using it in a production environment.
101+
using it in a production environment. If `--experimental-vm-modules` isn't
102+
set, this callback will be ignored and calls to `import()` will reject with
103+
[`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][].
102104
* `specifier` {string} specifier passed to `import()`
103105
* `script` {vm.Script}
104106
* `importAttributes` {Object} The `"assert"` value passed to the
@@ -765,6 +767,9 @@ changes:
765767
* `importModuleDynamically` {Function} Called during evaluation of this module
766768
when `import()` is called. If this option is not specified, calls to
767769
`import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][].
770+
If `--experimental-vm-modules` isn't set, this callback will be ignored
771+
and calls to `import()` will reject with
772+
[`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][].
768773
* `specifier` {string} specifier passed to `import()`
769774
* `module` {vm.Module}
770775
* `importAttributes` {Object} The `"assert"` value passed to the
@@ -1022,7 +1027,9 @@ changes:
10221027
when `import()` is called. If this option is not specified, calls to
10231028
`import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][].
10241029
This option is part of the experimental modules API, and should not be
1025-
considered stable.
1030+
considered stable. If `--experimental-vm-modules` isn't
1031+
set, this callback will be ignored and calls to `import()` will reject with
1032+
[`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][].
10261033
* `specifier` {string} specifier passed to `import()`
10271034
* `function` {Function}
10281035
* `importAttributes` {Object} The `"assert"` value passed to the
@@ -1246,7 +1253,9 @@ changes:
12461253
when `import()` is called. If this option is not specified, calls to
12471254
`import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][].
12481255
This option is part of the experimental modules API. We do not recommend
1249-
using it in a production environment.
1256+
using it in a production environment. If `--experimental-vm-modules` isn't
1257+
set, this callback will be ignored and calls to `import()` will reject with
1258+
[`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][].
12501259
* `specifier` {string} specifier passed to `import()`
12511260
* `script` {vm.Script}
12521261
* `importAttributes` {Object} The `"assert"` value passed to the
@@ -1345,7 +1354,9 @@ changes:
13451354
when `import()` is called. If this option is not specified, calls to
13461355
`import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][].
13471356
This option is part of the experimental modules API. We do not recommend
1348-
using it in a production environment.
1357+
using it in a production environment. If `--experimental-vm-modules` isn't
1358+
set, this callback will be ignored and calls to `import()` will reject with
1359+
[`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][].
13491360
* `specifier` {string} specifier passed to `import()`
13501361
* `script` {vm.Script}
13511362
* `importAttributes` {Object} The `"assert"` value passed to the
@@ -1425,7 +1436,9 @@ changes:
14251436
when `import()` is called. If this option is not specified, calls to
14261437
`import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][].
14271438
This option is part of the experimental modules API. We do not recommend
1428-
using it in a production environment.
1439+
using it in a production environment. If `--experimental-vm-modules` isn't
1440+
set, this callback will be ignored and calls to `import()` will reject with
1441+
[`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][].
14291442
* `specifier` {string} specifier passed to `import()`
14301443
* `script` {vm.Script}
14311444
* `importAttributes` {Object} The `"assert"` value passed to the
@@ -1589,6 +1602,7 @@ are not controllable through the timeout either.
15891602
[Source Text Module Record]: https://tc39.es/ecma262/#sec-source-text-module-records
15901603
[Synthetic Module Record]: https://heycam.github.io/webidl/#synthetic-module-records
15911604
[V8 Embedder's Guide]: https://v8.dev/docs/embed#contexts
1605+
[`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`]: errors.md#err_vm_dynamic_import_callback_missing_flag
15921606
[`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`]: errors.md#err_vm_dynamic_import_callback_missing
15931607
[`ERR_VM_MODULE_STATUS`]: errors.md#err_vm_module_status
15941608
[`Error`]: errors.md#class-error
Collapse file

‎lib/internal/errors.js‎

Copy file name to clipboardExpand all lines: lib/internal/errors.js
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1716,6 +1716,9 @@ E('ERR_VALID_PERFORMANCE_ENTRY_TYPE',
17161716
'At least one valid performance entry type is required', Error);
17171717
E('ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING',
17181718
'A dynamic import callback was not specified.', TypeError);
1719+
E('ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG',
1720+
'A dynamic import callback was invoked without --experimental-vm-modules',
1721+
TypeError);
17191722
E('ERR_VM_MODULE_ALREADY_LINKED', 'Module has already been linked', Error);
17201723
E('ERR_VM_MODULE_CANNOT_CREATE_CACHED_DATA',
17211724
'Cached data cannot be created for a module which has been evaluated', Error);
Collapse file

‎lib/internal/modules/esm/utils.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/utils.js
+7-1Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@ const {
1414
} = internalBinding('util');
1515
const {
1616
default_host_defined_options,
17+
vm_dynamic_import_missing_flag,
1718
} = internalBinding('symbols');
1819

1920
const {
21+
ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG,
2022
ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING,
2123
ERR_INVALID_ARG_VALUE,
2224
} = require('internal/errors').codes;
@@ -132,7 +134,8 @@ const moduleRegistries = new SafeWeakMap();
132134
*/
133135
function registerModule(referrer, registry) {
134136
const idSymbol = referrer[host_defined_option_symbol];
135-
if (idSymbol === default_host_defined_options) {
137+
if (idSymbol === default_host_defined_options ||
138+
idSymbol === vm_dynamic_import_missing_flag) {
136139
// The referrer is compiled without custom callbacks, so there is
137140
// no registry to hold on to. We'll throw
138141
// ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING when a callback is
@@ -173,6 +176,9 @@ async function importModuleDynamicallyCallback(symbol, specifier, attributes) {
173176
return importModuleDynamically(specifier, callbackReferrer, attributes);
174177
}
175178
}
179+
if (symbol === vm_dynamic_import_missing_flag) {
180+
throw new ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG();
181+
}
176182
throw new ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING();
177183
}
178184

Collapse file

‎lib/internal/vm.js‎

Copy file name to clipboardExpand all lines: lib/internal/vm.js
+16Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,18 @@ const {
1515
} = ContextifyScript.prototype;
1616
const {
1717
default_host_defined_options,
18+
vm_dynamic_import_missing_flag,
1819
} = internalBinding('symbols');
1920
const {
2021
validateFunction,
2122
validateObject,
2223
} = require('internal/validators');
2324

25+
const {
26+
getOptionValue,
27+
} = require('internal/options');
28+
29+
2430
function isContext(object) {
2531
validateObject(object, 'object', { __proto__: null, allowArray: true });
2632

@@ -40,6 +46,16 @@ function getHostDefinedOptionId(importModuleDynamically, filename) {
4046
// compilation cache can be hit.
4147
return default_host_defined_options;
4248
}
49+
// We should've thrown here immediately when we introduced
50+
// --experimental-vm-modules and importModuleDynamically, but since
51+
// users are already using this callback to throw a similar error,
52+
// we also defer the error to the time when an actual import() is called
53+
// to avoid breaking them. To ensure that the isolate compilation
54+
// cache can still be hit, use a constant sentinel symbol here.
55+
if (!getOptionValue('--experimental-vm-modules')) {
56+
return vm_dynamic_import_missing_flag;
57+
}
58+
4359
return Symbol(filename);
4460
}
4561

Collapse file

‎src/env_properties.h‎

Copy file name to clipboardExpand all lines: src/env_properties.h
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@
4444
V(owner_symbol, "owner_symbol") \
4545
V(onpskexchange_symbol, "onpskexchange") \
4646
V(resource_symbol, "resource_symbol") \
47-
V(trigger_async_id_symbol, "trigger_async_id_symbol")
47+
V(trigger_async_id_symbol, "trigger_async_id_symbol") \
48+
V(vm_dynamic_import_missing_flag, "vm_dynamic_import_missing_flag")
4849

4950
// Strings are per-isolate primitives but Environment proxies them
5051
// for the sake of convenience. Strings should be ASCII-only.
Collapse file

‎test/parallel/test-vm-dynamic-import-callback-missing-flag.js‎

Copy file name to clipboard
+28Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const { Script, compileFunction } = require('vm');
5+
const assert = require('assert');
6+
7+
assert(
8+
!process.execArgv.includes('--experimental-vm-modules'),
9+
'This test must be run without --experimental-vm-modules');
10+
11+
assert.rejects(async () => {
12+
const script = new Script('import("fs")', {
13+
importModuleDynamically: common.mustNotCall(),
14+
});
15+
const imported = script.runInThisContext();
16+
await imported;
17+
}, {
18+
code: 'ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG'
19+
}).then(common.mustCall());
20+
21+
assert.rejects(async () => {
22+
const imported = compileFunction('return import("fs")', [], {
23+
importModuleDynamically: common.mustNotCall(),
24+
})();
25+
await imported;
26+
}, {
27+
code: 'ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG'
28+
}).then(common.mustCall());

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.