diff options
author | Anna Henningsen <anna@addaleax.net> | 2018-03-22 00:30:21 +0100 |
---|---|---|
committer | Ruben Bridgewater <ruben@bridgewater.de> | 2018-04-10 00:43:41 +0200 |
commit | 9d4ab9011796902a086ca12b0a18088e2fb35cd4 (patch) | |
tree | 9e8e5076a53ce845af0c9a0893d5e7482c4ad227 | |
parent | e048b1552363df05d21ef3fa0054d9ab6b801df4 (diff) | |
download | android-node-v8-9d4ab9011796902a086ca12b0a18088e2fb35cd4.tar.gz android-node-v8-9d4ab9011796902a086ca12b0a18088e2fb35cd4.tar.bz2 android-node-v8-9d4ab9011796902a086ca12b0a18088e2fb35cd4.zip |
buffer: do deprecation warning outside `node_modules`
In addition to `--pending-deprecation`, emit a deprecation warning
for usage of the `Buffer()` constructor for call sites that are outside
of `node_modules`.
The goal of this is to better target developers, rather than
burdening users with an omnipresent and quickly ignored warning.
This implements the result of a TSC meeting discussion
from March 22, 2018.
PR-URL: https://github.com/nodejs/node/pull/19524
Refs: https://github.com/nodejs/node/issues/19079#issuecomment-375121443
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Michaƫl Zasso <targos@protonmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
-rw-r--r-- | doc/api/buffer.md | 20 | ||||
-rw-r--r-- | doc/api/deprecations.md | 6 | ||||
-rw-r--r-- | lib/buffer.js | 31 | ||||
-rw-r--r-- | lib/internal/util.js | 56 | ||||
-rw-r--r-- | test/parallel/test-buffer-constructor-node-modules-paths.js | 35 | ||||
-rw-r--r-- | test/parallel/test-buffer-constructor-outside-node-modules.js | 29 |
6 files changed, 163 insertions, 14 deletions
diff --git a/doc/api/buffer.md b/doc/api/buffer.md index 2ea989e7fe..fbd7ace6b6 100644 --- a/doc/api/buffer.md +++ b/doc/api/buffer.md @@ -304,6 +304,10 @@ It can be constructed in a variety of ways. <!-- YAML deprecated: v6.0.0 changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/19524 + description: Calling this constructor emits a deprecation warning when + run from code outside the `node_modules` directory. - version: v7.2.1 pr-url: https://github.com/nodejs/node/pull/9529 description: Calling this constructor no longer emits a deprecation warning. @@ -328,6 +332,10 @@ const buf = new Buffer([0x62, 0x75, 0x66, 0x66, 0x65, 0x72]); added: v3.0.0 deprecated: v6.0.0 changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/19524 + description: Calling this constructor emits a deprecation warning when + run from code outside the `node_modules` directory. - version: v7.2.1 pr-url: https://github.com/nodejs/node/pull/9529 description: Calling this constructor no longer emits a deprecation warning. @@ -380,6 +388,10 @@ console.log(buf); <!-- YAML deprecated: v6.0.0 changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/19524 + description: Calling this constructor emits a deprecation warning when + run from code outside the `node_modules` directory. - version: v7.2.1 pr-url: https://github.com/nodejs/node/pull/9529 description: Calling this constructor no longer emits a deprecation warning. @@ -410,6 +422,10 @@ console.log(buf2.toString()); <!-- YAML deprecated: v6.0.0 changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/19524 + description: Calling this constructor emits a deprecation warning when + run from code outside the `node_modules` directory. - version: v8.0.0 pr-url: https://github.com/nodejs/node/pull/12141 description: new Buffer(size) will return zero-filled memory by default. @@ -447,6 +463,10 @@ console.log(buf); <!-- YAML deprecated: v6.0.0 changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/19524 + description: Calling this constructor emits a deprecation warning when + run from code outside the `node_modules` directory. - version: v7.2.1 pr-url: https://github.com/nodejs/node/pull/9529 description: Calling this constructor no longer emits a deprecation warning. diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 6fd8d1d4f5..b78244b5d5 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -72,7 +72,7 @@ be used. <a id="DEP0005"></a> ### DEP0005: Buffer() constructor -Type: Documentation-only (supports [`--pending-deprecation`][]) +Type: Runtime (supports [`--pending-deprecation`][]) The `Buffer()` function and `new Buffer()` constructor are deprecated due to API usability issues that can potentially lead to accidental security issues. @@ -93,6 +93,10 @@ is strongly recommended: * [`Buffer.from(string[, encoding])`][from_string_encoding] - Create a `Buffer` that copies `string`. +As of REPLACEME, a deprecation warning is printed at runtime when +`--pending-deprecation` is used or when the calling code is +outside `node_modules` in order to better target developers, rather than users. + <a id="DEP0006"></a> ### DEP0006: child\_process options.customFds diff --git a/lib/buffer.js b/lib/buffer.js index 73e3bc4420..0737b8eccc 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -48,6 +48,7 @@ try { } const { customInspectSymbol, + isInsideNodeModules, normalizeEncoding, kIsEncodingSymbol } = require('internal/util'); @@ -139,25 +140,29 @@ function alignPool() { } } -var bufferWarn = true; +let bufferWarningAlreadyEmitted = false; +let nodeModulesCheckCounter = 0; const bufferWarning = 'Buffer() is deprecated due to security and usability ' + 'issues. Please use the Buffer.alloc(), ' + 'Buffer.allocUnsafe(), or Buffer.from() methods instead.'; function showFlaggedDeprecation() { - if (bufferWarn) { - // This is a *pending* deprecation warning. It is not emitted by - // default unless the --pending-deprecation command-line flag is - // used or the NODE_PENDING_DEPRECATION=1 env var is set. - process.emitWarning(bufferWarning, 'DeprecationWarning', 'DEP0005'); - bufferWarn = false; + if (bufferWarningAlreadyEmitted || + ++nodeModulesCheckCounter > 10000 || + (!pendingDeprecation && + isInsideNodeModules())) { + // We don't emit a warning, because we either: + // - Already did so, or + // - Already checked too many times whether a call is coming + // from node_modules and want to stop slowing down things, or + // - We aren't running with `--pending-deprecation` enabled, + // and the code is inside `node_modules`. + return; } -} -const doFlaggedDeprecation = - pendingDeprecation ? - showFlaggedDeprecation : - function() {}; + process.emitWarning(bufferWarning, 'DeprecationWarning', 'DEP0005'); + bufferWarningAlreadyEmitted = true; +} /** * The Buffer() constructor is deprecated in documentation and should not be @@ -170,7 +175,7 @@ const doFlaggedDeprecation = * Deprecation Code: DEP0005 */ function Buffer(arg, encodingOrOffset, length) { - doFlaggedDeprecation(); + showFlaggedDeprecation(); // Common case. if (typeof arg === 'number') { if (typeof encodingOrOffset === 'string') { diff --git a/lib/internal/util.js b/lib/internal/util.js index 247ac2dd5a..b92e4c5b58 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -366,6 +366,61 @@ function spliceOne(list, index) { list.pop(); } +const kNodeModulesRE = /^(.*)[\\/]node_modules[\\/]/; + +let getStructuredStack; +let mainPrefix; + +function isInsideNodeModules() { + // Match the prefix of the main file, if it is inside a `node_modules` folder, + // up to and including the innermost `/node_modules/` bit, so that + // we can later ignore files which are at the same node_modules level. + // For example, having the main script at `/c/node_modules/d/d.js` + // would match all code coming from `/c/node_modules/`. + if (mainPrefix === undefined) { + const match = process.mainModule && + process.mainModule.filename && + process.mainModule.filename.match(kNodeModulesRE); + mainPrefix = match ? match[0] : ''; + } + + if (getStructuredStack === undefined) { + // Lazy-load to avoid a circular dependency. + const { runInNewContext } = require('vm'); + // Use `runInNewContext()` to get something tamper-proof and + // side-effect-free. Since this is currently only used for a deprecated API, + // the perf implications should be okay. + getStructuredStack = runInNewContext('(' + function() { + Error.prepareStackTrace = function(err, trace) { + err.stack = trace; + }; + Error.stackTraceLimit = Infinity; + + return function structuredStack() { + // eslint-disable-next-line no-restricted-syntax + return new Error().stack; + }; + } + ')()', {}, { filename: 'structured-stack' }); + } + + const stack = getStructuredStack(); + + // Iterate over all stack frames and look for the first one not coming + // from inside Node.js itself: + for (const frame of stack) { + const filename = frame.getFileName(); + // If a filename does not start with / or contain \, + // it's likely from Node.js core. + if (!/^\/|\\/.test(filename)) + continue; + const match = filename.match(kNodeModulesRE); + return !!match && match[0] !== mainPrefix; + } + + return false; // This should be unreachable. +} + + module.exports = { assertCrypto, cachedResult, @@ -379,6 +434,7 @@ module.exports = { getSystemErrorName, getIdentificationOf, isError, + isInsideNodeModules, join, normalizeEncoding, objectToString, diff --git a/test/parallel/test-buffer-constructor-node-modules-paths.js b/test/parallel/test-buffer-constructor-node-modules-paths.js new file mode 100644 index 0000000000..717f783511 --- /dev/null +++ b/test/parallel/test-buffer-constructor-node-modules-paths.js @@ -0,0 +1,35 @@ +'use strict'; + +const child_process = require('child_process'); +const assert = require('assert'); +const common = require('../common'); + +if (process.env.NODE_PENDING_DEPRECATION) + common.skip('test does not work when NODE_PENDING_DEPRECATION is set'); + +function test(main, callSite, expected) { + const { stderr } = child_process.spawnSync(process.execPath, ['-p', ` + process.mainModule = { filename: ${JSON.stringify(main)} }; + + vm.runInNewContext('new Buffer(10)', { Buffer }, { + filename: ${JSON.stringify(callSite)} + });`], { encoding: 'utf8' }); + if (expected) + assert(stderr.includes('[DEP0005] DeprecationWarning'), stderr); + else + assert.strictEqual(stderr.trim(), ''); +} + +test('/a/node_modules/b.js', '/a/node_modules/x.js', true); +test('/a/node_modules/b.js', '/a/node_modules/foo/node_modules/x.js', false); +test('/a/node_modules/foo/node_modules/b.js', '/a/node_modules/x.js', false); +test('/node_modules/foo/b.js', '/node_modules/foo/node_modules/x.js', false); +test('/a.js', '/b.js', true); +test('/a.js', '/node_modules/b.js', false); +test('c:\\a\\node_modules\\b.js', 'c:\\a\\node_modules\\x.js', true); +test('c:\\a\\node_modules\\b.js', + 'c:\\a\\node_modules\\foo\\node_modules\\x.js', false); +test('c:\\node_modules\\foo\\b.js', + 'c:\\node_modules\\foo\\node_modules\\x.js', false); +test('c:\\a.js', 'c:\\b.js', true); +test('c:\\a.js', 'c:\\node_modules\\b.js', false); diff --git a/test/parallel/test-buffer-constructor-outside-node-modules.js b/test/parallel/test-buffer-constructor-outside-node-modules.js new file mode 100644 index 0000000000..1a1e146f2d --- /dev/null +++ b/test/parallel/test-buffer-constructor-outside-node-modules.js @@ -0,0 +1,29 @@ +// Flags: --no-warnings +'use strict'; + +const vm = require('vm'); +const assert = require('assert'); +const common = require('../common'); + +if (new Error().stack.includes('node_modules')) + common.skip('test does not work when inside `node_modules` directory'); +if (process.env.NODE_PENDING_DEPRECATION) + common.skip('test does not work when NODE_PENDING_DEPRECATION is set'); + +const bufferWarning = 'Buffer() is deprecated due to security and usability ' + + 'issues. Please use the Buffer.alloc(), ' + + 'Buffer.allocUnsafe(), or Buffer.from() methods instead.'; + +process.addListener('warning', common.mustCall((warning) => { + assert(warning.stack.includes('this_should_emit_a_warning'), warning.stack); +})); + +vm.runInNewContext('new Buffer(10)', { Buffer }, { + filename: '/a/node_modules/b' +}); + +common.expectWarning('DeprecationWarning', bufferWarning, 'DEP0005'); + +vm.runInNewContext('new Buffer(10)', { Buffer }, { + filename: '/this_should_emit_a_warning' +}); |