Skip to content

lib: move internalBinding whitelisting into loaders.js #24088

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions lib/internal/bootstrap/loaders.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,52 @@
writable: false
});

// internalBindingWhitelist contains the name of internalBinding modules
// that are whitelisted for access via process.binding()... this is used
// to provide a transition path for modules that are being moved over to
// internalBinding.
const internalBindingWhitelist = [
'cares_wrap',
'fs_event_wrap',
'icu',
'udp_wrap',
'uv',
'pipe_wrap',
'http_parser',
'process_wrap',
'v8',
'tty_wrap',
'stream_wrap',
'signal_wrap',
'crypto',
'contextify',
'tcp_wrap',
'tls_wrap',
'util',
'async_wrap',
'url',
'spawn_sync',
'js_stream',
'zlib',
'buffer',
'natives',
'constants'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you so much for moving the closing brace to a new line

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's our policy about dangling commas (I'm +1 on adding one)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have no lint rule for or against them. if it were up to me we would require them. (I'm +1 on adding one)

];
// We will use a lazy loaded SafeSet in internalBindingWhitelistHas
// for checking existence in this list.
let internalBindingWhitelistSet;

// Set up process.binding() and process._linkedBinding()
{
const bindingObj = ObjectCreate(null);

process.binding = function binding(module) {
module = String(module);
// Deprecated specific process.binding() modules, but not all, allow
// selective fallback to internalBinding for the deprecated ones.
if (internalBindingWhitelistHas(module)) {
return internalBinding(module);
}
let mod = bindingObj[module];
if (typeof mod !== 'object') {
mod = bindingObj[module] = getBinding(module);
Expand Down Expand Up @@ -368,6 +408,14 @@
NativeModule.require('internal/process/coverage').setup();
}

function internalBindingWhitelistHas(name) {
if (!internalBindingWhitelistSet) {
const { SafeSet } = NativeModule.require('internal/safe_globals');
internalBindingWhitelistSet = new SafeSet(internalBindingWhitelist);
}
return internalBindingWhitelistSet.has(name);
}

// This will be passed to the bootstrapNodeJSCore function in
// bootstrap/node.js.
return loaderExports;
Expand Down
41 changes: 0 additions & 41 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -384,47 +384,6 @@
for (var i = 0; i < arguments.length; i++)
this.push(arguments[i]);
}

// Deprecated specific process.binding() modules, but not all, allow
// selective fallback to internalBinding for the deprecated ones.
const { SafeSet } = NativeModule.require('internal/safe_globals');
const processBinding = process.binding;
// internalBindingWhitelist contains the name of internalBinding modules
// that are whitelisted for access via process.binding()... this is used
// to provide a transition path for modules that are being moved over to
// internalBinding.
const internalBindingWhitelist =
new SafeSet([
'cares_wrap',
'fs_event_wrap',
'icu',
'udp_wrap',
'uv',
'pipe_wrap',
'http_parser',
'process_wrap',
'v8',
'tty_wrap',
'stream_wrap',
'signal_wrap',
'crypto',
'contextify',
'tcp_wrap',
'tls_wrap',
'util',
'async_wrap',
'url',
'spawn_sync',
'js_stream',
'zlib',
'buffer',
'natives',
'constants']);
process.binding = function binding(name) {
return internalBindingWhitelist.has(name) ?
internalBinding(name) :
processBinding(name);
};
}

function setupGlobalVariables() {
Expand Down