From 5c88c76e0dae22e3d86261e4b32aba40986ceafd Mon Sep 17 00:00:00 2001 From: Bryan English Date: Sat, 21 Oct 2017 23:25:59 -0700 Subject: [PATCH] buffer: move setupBufferJS to internal Stashing it away in internal/buffer so that it can't be used in userland, but can still be used in internals. PR-URL: https://github.com/nodejs/node/pull/16391 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Timothy Gu Reviewed-By: Refael Ackermann Reviewed-By: James M Snell --- lib/buffer.js | 3 ++- lib/internal/bootstrap_node.js | 4 ++++ lib/internal/buffer.js | 13 +++++++++-- .../test-buffer-bindingobj-no-zerofill.js | 22 ++++++++----------- 4 files changed, 26 insertions(+), 16 deletions(-) diff --git a/lib/buffer.js b/lib/buffer.js index 908e8b56e8..f326aef751 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -35,7 +35,6 @@ const { readDoubleLE: _readDoubleLE, readFloatBE: _readFloatBE, readFloatLE: _readFloatLE, - setupBufferJS, swap16: _swap16, swap32: _swap32, swap64: _swap64, @@ -63,6 +62,8 @@ const errors = require('internal/errors'); const internalBuffer = require('internal/buffer'); +const { setupBufferJS } = internalBuffer; + const bindingObj = {}; class FastBuffer extends Uint8Array { diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js index 4ebc81c488..868a984e56 100644 --- a/lib/internal/bootstrap_node.js +++ b/lib/internal/bootstrap_node.js @@ -300,6 +300,10 @@ }); } + // This, as side effect, removes `setupBufferJS` from the buffer binding, + // and exposes it on `internal/buffer`. + NativeModule.require('internal/buffer'); + global.Buffer = NativeModule.require('buffer').Buffer; process.domain = null; process._exiting = false; diff --git a/lib/internal/buffer.js b/lib/internal/buffer.js index 178a475715..105ff4132a 100644 --- a/lib/internal/buffer.js +++ b/lib/internal/buffer.js @@ -1,4 +1,13 @@ 'use strict'; -// This is needed still for FastBuffer -module.exports = {}; +const binding = process.binding('buffer'); +const { setupBufferJS } = binding; + +// Remove from the binding so that function is only available as exported here. +// (That is, for internal use only.) +delete binding.setupBufferJS; + +// FastBuffer wil be inserted here by lib/buffer.js +module.exports = { + setupBufferJS +}; diff --git a/test/parallel/test-buffer-bindingobj-no-zerofill.js b/test/parallel/test-buffer-bindingobj-no-zerofill.js index be89c99d67..ab584c2597 100644 --- a/test/parallel/test-buffer-bindingobj-no-zerofill.js +++ b/test/parallel/test-buffer-bindingobj-no-zerofill.js @@ -11,13 +11,11 @@ const assert = require('assert'); const buffer = require('buffer'); // Monkey-patch setupBufferJS() to have an undefined zeroFill. -const process = require('process'); -const originalBinding = process.binding; +const internalBuffer = require('internal/buffer'); -const binding = originalBinding('buffer'); -const originalSetup = binding.setupBufferJS; +const originalSetup = internalBuffer.setupBufferJS; -binding.setupBufferJS = (proto, obj) => { +internalBuffer.setupBufferJS = (proto, obj) => { originalSetup(proto, obj); assert.strictEqual(obj.zeroFill[0], 1); delete obj.zeroFill; @@ -25,19 +23,14 @@ binding.setupBufferJS = (proto, obj) => { const bindingObj = {}; -binding.setupBufferJS(Buffer.prototype, bindingObj); +internalBuffer.setupBufferJS(Buffer.prototype, bindingObj); assert.strictEqual(bindingObj.zeroFill, undefined); -process.binding = (bindee) => { - if (bindee === 'buffer') - return binding; - return originalBinding(bindee); -}; - // Load from file system because internal buffer is already loaded and we're // testing code that runs on first load only. // Do not move this require() to top of file. It is important that -// `process.binding('buffer').setupBufferJS` be monkey-patched before this runs. +// `require('internal/buffer').setupBufferJS` be monkey-patched before this +// runs. const monkeyPatchedBuffer = require('../../lib/buffer'); // On unpatched buffer, allocUnsafe() should not zero fill memory. It's always @@ -51,3 +44,6 @@ while (uninitialized.every((val) => val === 0)) // zero-fill in that case. const zeroFilled = monkeyPatchedBuffer.Buffer.allocUnsafe(1024); assert(zeroFilled.every((val) => val === 0)); + +// setupBufferJS shouldn't still be exposed on the binding +assert(!('setupBufferJs' in process.binding('buffer')));