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

Replace use of deprecated/unsafe new Buffer constructor #5477

Closed
johnmuhl opened this issue Mar 7, 2018 · 84 comments
Closed

Replace use of deprecated/unsafe new Buffer constructor #5477

johnmuhl opened this issue Mar 7, 2018 · 84 comments

Comments

@johnmuhl
Copy link

@johnmuhl johnmuhl commented Mar 7, 2018

What is the current behavior?

yarn uses deprecated new Buffer() constructor and causes deprecation warnings when run with NODE_PENDING_DEPRECATION=1.

$ ag '\bBuffer\('
src/registries/npm-registry.js
340:        const pw = new Buffer(String(password), 'base64').toString();
341:        return 'Basic ' + new Buffer(String(username) + ':' + pw).toString('base64');

src/util/fs.js
835:const cr = new Buffer('\r', 'utf8')[0];
836:const lf = new Buffer('\n', 'utf8')[0];

What is the expected behavior?

yarn should not use deprecated/unsafe Buffer constructor. According to the deprecation warning new Buffer() should be replaced with one of Buffer.alloc(), Buffer.allocUnsafe() or Buffer.from(); the safe-buffer package is another option.

Please mention your node.js, yarn and operating system version.

$ node -v
v8.9.4

$ yarn -v
1.5.1

$ uname -a
Linux 4.15.6-300.fc27.x86_64 #1 SMP Mon Feb 26 18:43:03 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
@malonehedges
Copy link

@malonehedges malonehedges commented Apr 24, 2018

Getting this same warning on macOS. I believe #5704 is a duplicate of this.

@BYK BYK closed this in #5731 Apr 26, 2018
BYK added a commit that referenced this issue Apr 26, 2018
**Summary**

Fixes #5477 and #5704.

Remove usager of deprecated `Buffer`constructor to avoid ugly warnings on Node 10.

**Test plan**

Existing test should pass.
@felixrabe
Copy link

@felixrabe felixrabe commented Apr 26, 2018

As a workaround for now, I've created a ~/bin/node script that comes before (Homebrew-installed) /usr/local/bin/node in my $PATH:

#!/bin/bash

/usr/local/bin/node --no-deprecation "$@"
@marvinhagemeister
Copy link

@marvinhagemeister marvinhagemeister commented Apr 28, 2018

FYI: Just went through yarn's dependencies and there are two which still call new Buffer():

  • tar-stream (fixed, but we still depend on an older version)
  • v8-compile-cache (not fixed)

We will likely need to update those.

@jjhesk

This comment was marked as disruptive content.

@BYK BYK reopened this Apr 28, 2018
BYK added a commit that referenced this issue Apr 28, 2018
Another step towards fixing #5477.
@BYK BYK assigned BYK and unassigned kaylie-alexa Apr 28, 2018
BYK added a commit that referenced this issue Apr 28, 2018
…5752)

**Summary**

Another step towards fixing #5477.

**Test plan**

