stream,zlib: do not use _stream_* anymore. by mcollina · Pull Request #36618 · nodejs/node · GitHub

In two place we still used _stream_*, causing #36615 and potentially other issues.

Note that #36615 does not impact master for some independent reason.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

CI: https://ci.nodejs.org/job/node-test-pull-request/35065/

@nodejs/releasers @nodejs/tsc this should go to into one of the next v14 releases.

LGTM.

Do we need to add a regression test to catch the breakage (I'm assuming no existing test caught #36615)?

@richardlau I've added a test

@mcollina can you fix the lint error?

@targos done

CI: https://ci.nodejs.org/job/node-test-pull-request/35103/

CI: https://ci.nodejs.org/job/node-test-pull-request/35110/

Commit Queue failed

- Loading data for nodejs/node/pull/36618
✔  Done loading data for nodejs/node/pull/36618
----------------------------------- PR info ------------------------------------
Title      stream,zlib: do not use _stream_* anymore. (#36618)
Author     Matteo Collina  (@mcollina)
Branch     mcollina:always-use-stream -> nodejs:master
Labels     dont-land-on-v10.x, dont-land-on-v12.x, lts-watch-v14.x, zlib
Commits    1
 - stream,zlib: do not use _stream_* anymore.
Committers 1
 - Matteo Collina 
PR-URL: https://github.com/nodejs/node/pull/36618
Reviewed-By: Robert Nagy 
Reviewed-By: Rich Trott 
Reviewed-By: Richard Lau 
Reviewed-By: Luigi Pinca 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Michaël Zasso 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/36618
Reviewed-By: Robert Nagy 
Reviewed-By: Rich Trott 
Reviewed-By: Richard Lau 
Reviewed-By: Luigi Pinca 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Michaël Zasso 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - stream,zlib: do not use _stream_* anymore.
   ✔  Last GitHub Actions successful
   ℹ  Last Full PR CI on 2020-12-28T17:53:03Z: https://ci.nodejs.org/job/node-test-pull-request/35126/
- Querying data for job/node-test-pull-request/35126/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
   ℹ  This PR was created on Thu, 24 Dec 2020 15:29:15 GMT
   ✔  Approvals: 6
   ✔  - Robert Nagy (@ronag): https://github.com/nodejs/node/pull/36618#pullrequestreview-558672980
   ✔  - Rich Trott (@Trott) (TSC): https://github.com/nodejs/node/pull/36618#pullrequestreview-558684130
   ✔  - Richard Lau (@richardlau): https://github.com/nodejs/node/pull/36618#pullrequestreview-558965185
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/36618#pullrequestreview-558695858
   ✔  - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/36618#pullrequestreview-558712700
   ✔  - Michaël Zasso (@targos) (TSC): https://github.com/nodejs/node/pull/36618#pullrequestreview-559085773
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/449376978

May I get a last approval @targos?

Commit Queue failed

- Loading data for nodejs/node/pull/36618 ✔ Done loading data for nodejs/node/pull/36618 ----------------------------------- PR info ------------------------------------ Title stream,zlib: do not use _stream_* anymore. (#36618) Author Matteo Collina (@mcollina) Branch mcollina:always-use-stream -> nodejs:master Labels dont-land-on-v10.x, dont-land-on-v12.x, lts-watch-v14.x, zlib Commits 1 - stream,zlib: do not use _stream_* anymore. Committers 1 - Matteo Collina PR-URL: https://github.com/nodejs/node/pull/36618 Reviewed-By: Robert Nagy Reviewed-By: Rich Trott Reviewed-By: Richard Lau Reviewed-By: Luigi Pinca Reviewed-By: Benjamin Gruenbaum Reviewed-By: Michaël Zasso ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/36618 Reviewed-By: Robert Nagy Reviewed-By: Rich Trott Reviewed-By: Richard Lau Reviewed-By: Luigi Pinca Reviewed-By: Benjamin Gruenbaum Reviewed-By: Michaël Zasso -------------------------------------------------------------------------------- ✔ Last GitHub Actions successful ℹ Last Full PR CI on 2020-12-28T19:16:44Z: https://ci.nodejs.org/job/node-test-pull-request/35126/ - Querying data for job/node-test-pull-request/35126/ ✔ Build data downloaded ✔ Last Jenkins CI successful ℹ This PR was created on Thu, 24 Dec 2020 15:29:15 GMT ✔ Approvals: 6 ✔ - Robert Nagy (@ronag): https://github.com/nodejs/node/pull/36618#pullrequestreview-558672980 ✔ - Rich Trott (@Trott) (TSC): https://github.com/nodejs/node/pull/36618#pullrequestreview-558684130 ✔ - Richard Lau (@richardlau): https://github.com/nodejs/node/pull/36618#pullrequestreview-558965185 ✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/36618#pullrequestreview-558695858 ✔ - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/36618#pullrequestreview-558712700 ✔ - Michaël Zasso (@targos) (TSC): https://github.com/nodejs/node/pull/36618#pullrequestreview-559319170 -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/master up to date... From https://github.com/nodejs/node * branch master -> FETCH_HEAD ✔ origin/master is now up-to-date - Downloading patch for 36618 From https://github.com/nodejs/node * branch refs/pull/36618/merge -> FETCH_HEAD ✔ Fetched commits as 053874939a60..be22d247cfc8 -------------------------------------------------------------------------------- [master ec57bf8288] stream,zlib: do not use _stream_* anymore. Author: Matteo Collina Date: Thu Dec 24 16:25:53 2020 +0100 3 files changed, 16 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-zlib-no-stream.js ✔ Patches applied -------------------------------------------------------------------------------- --------------------------------- New Message ---------------------------------- stream,zlib: do not use _stream_* anymore.

PR-URL: #36618
Reviewed-By: Robert Nagy ronagy@icloud.com
Reviewed-By: Rich Trott rtrott@gmail.com
Reviewed-By: Richard Lau rlau@redhat.com
Reviewed-By: Luigi Pinca luigipinca@gmail.com
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Michaël Zasso targos@protonmail.com

[master 83c0916250] stream,zlib: do not use *stream** anymore.
Author: Matteo Collina hello@matteocollina.com
Date: Thu Dec 24 16:25:53 2020 +0100
3 files changed, 16 insertions(+), 3 deletions(-)
create mode 100644 test/parallel/test-zlib-no-stream.js
✖ 83c0916250fb078d45eff18e06b7159eff4f69e4
✔ 0:0 skipping fixes-url fixes-url
✔ 0:0 blank line after title line-after-title
✔ 0:0 line-lengths are valid line-length
✔ 0:0 metadata is at end of message metadata-end
✔ 1:8 PR-URL is valid. pr-url
✔ 0:0 reviewers are valid reviewers
✔ 0:0 valid subsystems subsystem
✖ 0:42 Do not use punctuation at end of title. title-format
✔ 0:0 Title is <= 50 columns. title-length

ℹ Please fix the commit message and try again.

https://github.com/nodejs/node/actions/runs/449646494

PR-URL: nodejs#36618 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>

Landed in e57d8af

PR-URL: #36618 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>

Notable changes: - **deps**: upgrade npm to 6.14.11 (Darcy Clarke) [#36838](#36838) - **stream,zlib**: do not use \_stream\_\* anymore (Matteo Collina) [#36618](#36618) PR-URL: #37074

PR-URL: #36618 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>

Notable changes: - **deps**: upgrade npm to 6.14.11 (Ruy Adorno) (#37173) - **stream,zlib**: do not use \_stream\_\* anymore (Matteo Collina) (#36618) PR-URL: #37074

Notable changes: - **deps**: - upgrade npm to 6.14.11 (Ruy Adorno) (#37173) - V8: backport dfcf1e86fac0 (Michaël Zasso) (#37245) - Note: Node.js is not believed to be vulnerable to CVE-2021-21148. - **stream,zlib**: do not use \_stream\_\* anymore (Matteo Collina) (#36618) PR-URL: #37074

mcollina requested a review from ronag December 24, 2020 15:29

nodejs-github-bot added the zlib Issues and PRs related to the zlib subsystem. label Dec 24, 2020

mcollina added dont-land-on-v10.x request-ci Add this label to start a Jenkins CI on a PR. labels Dec 24, 2020

This was referenced Dec 24, 2020

stream: move to internal/streams #35239

Closed

gunzipSync DOA in Node 14.15.2 #36615

Closed

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 24, 2020

ronag approved these changes Dec 24, 2020

View reviewed changes

Trott approved these changes Dec 24, 2020

View reviewed changes

richardlau approved these changes Dec 24, 2020

View reviewed changes

lpinca approved these changes Dec 24, 2020

View reviewed changes

benjamingr approved these changes Dec 24, 2020

View reviewed changes

mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 27, 2020

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 27, 2020

richardlau approved these changes Dec 27, 2020

View reviewed changes

mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 28, 2020

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 28, 2020

targos approved these changes Dec 28, 2020

View reviewed changes

Comment thread test/parallel/test-zlib-no-stream.js Outdated

targos linked an issue Dec 28, 2020 that may be closed by this pull request

gunzipSync DOA in Node 14.15.2 #36615

Closed

mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 28, 2020

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 28, 2020

targos approved these changes Dec 28, 2020

View reviewed changes

mcollina added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Dec 28, 2020

github-actions Bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Dec 28, 2020

aduh95 force-pushed the always-use-stream branch from be22d24 to e57d8af Compare December 29, 2020 00:48

aduh95 merged commit e57d8af into nodejs:master Dec 29, 2020

danielleadams mentioned this pull request Jan 12, 2021

v15.6.0 proposal #36889

Merged

richardlau mentioned this pull request Jan 22, 2021

zlib deflateraw crash in readable stream in 14.15.2+ #37021

Closed

This was referenced Jan 22, 2021

DeflateCRC32Stream is broken with NodeJS 15.6.0 archiverjs/node-crc32-stream#31

Closed

Node v15.6.0 changes the behavior of .end on zlib.DeflateRaw #37027

Closed

BethGriggs removed the lts-watch-v14.x label Jan 26, 2021

BethGriggs mentioned this pull request Jan 26, 2021

v14.15.5 proposal #37074

Merged

BethGriggs added a commit that referenced this pull request Feb 5, 2021

2021-02-08, Version 14.15.5 'Fermium' (LTS)

a1b41b3

Notable changes: - **deps**: upgrade npm to 6.14.11 (Ruy Adorno) (#37173) - **stream,zlib**: do not use \_stream\_\* anymore (Matteo Collina) (#36618) PR-URL: #37074

RoyHP mentioned this pull request Feb 10, 2021

Node silently exits with code 0, no error, when using a specific string for String.includes inside worker piscinajs/piscina#107

Closed

targos removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Sep 5, 2021

stream,zlib: do not use _stream_* anymore. by mcollina · Pull Request #36618 · nodejs/node · GitHub

Uh oh!

Uh oh!

stream,zlib: do not use _stream_* anymore. - #36618

stream,zlib: do not use _stream_* anymore.#36618
aduh95 merged 1 commit into
nodejs:masterfrom
mcollina:always-use-stream

mcollina commented Dec 24, 2020

Uh oh!

nodejs-github-bot commented Dec 24, 2020

Uh oh!

mcollina commented Dec 24, 2020

Uh oh!

richardlau left a comment

Uh oh!

mcollina commented Dec 26, 2020

Uh oh!

targos commented Dec 27, 2020

Uh oh!

mcollina commented Dec 27, 2020

Uh oh!

nodejs-github-bot commented Dec 27, 2020

Uh oh!

nodejs-github-bot commented Dec 28, 2020

Uh oh!

Uh oh!

github-actions Bot commented Dec 28, 2020

Uh oh!

mcollina commented Dec 28, 2020

Uh oh!

github-actions Bot commented Dec 28, 2020

Uh oh!

aduh95 commented Dec 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

stream,zlib: do not use _stream_* anymore. by mcollina · Pull Request #36618 · nodejs/node · GitHub

Uh oh!

Uh oh!

stream,zlib: do not use _stream_* anymore. - #36618

stream,zlib: do not use _stream_* anymore.#36618aduh95 merged 1 commit intonodejs:masternodejs/node:masterfrom mcollina:always-use-streammcollina/node:always-use-streamCopy head branch name to clipboard

Conversation

mcollina commented Dec 24, 2020

Checklist

Uh oh!

nodejs-github-bot commented Dec 24, 2020

Uh oh!

mcollina commented Dec 24, 2020

Uh oh!

richardlau left a comment

Choose a reason for hiding this comment

Uh oh!

mcollina commented Dec 26, 2020

Uh oh!

targos commented Dec 27, 2020

Uh oh!

mcollina commented Dec 27, 2020

Uh oh!

nodejs-github-bot commented Dec 27, 2020

Uh oh!

nodejs-github-bot commented Dec 28, 2020

Uh oh!

Uh oh!

github-actions Bot commented Dec 28, 2020

Uh oh!

mcollina commented Dec 28, 2020

Uh oh!

github-actions Bot commented Dec 28, 2020

PR-URL: #36618 Reviewed-By: Robert Nagy ronagy@icloud.com Reviewed-By: Rich Trott rtrott@gmail.com Reviewed-By: Richard Lau rlau@redhat.com Reviewed-By: Luigi Pinca luigipinca@gmail.com Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com Reviewed-By: Michaël Zasso targos@protonmail.com

Uh oh!

aduh95 commented Dec 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

stream,zlib: do not use _stream_* anymore.#36618
aduh95 merged 1 commit into
nodejs:masterfrom
mcollina:always-use-stream

PR-URL: #36618
Reviewed-By: Robert Nagy ronagy@icloud.com
Reviewed-By: Rich Trott rtrott@gmail.com
Reviewed-By: Richard Lau rlau@redhat.com
Reviewed-By: Luigi Pinca luigipinca@gmail.com
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Michaël Zasso targos@protonmail.com