fix(sshpk): Upgrade request and sshpk to latest versions

[BYK]

Summary

Fixes #5477. Upgrades request ve sshpk modules to latest versions to avoid deprecated Buffer usages.

Test plan

Install new dependencies, build and run Yarn with Node 10. Make sure there are no Buffer warnings.

[BYK]

I keep seeing (node:3088) ExperimentalWarning: The fs.promises API is experimental errors now but not sure if related to this patch.

[buildsize]

File name	Previous Size	New Size	Change
yarn-[version].noarch.rpm	925.46 KB	909.36 KB	-16.1 KB (2%)
yarn-[version].js	4 MB	3.98 MB	-26.21 KB (1%)
yarn-legacy-[version].js	4.16 MB	4.14 MB	-26.21 KB (1%)
yarn-v[version].tar.gz	930.86 KB	914.12 KB	-16.74 KB (2%)
yarn_[version]all.deb	685.22 KB	667.15 KB	-18.06 KB (3%)

[simonkberg]

Are you sure this fixes #5477 completely? Since sshpk depends on asn1 which hasn't been patched yet: TritonDataCenter/node-asn1#30.

[BYK]

@simonkberg good point. I haven't seen the warning myself and that may be because the code path is not being used by yarn (or only in my tests).

I'll keep an eye on that patch too!

[jacobq]

Another check you can do is search/grep for new Buffer in the stand-alone JS. Looks like it's much improved (thanks for your work!): down to 26 matches, some of which are comments and some of which are only fallbacks for older node versions:
https://nightly.yarnpkg.com/yarn-1.10.0-20180719.1544.js

Offenders:
duplexify v3.5.0 -- fixed in 3.5.4 (43cd2692)

Details

$ yarn why duplexify
yarn why v1.9.0-20180627.0947
[1/4] Why do we have the module "duplexify"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "duplexify@3.5.0"
info Reasons this module exists
   - "gunzip-maybe#pumpify" depends on it
   - Hoisted from "gunzip-maybe#pumpify#duplexify"
   - Hoisted from "gulp-if#ternary-stream#duplexify"
   - Hoisted from "gunzip-maybe#peek-stream#duplexify"

---

ecc-jsbn v0.1.1 -- waiting on merge of PR and new release (go +1 it!)

Details

$ yarn why ecc-jsbn
yarn why v1.9.0-20180627.0947
[1/4] Why do we have the module "ecc-jsbn"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "ecc-jsbn@0.1.1"
info Reasons this module exists
   - "http-signature#sshpk" depends on it
   - Hoisted from "http-signature#sshpk#ecc-jsbn"

---

asn1 v0.2.3 -- waiting on merge of PR (go +1 it!)

Details

$ yarn why asn1
yarn why v1.9.0-20180627.0947
[1/4] Why do we have the module "asn1"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "asn1@0.2.3"
info Reasons this module exists
   - "http-signature#sshpk" depends on it
   - Hoisted from "http-signature#sshpk#asn1"

---

peek-stream v1.1.2 -- fixed in v1.1.3

Details

$ yarn why peek-stream
yarn why v1.9.0-20180627.0947
[1/4] Why do we have the module "peek-stream"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "peek-stream@1.1.2"
info Reasons this module exists
   - "gunzip-maybe" depends on it
   - Hoisted from "gunzip-maybe#peek-stream"

---

lodash v1.0.2(!) -- fixed in (549def in 2016) -- yarn needs to update it's super-old version of gulp to 4.x to fix this, I think Need to figure out what is holding this back & where it needs to be fixed. Looks like gulp#vinyl-fs#glob-watcher#gaze#globule

Details

