Skip to content

Commit

Permalink
lib: promote process.binding/_tickCallback to runtime deprecation
Browse files Browse the repository at this point in the history
These have been pendingDeprecation for long enough.

This also adds the `--deprecated-process-binding` (and matching
`--no-deprecated-process-binding`) CLI option as a future transition
path for when we do ultimately remove `process.binding()`.

`--deprecated-process-binding` ensures that `process.binding(...)`
is available and is currently the default. In the future, when we
are ready to begin moving `process.binding(...)` to EOF, we can
flip the default so that `process.binding('...')` is unavailable
by default *but still able to be switched back on if necessary*,
giving users an escape hatch if they really need it. This will give
us a more nuanced path than abruptly removing it completely.
  • Loading branch information
jasnell committed Nov 12, 2023
1 parent 242cbd7 commit 957fc0f
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 14 deletions.
11 changes: 11 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,12 @@ added: v12.0.0
Specify the file name of the CPU profile generated by `--cpu-prof`.

### `--deprecated-process-binding`

Ensure that the deprecated `process.binding(...)` API is available. This
is currently the default. A runtime deprecation warning, however, will be
emitted when `process.binding(...)` is accessed.

### `--diagnostic-dir=directory`

Set the directory to which all diagnostic output files are written.
Expand Down Expand Up @@ -1172,6 +1178,10 @@ Disable the `node-addons` exports condition as well as disable loading
native addons. When `--no-addons` is specified, calling `process.dlopen` or
requiring a native C++ addon will fail and throw an exception.

### `--no-deprecated-process-binding`

Makes the deprecated `process.binding(...)` API unavailable in the process.

### `--no-deprecation`

<!-- YAML
Expand Down Expand Up @@ -2325,6 +2335,7 @@ Node.js options that are allowed are:
* `--allow-fs-write`
* `--allow-worker`
* `--conditions`, `-C`
* `--deprecated-process-binding`
* `--diagnostic-dir`
* `--disable-proto`
* `--dns-result-order`
Expand Down
10 changes: 8 additions & 2 deletions doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -2203,6 +2203,9 @@ The `produceCachedData` option is deprecated. Use

<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/50687
description: Upgraded to a runtime deprecation.
- version: v11.12.0
pr-url: https://github.com/nodejs/node/pull/26500
description: Added support for `--pending-deprecation`.
Expand All @@ -2211,7 +2214,7 @@ changes:
description: Documentation-only deprecation.
-->

Type: Documentation-only (supports [`--pending-deprecation`][])
Type: Runtime

`process.binding()` is for use by Node.js internal code only.

Expand Down Expand Up @@ -2604,12 +2607,15 @@ Prefer [`response.socket`][] over [`response.connection`][] and

<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/50687
description: Upgraded to a runtime deprecation.
- version: v12.12.0
pr-url: https://github.com/nodejs/node/pull/29781
description: Documentation-only deprecation.
-->

Type: Documentation-only (supports [`--pending-deprecation`][])
Type: Runtime

The `process._tickCallback` property was never documented as
an officially supported API.
Expand Down
4 changes: 3 additions & 1 deletion lib/internal/bootstrap/realm.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ ObjectDefineProperty(process, 'moduleLoadList', {
writable: false,
});


