buffer: move setupBufferJS to internal

bengl · jasnell · commit 8172f4547e7e · 2017-10-25T10:36:17.000-07:00
Stashing it away in internal/buffer so that it can't be used in userland, but can still be used in internals. PR-URL: #16391 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
diff --git 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
@@ -292,6 +292,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
@@ -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
@@ -11,33 +11,26 @@ 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;
 };
 
 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')));