All existing tests should pass.
BYK added a commit that referenced this issue Apr 30, 2018
**Summary**
These are a few upgrades of dependencies in order to address issues on node 10 (in relation to: #5477)

*duplexify* *3.5.0* => *3.5.4* (mafintosh/duplexify@00d08fa)
*v8-compile-cache* *^1.1.0* => *^2.0.0* (zertosh/v8-compile-cache#7)
*natives* *1.1.0* => *1.1.3* (gulpjs/gulp#2162)

The last did not have to do with `new Buffer` but was something I personally encountered when using `gulp` for building in `node 10`. I don't know how common this is, but the fix is easy and within scope, so decided to include it.


**Test plan**

CI should be green.
@jacobq
Copy link

@jacobq jacobq commented Aug 10, 2018

@vrobinson Yes, they should be fixed by #6208 -- You can see this by searching the standalone JS for new Buffer and observing that it only appears in fallbacks and comments. Have patience 😸

Update: Looks like it just landed 🎉 so watch for the next nightly. To everyone who helped make this happen, 🙇‍♂️ thank you 🙏!

@rof20004
Copy link

@rof20004 rof20004 commented Aug 10, 2018

I got this error, was this solved?

@jacobq
Copy link

@jacobq jacobq commented Aug 10, 2018

@rof20004 ☝️(right above your comment). PR has merged, so it is now "fixed" on the master branch of the source code. However, you'll continue to see this warning with Node 10 until (1) a new version is released and (2) you upgrade to that version.

@rof20004
Copy link

@rof20004 rof20004 commented Aug 10, 2018

@jacobq I am using debian package, I need to wait until the new package then '-'.

Thanks :)

@jacobq
Copy link

@jacobq jacobq commented Aug 10, 2018

@rof20004 You should be able to start using it tomorrow, if you want, just make sure you have apt pointing to the nightly:
https://yarnpkg.com/en/docs/install#debian-nightly

@damianobarbati
Copy link

@damianobarbati damianobarbati commented Aug 23, 2018

@jacobq so we're going to see this finally issue finally resolved once for all with yarn@1.9.5, correct?

@arcanis
Copy link
Member

@arcanis arcanis commented Aug 23, 2018

Probably won't be a patch release, more like 1.10

@josuedjh3
Copy link

@josuedjh3 josuedjh3 commented Sep 12, 2018

I have the same problem.
$ yarn install
node:39) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead. error An unexpected error occurred: "https://registry.yarnpkg.com/axios/-/axios-0.18.0.tgz: getaddrinfo EADDRNOTAVAIL registry.yarnpkg.com registry.yarnpkg.com:443". info If you think this is a bug, please open a bug report with the information provided in "/app/yarn-error.log". info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
/app $ node -v v10.10.0 /app $ yarn -v 1.9.4 /app $

@josuedjh3
Copy link

@josuedjh3 josuedjh3 commented Sep 12, 2018

someone has the solution?

@kbd
Copy link

@kbd kbd commented Sep 12, 2018

@codestart123 read the thread, they said it's fixed in nightly but not released yet.

@jacobq
Copy link

@jacobq jacobq commented Sep 17, 2018

@codestart123 The error you mentioned is not caused by this issue (you can ignore the deprecation warning):

error An unexpected error occurred: "https://registry.yarnpkg.com/axios/-/axios-0.18.0.tgz: getaddrinfo EADDRNOTAVAIL registry.yarnpkg.com registry.yarnpkg.com:443"

EADDRNOTAVAIL is a network-related error (error: address not available). Try running ping registry.yarnpkg.com to confirm that your system can resolve the name and reach the host.

wchargin added a commit to sourcecred/sourcecred that referenced this issue Sep 20, 2018
Summary:
SourceCred officially supports Node v8.x.x, so Travis should be testing
against that version of Node.

This fixes a cron-only, Travis-only failure. The test for the build
script asserts that stderr only contains some known expected messages.
On Node v10, which Travis was using, there is an additional deprecation
message due to <yarnpkg/yarn#5477>.

Test Plan:
To see that this fixes the current cron-only build failure, add the
`--full` argument to the `test` script in `package.json`, and push the
resulting tree to Travis. Note that it runs `sharness-full`, which
passes.

wchargin-branch: travis-node-8
wchargin added a commit to sourcecred/sourcecred that referenced this issue Sep 20, 2018
Summary:
SourceCred officially supports Node v8.x.x, so Travis should be testing
against that version of Node.

This fixes a cron-only, Travis-only failure. The test for the build
script asserts that stderr only contains some known expected messages.
On Node v10, which Travis was using, there is an additional deprecation
message due to <yarnpkg/yarn#5477>.

Test Plan:
To see that this fixes the current cron-only build failure, add the
`--full` argument to the `test` script in `package.json`, and push the
resulting tree to Travis. Note that it runs `sharness-full`, which
passes.

wchargin-branch: travis-node-8
wchargin added a commit to sourcecred/sourcecred that referenced this issue Sep 20, 2018
Summary:
SourceCred officially supports Node v8.x.x, so Travis should be testing
against that version of Node.

This fixes a cron-only, Travis-only failure. The test for the build
script asserts that stderr only contains some known expected messages.
On Node v10, which Travis was using, there is an additional deprecation
message due to <yarnpkg/yarn#5477>.

Test Plan:
To see that this fixes the current cron-only build failure, add the
`--full` argument to the `test` script in `package.json`, and push the
resulting tree to Travis. Note that it runs `sharness-full`, which
passes.

wchargin-branch: travis-node-8
@RyanZhu1024
Copy link

@RyanZhu1024 RyanZhu1024 commented Sep 28, 2018

Has it been fixed yet?

@wopian
Copy link

@wopian wopian commented Sep 28, 2018

@RyanZhu1024 it was fixed in 1.10.0, released a few days ago 👍

@iofu728
Copy link

@iofu728 iofu728 commented Sep 29, 2018

Waiting node upgrade yarn version.

@vladshcherbin
Copy link

@vladshcherbin vladshcherbin commented Sep 29, 2018

Can't believe this annoying warning is finally gone. Thank you :)

Hooraaaay 🙌 🎉 🍾

@jacobq
Copy link

@jacobq jacobq commented Nov 8, 2018

@goktugyil The question you linked does not make any mention of yarn. If you are using yarn, please make sure you have the latest version (currently 1.12.3). If you are not using yarn please do not cross-post here. StackOverflow is a good place for asking general programming questions, and https://github.com/nodejs/help is a good place to go for Node questions. https://github.com/yarnpkg/yarn/issues is for feature requests and problems related to yarn (not for general development support/questions).

@arcanis / @BYK / @imsnif Would you mind locking this thread? This problem has been fixed in yarn and its dependencies for a while now, and I don't think further discussion here will benefit anyone.

@yarnpkg yarnpkg locked as resolved and limited conversation to collaborators Nov 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.