Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use native Buffer whenever available #1032

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.yaml
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
unreleased:
breaking changes:
- GH-1052 Dropped support for Node < v18
new features:
- GH-1032 Enhanced performance when operating on buffers in Node environment
fixed bugs:
- GH-1036 Fixed `uncaughtException` event listener not being removed
- GH-1034 Fixed an issue where sandbox crashes for large response body
chores:
- GH-1033 Update ci.yml to run coverage on latest node version
- GH-1032 Add support for configuring module resolver based on environment

5.1.2:
date: 2024-09-04
Expand Down
28 changes: 25 additions & 3 deletions lib/bundle/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,18 @@ class Bundle {
*/
this.compress = options.compress;

this.preferBrowserResolver = options.preferBrowserResolver;

// process any list of modules externally required and also accommodate the use of built-ins if needed
_.forEach(options.require, (options, resolve) => {
// allow resolution override where the required module is resolved
// from a different name than the one provided in options
options.resolve && (resolve = options.resolve);
if (this.preferBrowserResolver && options.resolveBrowser) {
resolve = options.resolveBrowser;
}
else if (options.resolve) {
resolve = options.resolve;
}

// set the name using which the module is exported to the one marked as the module name (only in case when
// one is not explicitly provided in options.)
Expand All @@ -88,10 +95,25 @@ class Bundle {
this.bundler.bundle((err, bundle) => {
if (err) { return done(err); }

// Expose only the required node built-ins to be used in vendor modules
// These will be cleared out by sandbox when the bundle is loaded
const safeExposeBuiltIns = `
if (typeof require === 'function') {
globalThis._nodeRequires = {
buffer: require('buffer')
};

// prevent access to node's require function
require = null;
}
`,

bundleString = safeExposeBuiltIns + bundle.toString();

// bail out if compression is disabled
if (!this.compress) { return done(null, bundle.toString()); }
if (!this.compress) { return done(null, bundleString); }

minify(bundle.toString(), {
minify(bundleString, {
compress: {
drop_console: true // discard calls to console.* functions
},
Expand Down
7 changes: 6 additions & 1 deletion lib/environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ module.exports = {
util: { preferBuiltin: true, glob: true },
stream: { preferBuiltin: true, glob: true },
string_decoder: { preferBuiltin: true, glob: true },
buffer: { resolve: 'buffer/index.js', expose: 'buffer', glob: true },
buffer: {
resolve: '../vendor/buffer/index.js',
resolveBrowser: '../vendor/buffer/index.browser.js',
expose: 'buffer',
glob: true
},
url: { preferBuiltin: true, glob: true },
punycode: { preferBuiltin: true, glob: true },
querystring: { preferBuiltin: true, glob: true },
Expand Down
7 changes: 7 additions & 0 deletions lib/sandbox/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,13 @@
// Setup Timerz before we delete the global timers
require('./timers');

// Require buffer to make sure it's available in the sandbox
// Browserify statically analyses the usage of buffers and only
// injects Buffer into the scope if it's used. `Buffer` injected
// by browserify is part of the functional scope and does not get
// deleted when we mutate the global scope below.
require('buffer');

// Although we execute the user code in a well-defined scope using the uniscope
// module but still to cutoff the reference to the globally available properties
// we sanitize the global scope by deleting the forbidden properties in this UVM
Expand Down
10 changes: 10 additions & 0 deletions lib/vendor/buffer/buffer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
function getBufferModule (buffer) {
return {
Buffer: buffer.Buffer,
SlowBuffer: buffer.SlowBuffer,
INSPECT_MAX_BYTES: buffer.INSPECT_MAX_BYTES,
kMaxLength: buffer.kMaxLength
}
}

module.exports = getBufferModule;
8 changes: 8 additions & 0 deletions lib/vendor/buffer/index.browser.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
const getBufferModule = require('./buffer');
const SpecificBuffer = require('./specific-buffer');
const buffer = require('buffer/');

module.exports = getBufferModule({
...buffer,
Buffer: SpecificBuffer(buffer.Buffer)
})
8 changes: 8 additions & 0 deletions lib/vendor/buffer/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
const getBufferModule = require('./buffer');
const SpecificBuffer = require('./specific-buffer');
const buffer = globalThis._nodeRequires.buffer;

module.exports = getBufferModule({
...buffer,
Buffer: SpecificBuffer(globalThis.Buffer),
});
57 changes: 57 additions & 0 deletions lib/vendor/buffer/specific-buffer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
function SpecificBuffer (_Buffer) {
const Buffer = function () {
if (typeof arguments[0] === 'number') {
return _Buffer.alloc(...arguments);
}

return _Buffer.from(...arguments);
}

Buffer.poolSize = _Buffer.poolSize;

Object.defineProperty(Buffer, Symbol.hasInstance, {
value: function (instance) {
return instance instanceof _Buffer;
}
});

Buffer.isBuffer = function () {
return _Buffer.isBuffer(...arguments);
};

Buffer.alloc = function () {
return _Buffer.alloc(...arguments);
};

Buffer.allocUnsafe = function () {
return _Buffer.allocUnsafe(...arguments);
};

Buffer.allocUnsafeSlow = function () {
return _Buffer.allocUnsafeSlow(...arguments);
};

Buffer.from = function () {
return _Buffer.from(...arguments);
};

Buffer.compare = function () {
return _Buffer.compare(...arguments);
};

Buffer.isEncoding = function () {
return _Buffer.isEncoding(...arguments);
};

Buffer.concat = function () {
return _Buffer.concat(...arguments);
};

Buffer.byteLength = function () {
return _Buffer.byteLength(...arguments);
};

return Buffer;
}

module.exports = SpecificBuffer;
7 changes: 3 additions & 4 deletions npm/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,7 @@ function createBundle (options, file, done) {
},

function (codeString, next) {
// @note: we are appending "require=null;" here to avoid access to
// node's require function when running sandbox in worker_threads.
// This does not affect the require function injected by browserify.
fs.writeFile(file, `module.exports=c=>c(null,${JSON.stringify('require=null;' + codeString)})`, next);
fs.writeFile(file, `module.exports=c=>c(null,${JSON.stringify(codeString)})`, next);
},

function (next) {
Expand All @@ -48,10 +45,12 @@ module.exports = function (exit) {
async.parallel([
async.apply(createBundle, _.merge({
compress: true,
preferBrowserResolver: false,
bundler: { browserField: false }
}, options), './.cache/bootcode.js'),
async.apply(createBundle, _.merge({
compress: true,
preferBrowserResolver: true,
bundler: { browserField: true }
}, options), './.cache/bootcode.browser.js')
], function (err) {
Expand Down
Loading
Loading