-
Notifications
You must be signed in to change notification settings - Fork 775
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
Regarding Buffer #391
Comments
To get through this mess, we should create an internal function |
I agree, except should we be using |
To err on the side of caution, my preference is |
@jprichardson I'm fine with |
Here is my snipped from #380 thread: var _buffer = {
allocUnsafe: function (len) {
if (typeof Buffer.allocUnsafe === 'function') {
try {
// Node 4.4.* Buffer.allocUnsafe already exists, but throws exception in edge cases
return Buffer.allocUnsafe(len);
} catch (_error) {
return new Buffer(len);
}
}
return new Buffer(len);
}
}; |
@RyanZim good point. Let's just go with @dr-dimitru looks good! |
A little background:
new Buffer(size)
is deprecated: https://nodejs.org/api/buffer.html#buffer_new_buffer_sizeBuffer.alloc()
orBuffer.allocUnsafe()
should be used instead, but it's not supported on Node.js v4.0.0-v4.5.0.Buffer.alloc()
. This was released in fs-extra v2.1.0Now, what are we going to do moving forward? Using
new Buffer()
emits a deprecation warning in Node v7.0.0-v7.2.1.I'm wondering if we shouldn't consider some sort of fallback solution.
Also, this brings up the security question that prompted Node to change the Buffer API in the first place:
new Buffer
returns an uninitialized buffer. Currently, we are not zero-filling it. I can't see how this could be exploited in the current implementation, but it would be good to have this reviewed.If we do not need the buffer to be initialized, we should use
Buffer.allocUnsafe
, notBuffer.alloc
, in later versions.Atten: @jprichardson @manidlou @dr-dimitru
The text was updated successfully, but these errors were encountered: