From 43fabfe5d6c070faa0a116a4677ca031c7f5f4ad Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Sun, 25 Feb 2018 13:42:10 -0800 Subject: [PATCH] process: doc-only deprecate non-string env value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/18990 Refs: https://github.com/nodejs/node/pull/15089 Refs: https://github.com/nodejs/node/pull/18158 Reviewed-By: Matteo Collina Reviewed-By: Luigi Pinca Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Anna Henningsen Reviewed-By: Gibson Fahnestock Reviewed-By: Michaël Zasso Reviewed-By: Sakthipriyan Vairamani --- doc/api/deprecations.md | 13 ++++++++++++- doc/api/process.md | 7 ++++++- src/env-inl.h | 1 + src/env.h | 6 ++++++ src/node.cc | 11 +++++++++++ test/parallel/test-process-env-deprecation.js | 16 ++++++++++++++++ 6 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-process-env-deprecation.js diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 4eb90c4c38c2d8..b3b316f1a35e57 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -915,7 +915,6 @@ Type: Runtime This was never a documented feature. - ### DEP0101: --with-lttng @@ -940,6 +939,17 @@ Type: Documentation-only (supports [`--pending-deprecation`][]) Using `process.binding()` in general should be avoided. The type checking methods in particular can be replaced by using [`util.types`][]. + +### DEP0104: process.env string coercion + +Type: Documentation-only (supports [`--pending-deprecation`][]) + +Currently when assigning a property to [`process.env`][], the assigned value is +implicitly converted to a string if it is not a string. This behavior is +deprecated if the assigned value is not a string, boolean, or number. In the +future, such assignment may result in a thrown error. Please convert the +property to a string before assigning it to `process.env`. + [`--pending-deprecation`]: cli.html#cli_pending_deprecation [`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size [`Buffer.from(array)`]: buffer.html#buffer_class_method_buffer_from_array @@ -976,6 +986,7 @@ methods in particular can be replaced by using [`util.types`][]. [`fs.stat()`]: fs.html#fs_fs_stat_path_callback [`os.networkInterfaces`]: os.html#os_os_networkinterfaces [`os.tmpdir()`]: os.html#os_os_tmpdir +[`process.env`]: process.html#process_process_env [`punycode`]: punycode.html [`require.extensions`]: modules.html#modules_require_extensions [`setInterval()`]: timers.html#timers_setinterval_callback_delay_args diff --git a/doc/api/process.md b/doc/api/process.md index 2bd7a119bf8d6c..e89e5c0281a23b 100644 --- a/doc/api/process.md +++ b/doc/api/process.md @@ -836,6 +836,10 @@ emitMyWarning(); ## process.env * {Object} @@ -877,7 +881,8 @@ console.log(process.env.foo); ``` Assigning a property on `process.env` will implicitly convert the value -to a string. +to a string. **This behavior is deprecated.** Future versions of Node.js may +throw an error when the value is not a string, number, or boolean. Example: diff --git a/src/env-inl.h b/src/env-inl.h index 89a321a92003a7..a91ef578c8a04a 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -330,6 +330,7 @@ inline Environment::Environment(IsolateData* isolate_data, trace_sync_io_(false), abort_on_uncaught_exception_(false), emit_napi_warning_(true), + emit_env_nonstring_warning_(true), makecallback_cntr_(0), should_abort_on_uncaught_toggle_(isolate_, 1), #if HAVE_INSPECTOR diff --git a/src/env.h b/src/env.h index 27e72f35d12bce..8c73b8b206e6ec 100644 --- a/src/env.h +++ b/src/env.h @@ -725,6 +725,11 @@ class Environment { void AddPromiseHook(promise_hook_func fn, void* arg); bool RemovePromiseHook(promise_hook_func fn, void* arg); bool EmitNapiWarning(); + inline bool EmitProcessEnvWarning() { + bool current_value = emit_env_nonstring_warning_; + emit_env_nonstring_warning_ = false; + return current_value; + } typedef void (*native_immediate_callback)(Environment* env, void* data); // cb will be called as cb(env, data) on the next event loop iteration. @@ -779,6 +784,7 @@ class Environment { bool trace_sync_io_; bool abort_on_uncaught_exception_; bool emit_napi_warning_; + bool emit_env_nonstring_warning_; size_t makecallback_cntr_; std::vector destroy_async_id_list_; diff --git a/src/node.cc b/src/node.cc index 682052aadf1e44..9a1c4eeb6a72e6 100644 --- a/src/node.cc +++ b/src/node.cc @@ -2659,6 +2659,17 @@ static void EnvGetter(Local property, static void EnvSetter(Local property, Local value, const PropertyCallbackInfo& info) { + Environment* env = Environment::GetCurrent(info); + if (config_pending_deprecation && env->EmitProcessEnvWarning() && + !value->IsString() && !value->IsNumber() && !value->IsBoolean()) { + if (ProcessEmitDeprecationWarning( + env, + "Assigning any value other than a string, number, or boolean to a " + "process.env property is deprecated. Please make sure to convert the " + "value to a string before setting process.env with it.", + "DEP00XX").IsNothing()) + return; + } #ifdef __POSIX__ node::Utf8Value key(info.GetIsolate(), property); node::Utf8Value val(info.GetIsolate(), value); diff --git a/test/parallel/test-process-env-deprecation.js b/test/parallel/test-process-env-deprecation.js new file mode 100644 index 00000000000000..c3b9ea6da37a23 --- /dev/null +++ b/test/parallel/test-process-env-deprecation.js @@ -0,0 +1,16 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +// Flags: --pending-deprecation + +common.expectWarning( + 'DeprecationWarning', + 'Assigning any value other than a string, number, or boolean to a ' + + 'process.env property is deprecated. Please make sure to convert the value ' + + 'to a string before setting process.env with it.', + 'DEP00XX' +); + +process.env.ABC = undefined; +assert.strictEqual(process.env.ABC, 'undefined');