Skip to content
This repository has been archived by the owner on Aug 31, 2018. It is now read-only.

Commit

Permalink
buffer: move setupBufferJS to internal
Browse files Browse the repository at this point in the history
Stashing it away in internal/buffer so that it can't be used in
userland, but can still be used in internals.

PR-URL: nodejs/node#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>
  • Loading branch information
bengl authored and addaleax committed Oct 26, 2017
1 parent 1c81212 commit 5c88c76
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 16 deletions.
3 changes: 2 additions & 1 deletion lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ const {
readDoubleLE: _readDoubleLE,
readFloatBE: _readFloatBE,
readFloatLE: _readFloatLE,
setupBufferJS,
swap16: _swap16,
swap32: _swap32,
swap64: _swap64,
Expand Down Expand Up @@ -63,6 +62,8 @@ const errors = require('internal/errors');

const internalBuffer = require('internal/buffer');

const { setupBufferJS } = internalBuffer;

const bindingObj = {};

class FastBuffer extends Uint8Array {
Expand Down
4 changes: 4 additions & 0 deletions lib/internal/bootstrap_node.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 11 additions & 2 deletions lib/internal/buffer.js
Original file line number Diff line number Diff line change
@@ -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
};
22 changes: 9 additions & 13 deletions test/parallel/test-buffer-bindingobj-no-zerofill.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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')));

0 comments on commit 5c88c76

Please sign in to comment.