Skip to content

Commit

Permalink
process: runtime deprecate changing process.config
Browse files Browse the repository at this point in the history
The fact that `process.config` is mutable has long made it
unreliable when it really should just work. Start the process
of deprecating the ability to change it.

Fixes: #7803
Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #36902
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
  • Loading branch information
jasnell committed Jan 22, 2021
1 parent a35b32e commit 96f3977
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 1 deletion.
15 changes: 15 additions & 0 deletions doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -2711,6 +2711,21 @@ Type: Documentation-only.

Prefer [`message.socket`][] over [`message.connection`][].

### DEP0XXX: Changing the value of `process.config`
<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/36902
description: Runtime deprecation.
-->

Type: Runtime

The `process.config` property is intended to provide access to configuration
settings set when the Node.js binary was compiled. However, the property has
been mutable by user code making it impossible to rely on. The ability to
change the value has been deprecated and will be disabled in the future.

[Legacy URL API]: url.md#url_legacy_url_api
[NIST SP 800-38D]: https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38d.pdf
[RFC 6066]: https://tools.ietf.org/html/rfc6066#section-3
Expand Down
8 changes: 8 additions & 0 deletions doc/api/process.md
Original file line number Diff line number Diff line change
Expand Up @@ -759,6 +759,10 @@ This feature is not available in [`Worker`][] threads.
## `process.config`
<!-- YAML
added: v0.7.7
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/36902
description: Modifying process.config has been deprecated.
-->

* {Object}
Expand Down Expand Up @@ -803,6 +807,10 @@ The `process.config` property is **not** read-only and there are existing
modules in the ecosystem that are known to extend, modify, or entirely replace
the value of `process.config`.

Modifying the `process.config` property, or any child-property of the
`process.config` object has been deprecated. The `process.config` will be made
read-only in a future release.

## `process.connected`
<!-- YAML
added: v0.7.2
Expand Down
72 changes: 71 additions & 1 deletion lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,10 @@ const {
JSONParse,
ObjectDefineProperty,
ObjectGetPrototypeOf,
ObjectPreventExtensions,
ObjectSetPrototypeOf,
ReflectGet,
ReflectSet,
SymbolToStringTag,
} = primordials;
const config = internalBinding('config');
Expand All @@ -59,7 +62,74 @@ process._exiting = false;

// process.config is serialized config.gypi
const nativeModule = internalBinding('native_module');
process.config = JSONParse(nativeModule.config);

// TODO(@jasnell): Once this has gone through one full major
// release cycle, remove the Proxy and setter and update the
// getter to either return a read-only object or always return
// a freshly parsed version of nativeModule.config.

const deprecationHandler = {
warned: false,
message: 'Setting process.config is deprecated. ' +
'In the future the property will be read-only.',
code: 'DEP0XXX',
maybeWarn() {
if (!this.warned) {
process.emitWarning(this.message, {
type: 'DeprecationWarning',
code: this.code
});
this.warned = true;
}
},

defineProperty(target, key, descriptor) {
this.maybeWarn();
return ObjectDefineProperty(target, key, descriptor);
},

deleteProperty(target, key) {
this.maybeWarn();
delete target[key];
},

preventExtensions(target) {
this.maybeWarn();
return ObjectPreventExtensions(target);
},

set(target, key, value) {
this.maybeWarn();
return ReflectSet(target, key, value);
},

get(target, key, receiver) {
const val = ReflectGet(target, key, receiver);
if (val != null && typeof val === 'object')
return new Proxy(val, deprecationHandler);
return val;
},

setPrototypeOf(target, proto) {
this.maybeWarn();
return ObjectSetPrototypeOf(target, proto);
}
};

let processConfig = new Proxy(
JSONParse(nativeModule.config),
deprecationHandler);

ObjectDefineProperty(process, 'config', {
enumerable: true,
configurable: true,
get() { return processConfig; },
set(value) {
deprecationHandler.maybeWarn();
processConfig = value;
}
});

require('internal/worker/js_transferable').setup();

// Bootstrappers for all threads, including worker threads and main thread
Expand Down

0 comments on commit 96f3977

Please sign in to comment.