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

Fix deprecated Buffer constructor usage and add safeguards #506

Merged
merged 2 commits into from
Feb 21, 2019

Conversation

ChALkeR
Copy link
Contributor

@ChALkeR ChALkeR commented Feb 18, 2018

Two commits here, one for moving off the deprecated Buffer constructor API, and the second for the additional safeguards (just in case).

nodejsUtils: don't use deprecated Buffer api

This avoids using Buffer constructor API on newer Node.js versions.

To achieve that, Buffer.from presence is checked, with validating that it's
not the same method as Uint8Array.from.

Refs:
https://nodejs.org/api/deprecations.html#deprecations_dep0005_buffer_constructor
nodejsUtils: additional Buffer safeguards for older Node.js

1. In allocBuffer(), ensure buffers allocated with new Buffer() are zero-filled.
   On newer Node.js versions, Buffer.alloc() zero-fills already.

2. In newBufferFrom(), throw when 'data' argument is a number.
   On newer Node.js versions, Buffer.from(number)/new Buffer(number, encooding)
   throw already.

This avoids using Buffer constructor API on newer Node.js versions.

To achieve that, Buffer.from presence is checked, with validating that it's
not the same method as Uint8Array.from.

Refs:
https://nodejs.org/api/deprecations.html#deprecations_dep0005_buffer_constructor
1. In allocBuffer(), ensure buffers allocated with new Buffer() are zero-filled.
   On newer Node.js versions, Buffer.alloc() zero-fills already.

2. In newBufferFrom(), throw when 'data' argument is a number.
   On newer Node.js versions, Buffer.from(number)/new Buffer(number, encooding)
   throw already.
@ChALkeR
Copy link
Contributor Author

ChALkeR commented Mar 2, 2018

Tracking: nodejs/node#19079

@arcanis
Copy link

arcanis commented Sep 30, 2018

Ping @dduponchel?

@zoe-mcelderry
Copy link

Is there any time line for addressing this issue please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants