-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
(v6.x backport) zlib: fix node crashing on invalid options #13201
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -379,9 +379,15 @@ function Zlib(opts, mode) { | |
var strategy = exports.Z_DEFAULT_STRATEGY; | ||
if (typeof opts.strategy === 'number') strategy = opts.strategy; | ||
|
||
this._handle.init(opts.windowBits || exports.Z_DEFAULT_WINDOWBITS, | ||
var windowBits = exports.Z_DEFAULT_WINDOWBITS; | ||
if (typeof opts.windowBits === 'number') windowBits = opts.windowBits; | ||
|
||
var memLevel = exports.Z_DEFAULT_MEMLEVEL; | ||
if (typeof opts.memLevel === 'number') memLevel = opts.memLevel; | ||
|
||
this._handle.init(windowBits || exports.Z_DEFAULT_WINDOWBITS, | ||
level, | ||
opts.memLevel || exports.Z_DEFAULT_MEMLEVEL, | ||
memLevel || exports.Z_DEFAULT_MEMLEVEL, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto |
||
strategy, | ||
opts.dictionary); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -530,16 +530,19 @@ class ZCtx : public AsyncWrap { | |
CHECK(0 && "wtf?"); | ||
} | ||
|
||
if (ctx->err_ != Z_OK) { | ||
ZCtx::Error(ctx, "Init error"); | ||
} | ||
|
||
|
||
ctx->dictionary_ = reinterpret_cast<Bytef *>(dictionary); | ||
ctx->dictionary_len_ = dictionary_len; | ||
|
||
ctx->write_in_progress_ = false; | ||
ctx->init_done_ = true; | ||
|
||
if (ctx->err_ != Z_OK) { | ||
if (dictionary != nullptr) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fwiw, you don't need the conditional, the following two lines of code can run correctly whether dictionary is nullptr or not. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, nice. Thanks. |
||
delete[] dictionary; | ||
ctx->dictionary_ = nullptr; | ||
} | ||
ctx->env()->ThrowError("Init error"); | ||
} | ||
} | ||
|
||
static void SetDictionary(ZCtx* ctx) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
'use strict'; | ||
|
||
require('../common'); | ||
|
||
const assert = require('assert'); | ||
const zlib = require('zlib'); | ||
|
||
// For raw deflate encoding, requests for 256-byte windows are rejected as | ||
// invalid by zlib. | ||
// (http://zlib.net/manual.html#Advanced) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Dismissed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a good idea to change this one since that's how it is already committed to master. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ack. |
||
assert.throws(() => { | ||
zlib.createDeflateRaw({ windowBits: 8 }); | ||
}, /^Error: Init error$/); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If your changing this line anyway, IMHO handle the fallback in 383 (
opts.windowBits && typeof...
)