From 09f844a7b564b2f06f8ca7d72a0e144a7f5ee2cf Mon Sep 17 00:00:00 2001 From: briete Date: Mon, 17 Dec 2018 23:45:24 +0900 Subject: [PATCH 1/4] os: move process.binding('os') to internalBinding PR-URL: https://github.com/nodejs/node/pull/25087 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: James M Snell --- lib/internal/bootstrap/loaders.js | 1 + lib/os.js | 2 +- src/node_os.cc | 2 +- test/parallel/test-os-checked-function.js | 6 +++++- .../test-process-binding-internalbinding-whitelist.js | 1 + 5 files changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index 2d197c768c7107..5c0a5b6aa1fb80 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -87,6 +87,7 @@ 'icu', 'js_stream', 'natives', + 'os', 'pipe_wrap', 'process_wrap', 'signal_wrap', diff --git a/lib/os.js b/lib/os.js index 2c806908eeac98..1acc69a2415b3d 100644 --- a/lib/os.js +++ b/lib/os.js @@ -44,7 +44,7 @@ const { getUptime, isBigEndian, setPriority: _setPriority -} = process.binding('os'); +} = internalBinding('os'); function getCheckedFunction(fn) { return function checkError(...args) { diff --git a/src/node_os.cc b/src/node_os.cc index 72719f2933728c..6ecfb7fe9f8f44 100644 --- a/src/node_os.cc +++ b/src/node_os.cc @@ -467,4 +467,4 @@ void Initialize(Local target, } // namespace os } // namespace node -NODE_BUILTIN_MODULE_CONTEXT_AWARE(os, node::os::Initialize) +NODE_MODULE_CONTEXT_AWARE_INTERNAL(os, node::os::Initialize) diff --git a/test/parallel/test-os-checked-function.js b/test/parallel/test-os-checked-function.js index 04c2c3a1f82ea3..2a393f830d4b76 100644 --- a/test/parallel/test-os-checked-function.js +++ b/test/parallel/test-os-checked-function.js @@ -1,7 +1,11 @@ 'use strict'; +// Flags: --expose_internals + +const { internalBinding } = require('internal/test/binding'); + // Monkey patch the os binding before requiring any other modules, including // common, which requires the os module. -process.binding('os').getHomeDirectory = function(ctx) { +internalBinding('os').getHomeDirectory = function(ctx) { ctx.syscall = 'foo'; ctx.code = 'bar'; ctx.message = 'baz'; diff --git a/test/parallel/test-process-binding-internalbinding-whitelist.js b/test/parallel/test-process-binding-internalbinding-whitelist.js index 9cd12b54ea0f8a..c7b683a88a1376 100644 --- a/test/parallel/test-process-binding-internalbinding-whitelist.js +++ b/test/parallel/test-process-binding-internalbinding-whitelist.js @@ -16,3 +16,4 @@ assert(process.binding('url')); assert(process.binding('spawn_sync')); assert(process.binding('js_stream')); assert(process.binding('buffer')); +assert(process.binding('os')); From 57a5b973db4500219640bb5e432f7992773768af Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 12 Dec 2018 01:24:59 +0800 Subject: [PATCH 2/4] lib: remove internalBinding('config').pendingDeprecation Instead use `require('internal/options').getOptionValue('--pending-deprecation')` PR-URL: https://github.com/nodejs/node/pull/24962 Reviewed-By: James M Snell Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Ruben Bridgewater --- lib/buffer.js | 6 ++--- lib/internal/bootstrap/node.js | 2 +- src/node_config.cc | 3 --- test/parallel/test-pending-deprecation.js | 32 +++++++++++++++-------- 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/lib/buffer.js b/lib/buffer.js index 83deacd1fb0ce1..aea97ae53bee0c 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -48,9 +48,7 @@ const { isArrayBufferView, isUint8Array } = require('internal/util/types'); -const { - pendingDeprecation -} = internalBinding('config'); + const { ERR_BUFFER_OUT_OF_BOUNDS, ERR_OUT_OF_RANGE, @@ -138,7 +136,7 @@ const bufferWarning = 'Buffer() is deprecated due to security and usability ' + function showFlaggedDeprecation() { if (bufferWarningAlreadyEmitted || ++nodeModulesCheckCounter > 10000 || - (!pendingDeprecation && + (!require('internal/options').getOptionValue('--pending-deprecation') && isInsideNodeModules())) { // We don't emit a warning, because we either: // - Already did so, or diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index d7e8ff5016f763..39237adab376c4 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -177,7 +177,7 @@ { // Install legacy getters on the `util` binding for typechecking. // TODO(addaleax): Turn into a full runtime deprecation. - const { pendingDeprecation } = internalBinding('config'); + const pendingDeprecation = getOptionValue('--pending-deprecation'); const utilBinding = internalBinding('util'); const types = internalBinding('types'); for (const name of [ diff --git a/src/node_config.cc b/src/node_config.cc index a8ffac2cc158a4..1ba1fe9d277af5 100644 --- a/src/node_config.cc +++ b/src/node_config.cc @@ -77,9 +77,6 @@ static void Initialize(Local target, if (env->options()->experimental_repl_await) READONLY_TRUE_PROPERTY(target, "experimentalREPLAwait"); - if (env->options()->pending_deprecation) - READONLY_TRUE_PROPERTY(target, "pendingDeprecation"); - if (env->options()->expose_internals) READONLY_TRUE_PROPERTY(target, "exposeInternals"); diff --git a/test/parallel/test-pending-deprecation.js b/test/parallel/test-pending-deprecation.js index bb11d84bcc0aec..6cabf9ddc693f7 100644 --- a/test/parallel/test-pending-deprecation.js +++ b/test/parallel/test-pending-deprecation.js @@ -1,36 +1,45 @@ 'use strict'; -// Tests that --pending-deprecation and NODE_PENDING_DEPRECATION both -// set the process.binding('config').pendingDeprecation flag that is -// used to determine if pending deprecation messages should be shown. -// The test is performed by launching two child processes that run +// Flags: --expose-internals +// Tests that --pending-deprecation and NODE_PENDING_DEPRECATION both set the +// `require('internal/options').getOptionValue('--pending-deprecation')` +// flag that is used to determine if pending deprecation messages should be +// shown. The test is performed by launching two child processes that run // this same test script with different arguments. If those exit with // code 0, then the test passes. If they don't, it fails. +// TODO(joyeecheung): instead of testing internals, test the actual features +// pending deprecations. + const common = require('../common'); const assert = require('assert'); -const config = process.binding('config'); const fork = require('child_process').fork; +const { getOptionValue } = require('internal/options'); function message(name) { - return `${name} did not set the process.binding('config').` + - 'pendingDeprecation flag.'; + return `${name} did not affect getOptionValue('--pending-deprecation')`; } switch (process.argv[2]) { case 'env': case 'switch': - assert.strictEqual(config.pendingDeprecation, true); + assert.strictEqual( + getOptionValue('--pending-deprecation'), + true + ); break; default: // Verify that the flag is off by default. const envvar = process.env.NODE_PENDING_DEPRECATION; - assert.strictEqual(config.pendingDeprecation, envvar && envvar[0] === '1'); + assert.strictEqual( + getOptionValue('--pending-deprecation'), + !!(envvar && envvar[0] === '1') + ); // Test the --pending-deprecation command line switch. fork(__filename, ['switch'], { - execArgv: ['--pending-deprecation'], + execArgv: ['--pending-deprecation', '--expose-internals'], silent: true }).on('exit', common.mustCall((code) => { assert.strictEqual(code, 0, message('--pending-deprecation')); @@ -38,7 +47,7 @@ switch (process.argv[2]) { // Test the --pending_deprecation command line switch. fork(__filename, ['switch'], { - execArgv: ['--pending_deprecation'], + execArgv: ['--pending_deprecation', '--expose-internals'], silent: true }).on('exit', common.mustCall((code) => { assert.strictEqual(code, 0, message('--pending_deprecation')); @@ -47,6 +56,7 @@ switch (process.argv[2]) { // Test the NODE_PENDING_DEPRECATION environment var. fork(__filename, ['env'], { env: Object.assign({}, process.env, { NODE_PENDING_DEPRECATION: 1 }), + execArgv: ['--expose-internals'], silent: true }).on('exit', common.mustCall((code) => { assert.strictEqual(code, 0, message('NODE_PENDING_DEPRECATION')); From 40df84fe59014f0ae502ceeb7b6c671d69ef445b Mon Sep 17 00:00:00 2001 From: Beni von Cheni Date: Sun, 9 Dec 2018 22:12:39 -0500 Subject: [PATCH 3/4] inspector: move process.binding to internalBinding In places of process.binding('inspector'), migrate code to adapt internalBinding. PR-URL: https://github.com/nodejs/node/pull/24931 Refs: https://github.com/nodejs/node/issues/22160 Reviewed-By: Joyee Cheung Reviewed-By: Anna Henningsen Reviewed-By: Ruben Bridgewater --- lib/inspector.js | 2 +- lib/internal/bootstrap/loaders.js | 1 + lib/internal/inspector_async_hook.js | 2 +- lib/internal/modules/cjs/helpers.js | 2 +- lib/internal/modules/cjs/loader.js | 2 +- lib/internal/modules/esm/module_job.js | 2 +- lib/internal/process/coverage.js | 2 +- src/inspector_js_api.cc | 2 +- src/node.cc | 2 +- test/parallel/test-process-binding-internalbinding-whitelist.js | 1 + test/sequential/test-async-wrap-getasyncid.js | 2 +- 11 files changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/inspector.js b/lib/inspector.js index 6988eccf82c9ef..f79ef9cec4e639 100644 --- a/lib/inspector.js +++ b/lib/inspector.js @@ -11,7 +11,7 @@ const { } = require('internal/errors').codes; const { validateString } = require('internal/validators'); const util = require('util'); -const { Connection, open, url } = process.binding('inspector'); +const { Connection, open, url } = internalBinding('inspector'); const { originalConsole } = require('internal/process/per_thread'); if (!Connection) diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index 5c0a5b6aa1fb80..4d7bfa43f658d8 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -85,6 +85,7 @@ 'fs_event_wrap', 'http_parser', 'icu', + 'inspector', 'js_stream', 'natives', 'os', diff --git a/lib/internal/inspector_async_hook.js b/lib/internal/inspector_async_hook.js index 1ad0cbf3a533f2..4a3d31fc2ab7de 100644 --- a/lib/internal/inspector_async_hook.js +++ b/lib/internal/inspector_async_hook.js @@ -1,6 +1,6 @@ 'use strict'; -const inspector = process.binding('inspector'); +const inspector = internalBinding('inspector'); if (!inspector || !inspector.asyncTaskScheduled) { exports.setup = function() {}; diff --git a/lib/internal/modules/cjs/helpers.js b/lib/internal/modules/cjs/helpers.js index 2c856a99c6b8fd..fcb536c3df3738 100644 --- a/lib/internal/modules/cjs/helpers.js +++ b/lib/internal/modules/cjs/helpers.js @@ -108,7 +108,7 @@ if (getOptionValue('--experimental-worker')) { builtinLibs.sort(); } -if (typeof process.binding('inspector').open === 'function') { +if (typeof internalBinding('inspector').open === 'function') { builtinLibs.push('inspector'); builtinLibs.sort(); } diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 83ca0699773fa8..1b75f823f167ed 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -706,7 +706,7 @@ Module.prototype._compile = function(content, filename) { // Set breakpoint on module start if (filename === resolvedArgv) { delete process._breakFirstLine; - inspectorWrapper = process.binding('inspector').callAndPauseOnStart; + inspectorWrapper = internalBinding('inspector').callAndPauseOnStart; } } var dirname = path.dirname(filename); diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 50aa00a09a03ee..fbb29aef783006 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -73,7 +73,7 @@ class ModuleJob { try { if (this.isMain && process._breakFirstLine) { delete process._breakFirstLine; - const initWrapper = process.binding('inspector').callAndPauseOnStart; + const initWrapper = internalBinding('inspector').callAndPauseOnStart; initWrapper(this.module.instantiate, this.module); } else { this.module.instantiate(); diff --git a/lib/internal/process/coverage.js b/lib/internal/process/coverage.js index 9c9b2b47119ea7..5c5d0c2b6171f3 100644 --- a/lib/internal/process/coverage.js +++ b/lib/internal/process/coverage.js @@ -51,7 +51,7 @@ function disableAllAsyncHooks() { exports.writeCoverage = writeCoverage; function setup() { - const { Connection } = process.binding('inspector'); + const { Connection } = internalBinding('inspector'); if (!Connection) { console.warn('inspector not enabled'); return; diff --git a/src/inspector_js_api.cc b/src/inspector_js_api.cc index 8f0332cc616341..3f05f02bb50ddb 100644 --- a/src/inspector_js_api.cc +++ b/src/inspector_js_api.cc @@ -315,5 +315,5 @@ void Initialize(Local target, Local unused, } // namespace inspector } // namespace node -NODE_BUILTIN_MODULE_CONTEXT_AWARE(inspector, +NODE_MODULE_CONTEXT_AWARE_INTERNAL(inspector, node::inspector::Initialize); diff --git a/src/node.cc b/src/node.cc index e8710bef22d3d7..769d1ed8f447f3 100644 --- a/src/node.cc +++ b/src/node.cc @@ -2238,5 +2238,5 @@ int Start(int argc, char** argv) { #if !HAVE_INSPECTOR void Initialize() {} -NODE_BUILTIN_MODULE_CONTEXT_AWARE(inspector, Initialize) +NODE_MODULE_CONTEXT_AWARE_INTERNAL(inspector, Initialize) #endif // !HAVE_INSPECTOR diff --git a/test/parallel/test-process-binding-internalbinding-whitelist.js b/test/parallel/test-process-binding-internalbinding-whitelist.js index c7b683a88a1376..7689ff1ca31ed4 100644 --- a/test/parallel/test-process-binding-internalbinding-whitelist.js +++ b/test/parallel/test-process-binding-internalbinding-whitelist.js @@ -16,4 +16,5 @@ assert(process.binding('url')); assert(process.binding('spawn_sync')); assert(process.binding('js_stream')); assert(process.binding('buffer')); +assert(process.binding('inspector')); assert(process.binding('os')); diff --git a/test/sequential/test-async-wrap-getasyncid.js b/test/sequential/test-async-wrap-getasyncid.js index 3a59ade9a950a4..5c3a7b766fda99 100644 --- a/test/sequential/test-async-wrap-getasyncid.js +++ b/test/sequential/test-async-wrap-getasyncid.js @@ -291,7 +291,7 @@ if (common.hasCrypto) { // eslint-disable-line node-core/crypto-check if (process.config.variables.v8_enable_inspector !== 0 && common.isMainThread) { - const binding = process.binding('inspector'); + const binding = internalBinding('inspector'); const handle = new binding.Connection(() => {}); testInitialized(handle, 'Connection'); handle.disconnect(); From 7472860f7892b2f565ce7027c4891b084369a3dd Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 13 Dec 2018 05:23:16 +0100 Subject: [PATCH 4/4] util: inspect ArrayBuffers contents as well Inspecting an ArrayBuffer now also shows their binary contents. PR-URL: https://github.com/nodejs/node/pull/25006 Reviewed-By: Anna Henningsen --- doc/api/util.md | 3 + lib/internal/util/inspect.js | 26 +++++++-- .../test-util-format-shared-arraybuffer.js | 6 -- test/parallel/test-util-format.js | 5 ++ test/parallel/test-util-inspect.js | 55 ++++++++++++------- 5 files changed, 65 insertions(+), 30 deletions(-) delete mode 100644 test/parallel/test-util-format-shared-arraybuffer.js diff --git a/doc/api/util.md b/doc/api/util.md index 7c5a0d2cd5921c..2dd0d0912bccea 100644 --- a/doc/api/util.md +++ b/doc/api/util.md @@ -375,6 +375,9 @@ stream.write('With ES6');