zlib: fix node crashing on invalid options

The main reason behind this commit is fixing the Node process crashing
when zlib rejects the given options.

Besides that issue, which got reported and which is linked to this
commit, it turned out that Node also used to crash when a non-numeric
value was passed as the `windowBits` or the `memLevel` option. This was
fixed somewhat inadvertently; initially it was just a stylistic change
to avoid lines spanning longer than 80 characters that was written in a
manner consistent with surrounding code.

Fixes: #13082

lib/zlib.js

@@ -146,7 +146,14 @@ function zlibOnError(message, errno) {
   var error = new Error(message);
   error.errno = errno;
   error.code = codes[errno];
-  this.emit('error', error);
+
+  if (this._initSuccess) {
+    this.emit('error', error);
+  } else {
+    process.nextTick(() => {
+      this.emit('error', error);
+    });
+  }
 }
 
 function flushCallback(level, strategy, callback) {
@@ -222,20 +229,32 @@ class Zlib extends Transform {
     this._handle = new binding.Zlib(mode);
     this._handle.onerror = zlibOnError.bind(this);
     this._hadError = false;
+    this._initSuccess = false;
 
     var level = constants.Z_DEFAULT_COMPRESSION;
     if (typeof opts.level === 'number') level = opts.level;
 
     var strategy = constants.Z_DEFAULT_STRATEGY;
     if (typeof opts.strategy === 'number') strategy = opts.strategy;
 
-    this._handle.init(opts.windowBits || constants.Z_DEFAULT_WINDOWBITS,
-                      level,
-                      opts.memLevel || constants.Z_DEFAULT_MEMLEVEL,
-                      strategy,
-                      opts.dictionary);
+    var windowBits = constants.Z_DEFAULT_WINDOWBITS;
+    if (typeof opts.windowBits === 'number') windowBits = opts.windowBits;
+
+    var memLevel = constants.Z_DEFAULT_MEMLEVEL;
+    if (typeof opts.memLevel === 'number') memLevel = opts.memLevel;
+
+    this._initSuccess = this._handle.init(windowBits,
+                                          level,
+                                          memLevel,
+                                          strategy,
+                                          opts.dictionary);
+
+    if (this._initSuccess) {
+      this._buffer = Buffer.allocUnsafe(this._chunkSize);
+    } else {
+      this._buffer = null;
+    }
 
-    this._buffer = Buffer.allocUnsafe(this._chunkSize);
     this._offset = 0;
     this._level = level;
     this._strategy = strategy;
@@ -467,7 +486,9 @@ function _close(engine, callback) {
   if (!engine._handle)
     return;
 
-  engine._handle.close();
+  if (engine._initSuccess)
+    engine._handle.close();
+
   engine._handle = null;
 }
 

src/node_zlib.cc

@@ -40,6 +40,7 @@
 namespace node {
 
 using v8::Array;
+using v8::Boolean;
 using v8::Context;
 using v8::FunctionCallbackInfo;
 using v8::FunctionTemplate;
@@ -480,9 +481,11 @@ class ZCtx : public AsyncWrap {
       memcpy(dictionary, Buffer::Data(dictionary_), dictionary_len);
     }
 
-    Init(ctx, level, windowBits, memLevel, strategy,
-         dictionary, dictionary_len);
+    bool result = Init(ctx, level, windowBits, memLevel, strategy,
+                       dictionary, dictionary_len);
     SetDictionary(ctx);
+
+    args.GetReturnValue().Set(Boolean::New(args.GetIsolate(), result));
   }
 
   static void Params(const FunctionCallbackInfo<Value>& args) {
@@ -499,7 +502,7 @@ class ZCtx : public AsyncWrap {
     SetDictionary(ctx);
   }
 
-  static void Init(ZCtx *ctx, int level, int windowBits, int memLevel,
+  static bool Init(ZCtx *ctx, int level, int windowBits, int memLevel,
                    int strategy, char* dictionary, size_t dictionary_len) {
     ctx->level_ = level;
     ctx->windowBits_ = windowBits;
@@ -553,6 +556,9 @@ class ZCtx : public AsyncWrap {
 
     if (ctx->err_ != Z_OK) {
       ZCtx::Error(ctx, "Init error");
+      if (dictionary != nullptr)
+        delete[] dictionary;
+      return false;
     }
 
 
@@ -561,6 +567,8 @@ class ZCtx : public AsyncWrap {
 
     ctx->write_in_progress_ = false;
     ctx->init_done_ = true;
+
+    return true;
   }
 
   static void SetDictionary(ZCtx* ctx) {

test/parallel/test-zlib-failed-init.js

@@ -0,0 +1,15 @@
+'use strict';
+
+const common = require('../common');
+
+const assert = require('assert');
+const zlib = require('zlib');
+
+// For raw deflate or gzip encoding, a request for a 256-byte window is
+// rejected as invalid, since only zlib headers provide means of transmitting
+// the window size to the decompressor.
+// (http://zlib.net/manual.html#Advanced)
+const deflate = zlib.createDeflateRaw({ windowBits: 8 });
+deflate.on('error', common.mustCall((error) => {
+  assert.ok(/Init error/.test(error));
+}));