// processBindingAllowList contains the name of bindings that are allowed
// for access via process.binding(). This is used to provide a transition
// path for modules that are being moved over to internalBinding.
Expand Down Expand Up @@ -144,6 +143,9 @@ const experimentalModuleList = new SafeSet();
{
const bindingObj = { __proto__: null };

// TODO(@jasnell): If the --deprecated-process-binding option is false we really
// ought to skip setting this here. However, we cannot require internal/options or
// use internalBinding here so we'll have to pull it back out later.
process.binding = function binding(module) {
module = String(module);
// Deprecated specific process.binding() modules, but not all, allow
Expand Down
12 changes: 9 additions & 3 deletions lib/internal/process/policy.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ const {
ERR_MANIFEST_TDZ,
} = require('internal/errors').codes;
const { Manifest } = require('internal/policy/manifest');

const { getOptionValue } = require('internal/options');
const deprecatedProcessBinding = getOptionValue('--deprecated-process-binding');

let manifest;
let manifestSrc;
let manifestURL;
Expand All @@ -36,9 +40,11 @@ module.exports = ObjectFreeze({

// process.binding() is deprecated (DEP0111) and trivially allows bypassing
// policies, so if policies are enabled, make this API unavailable.
process.binding = function binding(_module) {
throw new ERR_ACCESS_DENIED('process.binding');
};
if (deprecatedProcessBinding) {
process.binding = function binding(_module) {
throw new ERR_ACCESS_DENIED('process.binding');
};
}
process._linkedBinding = function _linkedBinding(_module) {
throw new ERR_ACCESS_DENIED('process._linkedBinding');
};
Expand Down
23 changes: 15 additions & 8 deletions lib/internal/process/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -545,15 +545,20 @@ function initializeDeprecations() {
});
}

if (pendingDeprecation) {
if (getOptionValue('--deprecated-process-binding')) {
process.binding = deprecate(process.binding,
'process.binding() is deprecated. ' +
'Please use public APIs instead.', 'DEP0111');

process._tickCallback = deprecate(process._tickCallback,
'process._tickCallback() is deprecated',
'DEP0134');
} else {
// Ideally it wouldn't be defined at all already but realm.js does not
// provide a means of checking to see if the option is enabled or not,
// so we do it here.
process.binding = undefined;
}

process._tickCallback = deprecate(process._tickCallback,
'process._tickCallback() is deprecated',
'DEP0134');
}

function setupChildProcessIpcChannel() {
Expand Down Expand Up @@ -587,9 +592,11 @@ function initializeClusterIPC() {
function initializePermission() {
const experimentalPermission = getOptionValue('--experimental-permission');
if (experimentalPermission) {
process.binding = function binding(_module) {
throw new ERR_ACCESS_DENIED('process.binding');
};
if (getOptionValue('--deprecated-process-binding') && process.binding !== undefined) {
process.binding = function binding(_module) {
throw new ERR_ACCESS_DENIED('process.binding');
};
}
process.emitWarning('Permission is an experimental feature',
'ExperimentalWarning');
const { has, deny } = require('internal/process/permission');
Expand Down
5 changes: 5 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,11 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
"emit pending deprecation warnings",
&EnvironmentOptions::pending_deprecation,
kAllowedInEnvvar);
AddOption("--deprecated-process-binding",
"enable the deprecated process.binding(...) api",
&EnvironmentOptions::deprecated_process_binding,
kAllowedInEnvvar,
true);
AddOption("--preserve-symlinks",
"preserve symbolic links when resolving",
&EnvironmentOptions::preserve_symlinks,
Expand Down
10 changes: 10 additions & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,16 @@ class EnvironmentOptions : public Options {
false;
#endif // DEBUG

// TODO(@jasnell): In preparation for the eventual complete removal of
// the deprecated process.binding, this option controls whether or not
// process.binding is available. Currently the default is true, making
// the deprecated API available, albeit with a runtime deprecation message.
// Then, later, we can change the default to false, giving folks the option
// to re-enable if they really need it. Then, as the final step, we would
// remove the deprecated api entirely, in which case this option becomes
// a non-op.
bool deprecated_process_binding = true;

bool watch_mode = false;
bool watch_mode_report_to_parent = false;
bool watch_mode_preserve_output = false;
Expand Down
5 changes: 5 additions & 0 deletions test/parallel/test-no-deprecated-process-binding.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
'use strict';
// Flags: --no-deprecated-process-binding
require('../common');
const { strictEqual } = require('node:assert');
strictEqual(process.binding, undefined);

0 comments on commit 957fc0f

Please sign in to comment.