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 06d49c1

Browse filesBrowse files
joyeecheungRafaelGSS
joyeecheung
authored and
RafaelGSS
committed
src: implement natives binding without special casing
This patch removes special case in the internal binding loader for natives, and implements it using the builtins internal binding. Internally we do not actually need the natives binding, so implement it as a legacy wrapper instead. PR-URL: #48186 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
1 parent a2e107d commit 06d49c1
Copy full SHA for 06d49c1

File tree

Expand file treeCollapse file tree

8 files changed

+43
-29
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

8 files changed

+43
-29
lines changed
Open diff view settings
Collapse file

‎lib/internal/bootstrap/realm.js‎

Copy file name to clipboardExpand all lines: lib/internal/bootstrap/realm.js
+9-6Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,13 @@ ObjectDefineProperty(process, 'moduleLoadList', {
7878
});
7979

8080

81-
// internalBindingAllowlist contains the name of internalBinding modules
82-
// that are allowed for access via process.binding()... This is used
83-
// to provide a transition path for modules that are being moved over to
84-
// internalBinding.
85-
const internalBindingAllowlist = new SafeSet([
81+
// processBindingAllowList contains the name of bindings that are allowed
82+
// for access via process.binding(). This is used to provide a transition
83+
// path for modules that are being moved over to internalBinding.
84+
// Certain bindings may not actually correspond to an internalBinding any
85+
// more, we just implement them as legacy wrappers instead. See the
86+
// legacyWrapperList.
87+
const processBindingAllowList = new SafeSet([
8688
'async_wrap',
8789
'buffer',
8890
'cares_wrap',
@@ -124,6 +126,7 @@ const runtimeDeprecatedList = new SafeSet([
124126
]);
125127

126128
const legacyWrapperList = new SafeSet([
129+
'natives',
127130
'util',
128131
]);
129132

@@ -145,7 +148,7 @@ const experimentalModuleList = new SafeSet();
145148
module = String(module);
146149
// Deprecated specific process.binding() modules, but not all, allow
147150
// selective fallback to internalBinding for the deprecated ones.
148-
if (internalBindingAllowlist.has(module)) {
151+
if (processBindingAllowList.has(module)) {
149152
if (runtimeDeprecatedList.has(module)) {
150153
runtimeDeprecatedList.delete(module);
151154
process.emitWarning(
Collapse file

‎lib/internal/debugger/inspect_repl.js‎

Copy file name to clipboardExpand all lines: lib/internal/debugger/inspect_repl.js
+2-6Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -118,16 +118,12 @@ function extractFunctionName(description) {
118118
return fnNameMatch ? `: ${fnNameMatch[1]}` : '';
119119
}
120120

121-
const {
122-
builtinIds: PUBLIC_BUILTINS,
123-
} = internalBinding('builtins');
124-
const NATIVES = internalBinding('natives');
121+
const { builtinIds } = internalBinding('builtins');
125122
function isNativeUrl(url) {
126123
url = SideEffectFreeRegExpPrototypeSymbolReplace(/\.js$/, url, '');
127124

128125
return StringPrototypeStartsWith(url, 'node:internal/') ||
129-
ArrayPrototypeIncludes(PUBLIC_BUILTINS, url) ||
130-
url in NATIVES || url === 'bootstrap_node';
126+
ArrayPrototypeIncludes(builtinIds, url);
131127
}
132128

133129
function getRelativePath(filenameOrURL) {
Collapse file

‎lib/internal/legacy/processbinding.js‎

Copy file name to clipboardExpand all lines: lib/internal/legacy/processbinding.js
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,4 +33,12 @@ module.exports = {
3333
], key);
3434
})));
3535
},
36+
natives() {
37+
const { natives: result, configs } = internalBinding('builtins');
38+
// Legacy feature: process.binding('natives').config contains stringified
39+
// config.gypi. We do not use this object internally so it's fine to mutate
40+
// it.
41+
result.configs = configs;
42+
return result;
43+
},
3644
};
Collapse file

‎lib/internal/v8_prof_processor.js‎

Copy file name to clipboardExpand all lines: lib/internal/v8_prof_processor.js
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ const console = require('internal/console/global');
1212
const vm = require('vm');
1313
const { SourceTextModule } = require('internal/vm/module');
1414

15-
const natives = internalBinding('natives');
15+
const { natives } = internalBinding('builtins');
1616

1717
async function linker(specifier, referencingModule) {
1818
// Transform "./file.mjs" to "file"
Collapse file

‎src/node_binding.cc‎

Copy file name to clipboardExpand all lines: src/node_binding.cc
-9Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -635,15 +635,6 @@ void GetInternalBinding(const FunctionCallbackInfo<Value>& args) {
635635
exports = Object::New(isolate);
636636
CHECK(exports->SetPrototype(context, Null(isolate)).FromJust());
637637
DefineConstants(isolate, exports);
638-
} else if (!strcmp(*module_v, "natives")) {
639-
exports = realm->env()->builtin_loader()->GetSourceObject(context);
640-
// Legacy feature: process.binding('natives').config contains stringified
641-
// config.gypi
642-
CHECK(exports
643-
->Set(context,
644-
realm->isolate_data()->config_string(),
645-
realm->env()->builtin_loader()->GetConfigString(isolate))
646-
.FromJust());
647638
} else {
648639
return THROW_ERR_INVALID_MODULE(isolate, "No such binding: %s", *module_v);
649640
}
Collapse file

‎src/node_builtins.cc‎

Copy file name to clipboardExpand all lines: src/node_builtins.cc
+17-4Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,15 +63,19 @@ bool BuiltinLoader::Add(const char* id, const UnionBytes& source) {
6363
return result.second;
6464
}
6565

66-
Local<Object> BuiltinLoader::GetSourceObject(Local<Context> context) {
67-
Isolate* isolate = context->GetIsolate();
66+
void BuiltinLoader::GetNatives(Local<Name> property,
67+
const PropertyCallbackInfo<Value>& info) {
68+
Environment* env = Environment::GetCurrent(info);
69+
Isolate* isolate = env->isolate();
70+
Local<Context> context = env->context();
71+
6872
Local<Object> out = Object::New(isolate);
69-
auto source = source_.read();
73+
auto source = env->builtin_loader()->source_.read();
7074
for (auto const& x : *source) {
7175
Local<String> key = OneByteString(isolate, x.first.c_str(), x.first.size());
7276
out->Set(context, key, x.second.ToStringChecked(isolate)).FromJust();
7377
}
74-
return out;
78+
info.GetReturnValue().Set(out);
7579
}
7680

7781
Local<String> BuiltinLoader::GetConfigString(Isolate* isolate) {
@@ -689,6 +693,14 @@ void BuiltinLoader::CreatePerIsolateProperties(IsolateData* isolate_data,
689693
None,
690694
SideEffectType::kHasNoSideEffect);
691695

696+
target->SetAccessor(FIXED_ONE_BYTE_STRING(isolate, "natives"),
697+
GetNatives,
698+
nullptr,
699+
Local<Value>(),
700+
DEFAULT,
701+
None,
702+
SideEffectType::kHasNoSideEffect);
703+
692704
SetMethod(isolate, target, "getCacheUsage", BuiltinLoader::GetCacheUsage);
693705
SetMethod(isolate, target, "compileFunction", BuiltinLoader::CompileFunction);
694706
SetMethod(isolate, target, "hasCachedBuiltins", HasCachedBuiltins);
@@ -712,6 +724,7 @@ void BuiltinLoader::RegisterExternalReferences(
712724
registry->Register(CompileFunction);
713725
registry->Register(HasCachedBuiltins);
714726
registry->Register(SetInternalLoaders);
727+
registry->Register(GetNatives);
715728

716729
RegisterExternalReferencesForInternalizedBuiltinCode(registry);
717730
}
Collapse file

‎src/node_builtins.h‎

Copy file name to clipboardExpand all lines: src/node_builtins.h
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,6 @@ class NODE_EXTERN_PRIVATE BuiltinLoader {
107107
const char* id,
108108
Realm* realm);
109109

110-
v8::Local<v8::Object> GetSourceObject(v8::Local<v8::Context> context);
111110
// Returns config.gypi as a JSON string
112111
v8::Local<v8::String> GetConfigString(v8::Isolate* isolate);
113112
bool Exists(const char* id);
@@ -169,6 +168,9 @@ class NODE_EXTERN_PRIVATE BuiltinLoader {
169168
static void CompileFunction(const v8::FunctionCallbackInfo<v8::Value>& args);
170169
static void HasCachedBuiltins(
171170
const v8::FunctionCallbackInfo<v8::Value>& args);
171+
// For legacy process.binding('natives')
172+
static void GetNatives(v8::Local<v8::Name> property,
173+
const v8::PropertyCallbackInfo<v8::Value>& info);
172174

173175
void AddExternalizedBuiltin(const char* id, const char* filename);
174176

Collapse file

‎test/es-module/test-loaders-hidden-from-users.js‎

Copy file name to clipboardExpand all lines: test/es-module/test-loaders-hidden-from-users.js
+3-2Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,9 @@ assert.throws(
1717
assert.throws(
1818
() => {
1919
const source = 'module.exports = require("internal/bootstrap/realm")';
20-
const { internalBinding } = require('internal/test/binding');
21-
internalBinding('natives').owo = source;
20+
// This needs to be process.binding() to mimic what's normally available
21+
// in the user land.
22+
process.binding('natives').owo = source;
2223
require('owo');
2324
}, {
2425
code: 'MODULE_NOT_FOUND',

0 commit comments

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