Skip to content

(v4.x-backport) zlib: fix crash when initializing failed - #14860

Closed
aqrln wants to merge 2 commits into
nodejs:v4.x-stagingfrom
aqrln:v4.x-zlib-failed-init
Closed

(v4.x-backport) zlib: fix crash when initializing failed#14860
aqrln wants to merge 2 commits into
nodejs:v4.x-stagingfrom
aqrln:v4.x-zlib-failed-init

Conversation

@aqrln

Copy link
Copy Markdown
Contributor

First commit is a partial backport of semver-patch bits of #13098, cherry-picked from v6.x-staging (321c90f, #13201). Second one is a backport of #14666.

/cc @MylesBorins

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

zlib

aqrlnand others added 2 commits August 16, 2017 17:31
This is a partial backport of semver-patch bits of
9e4660b.
This commit fixes the Node process crashing when constructors of classes
of the zlib module are given invalid options.
* Throw an Error when the zlib library rejects the value of windowBits,
instead of crashing with an assertion.
* Treat windowBits and memLevel options consistently with other ones and
don't crash when non-numeric values are given.
PR-URL: nodejs#13098Fixes: nodejs#13082
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Unset `mode_` when initializing the zlib stream failed, so that
we don’t try to call the zlib end functions (`deflateEnd()` etc.)
when cleaning up in `ZCtx::Close()`.
Fixes: nodejs#14178
Ref: nodejs#13098
PR-URL: nodejs#14666
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
@aqrlnaqrln added v4.x zlib Issues and PRs related to the zlib subsystem. labels Aug 16, 2017
@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. v4.x zlib Issues and PRs related to the zlib subsystem. labels Aug 16, 2017
@aqrlnaqrln mentioned this pull request Aug 16, 2017
2 tasks
MylesBorins pushed a commit that referenced this pull request Sep 19, 2017
This is a partial backport of semver-patch bits of
9e4660b.
This commit fixes the Node process crashing when constructors of classes
of the zlib module are given invalid options.
* Throw an Error when the zlib library rejects the value of windowBits,
instead of crashing with an assertion.
* Treat windowBits and memLevel options consistently with other ones and
don't crash when non-numeric values are given.
Backport-PR-URL: #14860
PR-URL: #13098Fixes: #13082
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this pull request Sep 19, 2017
Unset `mode_` when initializing the zlib stream failed, so that
we don’t try to call the zlib end functions (`deflateEnd()` etc.)
when cleaning up in `ZCtx::Close()`.
Fixes: #14178
Ref: #13098
Backport-PR-URL: #14860
PR-URL: #14666
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
@MylesBorins

Copy link
Copy Markdown
Contributor

landed in 273e02d...5422eb8

MylesBorins pushed a commit that referenced this pull request Oct 25, 2017
This is a partial backport of semver-patch bits of
9e4660b.
This commit fixes the Node process crashing when constructors of classes
of the zlib module are given invalid options.
* Throw an Error when the zlib library rejects the value of windowBits,
instead of crashing with an assertion.
* Treat windowBits and memLevel options consistently with other ones and
don't crash when non-numeric values are given.
Backport-PR-URL: #14860
PR-URL: #13098Fixes: #13082
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this pull request Oct 25, 2017
Unset `mode_` when initializing the zlib stream failed, so that
we don’t try to call the zlib end functions (`deflateEnd()` etc.)
when cleaning up in `ZCtx::Close()`.
Fixes: #14178
Ref: #13098
Backport-PR-URL: #14860
PR-URL: #14666
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
@MylesBorins

Copy link
Copy Markdown
Contributor

I ended up modifying the first commit due to the sec fix

please review

31bf595

@MylesBorins

Copy link
Copy Markdown
Contributor

@nodejs/tsc @nodejs/lts
This PR is slated to land in the next v4.x maintenance release. Please let me know if you see any potential issues

@mcollina

Copy link
Copy Markdown
Member

Ok for me.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++Issues and PRs that require attention from people who are familiar with C++.zlibIssues and PRs related to the zlib subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@aqrln@MylesBorins@mcollina@nodejs-github-bot@addaleax