Skip to content

Commit 4791219

Browse files
sjungwon03sjungwon03
authored
module: centralize builtin exposure policies
Keep scheme-only and option-gated builtin exposure rules in the JavaScript loader. Leave option registration and code-cache categorization with their native owners. Assisted-by: Codex Signed-off-by: sjungwon03 <[email protected]> PR-URL: #66292 Refs: #65418 Refs: #65964 Refs: #65920 Refs: #65840 Reviewed-By: Daeyeon Jeong <[email protected]>
1 parent cfb6aa1 commit 4791219

4 files changed

Lines changed: 228 additions & 99 deletions

File tree

‎lib/internal/bootstrap/realm.js‎

Lines changed: 47 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -120,34 +120,6 @@ const legacyWrapperList = new SafeSet([
120120
'util',
121121
]);
122122

123-
// The code below assumes that the two lists must not contain any modules
124-
// beginning with "internal/".
125-
// Modules that can only be imported via the node: scheme.
126-
const schemelessBlockList = new SafeSet([
127-
'bench',
128-
'bench/reporters',
129-
'dtls',
130-
'ffi',
131-
'sea',
132-
'sqlite',
133-
'quic',
134-
'test',
135-
'test/reporters',
136-
'vfs',
137-
]);
138-
// Modules that will only be enabled at run time.
139-
const experimentalModuleList = new SafeSet([
140-
'bench',
141-
'bench/reporters',
142-
'dtls',
143-
'ffi',
144-
'quic',
145-
'sqlite',
146-
'stream/iter',
147-
'vfs',
148-
'zlib/iter',
149-
]);
150-
151123
// Set up process.binding() and process._linkedBinding().
152124
{
153125
const bindingObj = { __proto__: null };
@@ -220,10 +192,41 @@ const getOwn = (target, property, receiver) => {
220192
undefined;
221193
};
222194

195+
// Public builtin exposure policies. Each entry is [id, schemeOnly, option].
196+
// A null option means that the builtin is not gated by a runtime option.
197+
// Do not include internal modules. Runtime option definitions and code-cache
198+
// categories are owned by their respective native subsystems.
199+
const builtinModulePolicies = [
200+
['bench', true, '--experimental-bench'],
201+
['bench/reporters', true, '--experimental-bench'],
202+
['dtls', true, '--experimental-dtls'],
203+
['ffi', true, '--experimental-ffi'],
204+
['sea', true, null],
205+
['sqlite', true, '--experimental-sqlite'],
206+
['quic', true, '--experimental-quic'],
207+
['stream/iter', false, '--experimental-stream-iter'],
208+
['test', true, null],
209+
['test/reporters', true, null],
210+
['vfs', true, '--experimental-vfs'],
211+
['zlib/iter', false, '--experimental-stream-iter'],
212+
];
213+
214+
const schemelessBlockList = new SafeSet();
215+
const optionGatedBuiltinOptions = new SafeMap();
216+
for (let i = 0; i < builtinModulePolicies.length; i++) {
217+
const { 0: id, 1: schemeOnly, 2: option } = builtinModulePolicies[i];
218+
if (schemeOnly) {
219+
schemelessBlockList.add(id);
220+
}
221+
if (option !== null) {
222+
optionGatedBuiltinOptions.set(id, option);
223+
}
224+
}
225+
223226
const publicBuiltinIds = builtinIds
224227
.filter((id) =>
225228
!StringPrototypeStartsWith(id, 'internal/') &&
226-
!experimentalModuleList.has(id),
229+
!optionGatedBuiltinOptions.has(id),
227230
);
228231
// Do not expose the loaders to user land even with --expose-internals.
229232
const internalBuiltinIds = builtinIds
@@ -284,6 +287,21 @@ class BuiltinModule {
284287
}
285288
}
286289