$ yarn why lodash
yarn why v1.9.0-20180627.0947
[1/4] Why do we have the module "lodash"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "lodash@4.17.4"
info Has been hoisted to "lodash"
info Reasons this module exists
   - Hoisted from "async#lodash"
   - Hoisted from "eslint-plugin-flowtype#lodash"
   - Hoisted from "eslint#lodash"
   - Hoisted from "babel-core#lodash"
   - Hoisted from "babel-types#lodash"
   - Hoisted from "babel-template#lodash"
   - Hoisted from "babel-generator#lodash"
   - Hoisted from "babel-traverse#lodash"
   - Hoisted from "inquirer#lodash"
   - Hoisted from "eslint#table#lodash"
   - Hoisted from "babel-preset-env#babel-plugin-transform-es2015-block-scoping#lodash"
   - Hoisted from "babel-core#babel-generator#lodash"
   - Hoisted from "babel-core#babel-template#lodash"
   - Hoisted from "babel-core#babel-traverse#lodash"
   - Hoisted from "babel-plugin-transform-builtin-extend#babel-template#lodash"
   - Hoisted from "babel-core#babel-register#lodash"
   - Hoisted from "babel-core#babel-types#lodash"
   - Hoisted from "babel-eslint#babel-types#lodash"
   - Hoisted from "babel-eslint#babel-traverse#lodash"
   - Hoisted from "babel-preset-env#babel-plugin-transform-es2015-modules-commonjs#babel-types#lodash"
   - Hoisted from "babel-plugin-transform-builtin-extend#babel-template#babel-types#lodash"
   - Hoisted from "babel-preset-env#babel-plugin-transform-es2015-block-scoping#babel-template#lodash"
   - Hoisted from "babel-preset-env#babel-plugin-transform-es2015-modules-commonjs#babel-template#lodash"
   - Hoisted from "babel-preset-env#babel-plugin-transform-es2015-classes#babel-helper-define-map#lodash"
   - Hoisted from "babel-plugin-transform-builtin-extend#babel-template#babel-traverse#lodash"
   - Hoisted from "babel-preset-env#babel-plugin-transform-es2015-block-scoping#babel-traverse#lodash"
   - Hoisted from "babel-preset-env#babel-plugin-transform-es2015-block-scoping#babel-types#lodash"
   - Hoisted from "babel-preset-env#babel-plugin-transform-es2015-sticky-regex#babel-helper-regex#lodash"
   - Hoisted from "babel-preset-env#babel-plugin-transform-es2015-modules-commonjs#babel-template#babel-traverse#lodash"
   - Hoisted from "babel-preset-env#babel-plugin-transform-regenerator#regenerator-transform#babel-types#lodash"
   - Hoisted from "babel-preset-env#babel-plugin-transform-es2015-classes#babel-helper-define-map#babel-types#lodash"
info Disk size without dependencies: "4.86MB"
info Disk size with unique dependencies: "4.86MB"
info Disk size with transitive dependencies: "4.86MB"
info Number of shared dependencies: 0
=> Found "commitizen#lodash@4.17.2"
info This module exists because "commitizen" depends on it.
info Disk size without dependencies: "4.86MB"
info Disk size with unique dependencies: "4.86MB"
info Disk size with transitive dependencies: "4.86MB"
info Number of shared dependencies: 0
=> Found "commitizen#inquirer#lodash@4.17.4"
info This module exists because "commitizen#inquirer" depends on it.
info Disk size without dependencies: "4.86MB"
info Disk size with unique dependencies: "4.86MB"
info Disk size with transitive dependencies: "4.86MB"
info Number of shared dependencies: 0
=> Found "globule#lodash@1.0.2"
info This module exists because "gulp#vinyl-fs#glob-watcher#gaze#globule" depends on it.
info Disk size without dependencies: "540KB"
info Disk size with unique dependencies: "540KB"
info Disk size with transitive dependencies: "540KB"
info Number of shared dependencies: 0

[mkutny]

@jacobq , as for lodash - seems that they use gulp only as a dev dependency and it's quite recent:

yarn/package.json

Line 81 in 11eb9bb

"gulp": "^3.9.0",

Am missing something?

[jacobq]

@mkutny Oops, you're right, yarn is might be using current gulp, but there is still an ancient version of lodash (1.x) being brought in somehow, and it appears to be to be coming through gulp. It looks like it's a few levels deep, so I'm not sure exactly which upstream package needs a PR. I've updated my comment.

Update: The npm dist-tags for gulp make this a bit misleading. The "latest" version is 2 years old.

Details

[wopian]

@mkutny gulp^3.9.0 is not a recent version- it was released over 2 years ago (3.9.1). Latest is 4.0.0 back in Jan, which does not have the offending package (globule) in its chain as vinyl-fs no longer depends on glob-watcher

gulp^4 almost no longer has lodash in its dependency chain at all - just 1 dependency with lodash.debounce^4

[aduh95]

@mkutny I made a PR to upgrade to Gulp v4 (#6143) and it removes the dependency to lodash v1.0.2, so I guess @jacobq's first assumption was correct.

[jacobq]

BTW, it looks like we may still have a problem. Of the 14 string matches for new Buffer( calls in yarn-1.10.0-20180817.0955.js the first 9 and last 2 are OK.

In module "471" there are 2 cases where it is not being used as a fall-back for newer methods. I believe this code corresponds to chardet@0.4.2 (fixed in v0.7.0), which is blocked by commitizen/cz-cli#552. However, since commitizen is only a dev dependency I don't think it should be showing up in the standalone JS output.

In module "546", which I believe corresponds to node-http-signature, there is another. This is blocked until TritonDataCenter/node-http-signature#67 gets merged.

$ yarn why chardet
yarn why v1.9.4
[1/4] Why do we have the module "chardet"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "chardet@0.4.2"
info Reasons this module exists
   - "external-editor" depends on it
   - Hoisted from "external-editor#chardet"
Done in 1.77s.

$ yarn why http-signature
yarn why v1.9.4
[1/4] Why do we have the module "http-signature"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "http-signature@1.2.0"
info Reasons this module exists
   - "request" depends on it
   - Hoisted from "request#http-signature"
Done in 1.60s.