290+
// Called after runtime options have been initialized, before user modules.
291+
static allowOptionGatedBuiltins(getOptionValue) {
292+
for (const { 0: id, 1: option } of optionGatedBuiltinOptions) {
293+
if (getOptionValue(option)) {
294+
BuiltinModule.allowRequireByUsers(id);
295+
}
296+
}
297+
}
298+
299+
// Return a copy so internal tests cannot mutate the loader's policy.
300+
static getBuiltinModulePolicies() {
301+
return ArrayPrototypeMap(builtinModulePolicies,
302+
(policy) => ArrayPrototypeSlice(policy));
303+
}
304+
287305
static setRealmAllowRequireByUsers(ids) {
288306
canBeRequiredByUsersList =
289307
new SafeSet(ArrayPrototypeFilter(ids, (id) => ArrayPrototypeIncludes(publicBuiltinIds, id)));

‎lib/internal/process/pre_execution.js‎

Lines changed: 3 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -118,13 +118,7 @@ function prepareExecution(options) {
118118
setupNetworkInspection();
119119
setupNavigator();
120120
setupWarningHandler();
121-
setupBench();
122-
setupFFI();
123-
setupSQLite();
124-
setupStreamIter();
125-
setupDTLS();
126-
setupVfs();
127-
setupQuic();
121+
setupOptionGatedBuiltins();
128122
setupWebStorage();
129123
removeWebWorkersIfDisabled();
130124
setupEventsource();
@@ -491,32 +485,9 @@ function setupNavigator() {
491485
defineReplaceableLazyAttribute(globalThis, 'internal/navigator', ['navigator'], false);
492486
}
493487

494-
function setupBench() {
495-
if (!getOptionValue('--experimental-bench')) {
496-
return;
497-
}
498-
499-
const { BuiltinModule } = require('internal/bootstrap/realm');
500-
BuiltinModule.allowRequireByUsers('bench');
501-
BuiltinModule.allowRequireByUsers('bench/reporters');
502-
}
503-
504-
function setupFFI() {
505-
if (!getOptionValue('--experimental-ffi')) {
506-
return;
507-
}
508-
509-
const { BuiltinModule } = require('internal/bootstrap/realm');
510-
BuiltinModule.allowRequireByUsers('ffi');
511-
}
512-
513-
function setupSQLite() {
514-
if (getOptionValue('--no-experimental-sqlite')) {
515-
return;
516-
}
517-
488+
function setupOptionGatedBuiltins() {
518489
const { BuiltinModule } = require('internal/bootstrap/realm');
519-
BuiltinModule.allowRequireByUsers('sqlite');
490+
BuiltinModule.allowOptionGatedBuiltins(getOptionValue);
520491
}
521492

522493
function initializeConfigFileSupport() {
@@ -525,43 +496,6 @@ function initializeConfigFileSupport() {
525496
}
526497
}
527498

528-
function setupStreamIter() {
529-
if (!getOptionValue('--experimental-stream-iter')) {
530-
return;
531-
}
532-
533-
const { BuiltinModule } = require('internal/bootstrap/realm');
534-
BuiltinModule.allowRequireByUsers('stream/iter');
535-
BuiltinModule.allowRequireByUsers('zlib/iter');
536-
}
537-
538-
function setupDTLS() {
539-
if (!getOptionValue('--experimental-dtls')) {
540-
return;
541-
}
542-
543-
const { BuiltinModule } = require('internal/bootstrap/realm');
544-
BuiltinModule.allowRequireByUsers('dtls');
545-
}
546-
547-
function setupQuic() {
548-
if (!getOptionValue('--experimental-quic')) {
549-
return;
550-
}
551-
552-
const { BuiltinModule } = require('internal/bootstrap/realm');
553-
BuiltinModule.allowRequireByUsers('quic');
554-
}
555-
556-
function setupVfs() {
557-
if (!getOptionValue('--experimental-vfs')) {
558-
return;
559-
}
560-
561-
const { BuiltinModule } = require('internal/bootstrap/realm');
562-
BuiltinModule.allowRequireByUsers('vfs');
563-
}
564-
565499
function setupWebStorage() {
566500
if (getEmbedderOptions().noBrowserGlobals ||
567501
!getOptionValue('--experimental-webstorage')) {

‎lib/internal/test/binding.js‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,10 @@ if (module.isPreloading) {
3030
globalThis.primordials = primordials;
3131
}
3232

33-
module.exports = { internalBinding: filteredInternalBinding, primordials };
33+
module.exports = {
34+
internalBinding: filteredInternalBinding,
35+
primordials,
36+
getBuiltinModulePolicies() {
37+
return require('internal/bootstrap/realm').BuiltinModule.getBuiltinModulePolicies();
38+
},
39+
};

‎test/parallel/test-module-builtin-policies.js‎

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
// Flags: --expose-internals
2+
'use strict';
3+
4+
// Run before loading common, whose async-hooks checks need --expose-internals.
5+
// Child processes and Workers exercise only the public loaders.
6+
if (process.argv[2] === 'child') {
7+
const policies = JSON.parse(process.argv[3]);
8+
const enabled = process.argv[4] === 'true';
9+
Promise.all(policies.map((policy) => checkPolicy(policy, enabled)))
10+
.catch((err) => {
11+
console.error(err);
12+
process.exitCode = 1;
13+
});
14+
return;
15+
}
16+
17+
const common = require('../common');
18+
const assert = require('assert');
19+
const { isBuiltin } = require('module');
20+
const { Worker } = require('worker_threads');
21+
const { spawnSyncAndAssert } = require('../common/child_process');
22+
const {
23+
internalBinding,
24+
getBuiltinModulePolicies,
25+
} = require('internal/test/binding');
26+
const { getCLIOptionsInfo } = require('internal/options');
27+
28+
const policies = getBuiltinModulePolicies();
29+
const { builtinIds } = internalBinding('builtins');
30+
const { options } = getCLIOptionsInfo();
31+
const moduleAvailability = new Map([
32+
['dtls', common.hasDtls],
33+
['ffi', common.hasFFI],
34+
['quic', common.hasQuic],
35+
['sqlite', common.hasSQLite],
36+
]);
37+
38+
// Validate every declared policy before testing its behavior.
39+
{
40+
const seenIds = new Set();
41+
for (const policy of policies) {
42+
assert(Array.isArray(policy));
43+
assert.strictEqual(policy.length, 3);
44+
const [id, schemeOnly, option] = policy;
45+
assert.strictEqual(typeof id, 'string');
46+
assert(!id.startsWith('internal/'), id);
47+
assert(builtinIds.includes(id), `Unknown builtin: ${id}`);
48+
assert(!seenIds.has(id), `Duplicate policy: ${id}`);
49+
seenIds.add(id);
50+
assert.strictEqual(typeof schemeOnly, 'boolean', id);
51+
52+
if (option === null) continue;
53+
assert.match(option, /^--experimental-[a-z-]+$/);
54+
55+
// FFI has no CLI option in builds without FFI support.
56+
const missingFFIOption = id === 'ffi' && !common.hasFFI &&
57+
option === '--experimental-ffi';
58+
assert(options.has(option) || missingFFIOption,
59+
`Unknown builtin option: ${option}`);
60+
}
61+
}
62+
63+
// Callers must not be able to change the loader's policy through this API.
64+
{
65+
const copy = getBuiltinModulePolicies();
66+
copy[0][0] = 'changed';
67+
copy.pop();
68+
assert.deepStrictEqual(getBuiltinModulePolicies(), policies);
69+
}
70+
71+
// Test defaults, command-line flags, NODE_OPTIONS, and their precedence in
72+
// fresh processes.
73+
for (const policy of policies) {
74+
const [id, , option] = policy;
75+
if (moduleAvailability.get(id) === false) continue;
76+
77+
runPolicyTest(policy, [], option === null || options.get(option).defaultIsTrue);
78+
if (option !== null) {
79+
const disabledOption = option.replace('--', '--no-');
80+
runPolicyTest(policy, [option], true);
81+
runPolicyTest(policy, [disabledOption], false);
82+
83+
if (!process.config.variables.node_without_node_options) {
84+
runPolicyTest(policy, [], true, option);
85+
runPolicyTest(policy, [], false, disabledOption);
86+
// Command-line options take precedence over NODE_OPTIONS.
87+
runPolicyTest(policy, [option], true, disabledOption);
88+
runPolicyTest(policy, [disabledOption], false, option);
89+
}
90+
}
91+
}
92+
93+
// Test option-gated builtins in Workers without changing the parent state.
94+
{
95+
const policiesByOption = new Map();
96+
for (const policy of policies) {
97+
const [id, , option] = policy;
98+
if (option === null || moduleAvailability.get(id) === false) continue;
99+
100+
const optionPolicies = policiesByOption.get(option);
101+
if (optionPolicies === undefined) {
102+
policiesByOption.set(option, [policy]);
103+
} else {
104+
optionPolicies.push(policy);
105+
}
106+
}
107+
108+
// Test each option once, together with all builtins it controls.
109+
for (const [option, optionPolicies] of policiesByOption) {
110+
const parentState = optionPolicies.map(([id]) => [
111+
id,
112+
isBuiltin(`node:${id}`),
113+
]);
114+
const disabledOption = option.replace('--', '--no-');
115+
116+
for (const enabled of [true, false]) {
117+
const worker = new Worker(__filename, {
118+
argv: ['child', JSON.stringify(optionPolicies), String(enabled)],
119+
execArgv: [enabled ? option : disabledOption],
120+
env: { ...process.env, NODE_OPTIONS: '' },
121+
});
122+
123+
worker.on('exit', common.mustCall((code) => {
124+
assert.strictEqual(code, 0);
125+
for (const [id, parentEnabled] of parentState) {
126+
assert.strictEqual(isBuiltin(`node:${id}`), parentEnabled, id);
127+
}
128+
}));
129+
}
130+
}
131+
}
132+
133+
// Run in a fresh process so each case initializes the loader with its flags.
134+
function runPolicyTest(policy, flags, enabled, nodeOptions = '') {
135+
spawnSyncAndAssert(process.execPath, [
136+
...flags,
137+
__filename,
138+
'child',
139+
JSON.stringify([policy]),
140+
String(enabled),
141+
], { env: { ...process.env, NODE_OPTIONS: nodeOptions } }, { status: 0 });
142+
}
143+
144+
async function checkPolicy([id, schemeOnly], enabled) {
145+
const assert = require('assert');
146+
const { builtinModules, isBuiltin } = require('module');
147+
const prefixed = `node:${id}`;
148+
assert.strictEqual(builtinModules.includes(id), enabled && !schemeOnly, id);
149+
assert.strictEqual(builtinModules.includes(prefixed),
150+
enabled && schemeOnly, prefixed);
151+
152+
for (const specifier of [id, prefixed]) {
153+
const supported = enabled && (!schemeOnly || specifier === prefixed);
154+
assert.strictEqual(isBuiltin(specifier), supported, specifier);
155+
if (supported) {
156+
const exports = require(specifier);
157+
assert.strictEqual(process.getBuiltinModule(specifier), exports);
158+
assert.strictEqual((await import(specifier)).default, exports);
159+
} else {
160+
assert.strictEqual(process.getBuiltinModule(specifier), undefined);
161+
assert.throws(() => require(specifier), {
162+
code: specifier === prefixed ?
163+
'ERR_UNKNOWN_BUILTIN_MODULE' : 'MODULE_NOT_FOUND',
164+
});
165+
await assert.rejects(import(specifier), {
166+
code: specifier === prefixed ?
167+
'ERR_UNKNOWN_BUILTIN_MODULE' : 'ERR_MODULE_NOT_FOUND',
168+
});
169+
}
170+
}
171+
}

0 commit comments

Comments
 (0)