zlib: switch to lazy init for zlib streams

puzpuzpuz · MylesBorins · commit 252f37630a08 · 2020-07-27T18:09:53.000-04:00
PR-URL: #34048 Reviewed-By: Anna Henningsen <anna@addaleax.net>
diff --git a/lib/zlib.js b/lib/zlib.js
@@ -659,18 +659,13 @@ function Zlib(opts, mode) {
   // to come up with a good solution that doesn't break our internal API,
   // and with it all supported npm versions at the time of writing.
   this._writeState = new Uint32Array(2);
-  if (!handle.init(windowBits,
-                   level,
-                   memLevel,
-                   strategy,
-                   this._writeState,
-                   processCallback,
-                   dictionary)) {
-    // TODO(addaleax): Sometimes we generate better error codes in C++ land,
-    // e.g. ERR_BROTLI_PARAM_SET_FAILED -- it's hard to access them with
-    // the current bindings setup, though.
-    throw new ERR_ZLIB_INITIALIZATION_FAILED();
-  }
+  handle.init(windowBits,
+              level,
+              memLevel,
+              strategy,
+              this._writeState,
+              processCallback,
+              dictionary);
 
   ZlibBase.call(this, opts, mode, handle, zlibDefaultOpts);
 
@@ -816,6 +811,9 @@ function Brotli(opts, mode) {
     new binding.BrotliDecoder(mode) : new binding.BrotliEncoder(mode);
 
   this._writeState = new Uint32Array(2);
+  // TODO(addaleax): Sometimes we generate better error codes in C++ land,
+  // e.g. ERR_BROTLI_PARAM_SET_FAILED -- it's hard to access them with
+  // the current bindings setup, though.
   if (!handle.init(brotliInitParamsArray,
                    this._writeState,
                    processCallback)) {
diff --git a/src/node_zlib.cc b/src/node_zlib.cc
@@ -139,8 +139,8 @@ class ZlibContext : public MemoryRetainer {
   CompressionError ResetStream();
 
   // Zlib-specific:
-  CompressionError Init(int level, int window_bits, int mem_level, int strategy,
-                        std::vector<unsigned char>&& dictionary);
+  void Init(int level, int window_bits, int mem_level, int strategy,
+            std::vector<unsigned char>&& dictionary);
   void SetAllocationFunctions(alloc_func alloc, free_func free, void* opaque);
   CompressionError SetParams(int level, int strategy);
 
@@ -157,7 +157,10 @@ class ZlibContext : public MemoryRetainer {
  private:
   CompressionError ErrorForMessage(const char* message) const;
   CompressionError SetDictionary();
+  bool InitZlib();
 
+  Mutex mutex_;  // Protects zlib_init_done_.
+  bool zlib_init_done_ = false;
   int err_ = 0;
   int flush_ = 0;
   int level_ = 0;
@@ -615,13 +618,8 @@ class ZlibStream : public CompressionStream<ZlibContext> {
     AllocScope alloc_scope(wrap);
     wrap->context()->SetAllocationFunctions(
         AllocForZlib, FreeForZlib, static_cast<CompressionStream*>(wrap));
-    const CompressionError err =
-        wrap->context()->Init(level, window_bits, mem_level, strategy,
-                              std::move(dictionary));
-    if (err.IsError())
-      wrap->EmitError(err);
-
-    return args.GetReturnValue().Set(!err.IsError());
+    wrap->context()->Init(level, window_bits, mem_level, strategy,
+                          std::move(dictionary));
   }
 
   static void Params(const FunctionCallbackInfo<Value>& args) {
@@ -724,6 +722,15 @@ using BrotliEncoderStream = BrotliCompressionStream<BrotliEncoderContext>;
 using BrotliDecoderStream = BrotliCompressionStream<BrotliDecoderContext>;
 
 void ZlibContext::Close() {
+  {
+    Mutex::ScopedLock lock(mutex_);
+    if (!zlib_init_done_) {
+      dictionary_.clear();
+      mode_ = NONE;
+      return;
+    }
+  }
+
   CHECK_LE(mode_, UNZIP);
 
   int status = Z_OK;
@@ -742,6 +749,11 @@ void ZlibContext::Close() {
 
 
 void ZlibContext::DoThreadPoolWork() {
+  bool first_init_call = InitZlib();
+  if (first_init_call && err_ != Z_OK) {
+    return;
+  }
+
   const Bytef* next_expected_header_byte = nullptr;
 
   // If the avail_out is left at 0, then it means that it ran out
@@ -897,6 +909,11 @@ CompressionError ZlibContext::GetErrorInfo() const {
 
 
 CompressionError ZlibContext::ResetStream() {
+  bool first_init_call = InitZlib();
+  if (first_init_call && err_ != Z_OK) {
+    return ErrorForMessage("Failed to init stream before reset");
+  }
+
   err_ = Z_OK;
 
   switch (mode_) {
@@ -930,7 +947,7 @@ void ZlibContext::SetAllocationFunctions(alloc_func alloc,
 }
 
 
-CompressionError ZlibContext::Init(
+void ZlibContext::Init(
     int level, int window_bits, int mem_level, int strategy,
     std::vector<unsigned char>&& dictionary) {
   if (!((window_bits == 0) &&
@@ -974,6 +991,15 @@ CompressionError ZlibContext::Init(
     window_bits_ *= -1;
   }
 
+  dictionary_ = std::move(dictionary);
+}
+
+bool ZlibContext::InitZlib() {
+  Mutex::ScopedLock lock(mutex_);
+  if (zlib_init_done_) {
+    return false;
+  }
+
   switch (mode_) {
     case DEFLATE:
     case GZIP:
@@ -995,15 +1021,15 @@ CompressionError ZlibContext::Init(
       UNREACHABLE();
   }
 
-  dictionary_ = std::move(dictionary);
-
   if (err_ != Z_OK) {
     dictionary_.clear();
     mode_ = NONE;
-    return ErrorForMessage("zlib error");
+    return true;
   }
 
-  return SetDictionary();
+  SetDictionary();
+  zlib_init_done_ = true;
+  return true;
 }
 
 
@@ -1040,6 +1066,11 @@ CompressionError ZlibContext::SetDictionary() {
 
 
 CompressionError ZlibContext::SetParams(int level, int strategy) {
+  bool first_init_call = InitZlib();
+  if (first_init_call && err_ != Z_OK) {
+    return ErrorForMessage("Failed to init stream before set parameters");
+  }
+
   err_ = Z_OK;
 
   switch (mode_) {
diff --git a/test/parallel/test-zlib-reset-before-write.js b/test/parallel/test-zlib-reset-before-write.js
@@ -0,0 +1,37 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const zlib = require('zlib');
+
+// Tests that zlib streams support .reset() and .params()
+// before the first write. That is important to ensure that
+// lazy init of zlib native library handles these cases.
+
+for (const fn of [
+  (z, cb) => {
+    z.reset();
+    cb();
+  },
+  (z, cb) => z.params(0, zlib.constants.Z_DEFAULT_STRATEGY, cb)
+]) {
+  const deflate = zlib.createDeflate();
+  const inflate = zlib.createInflate();
+
+  deflate.pipe(inflate);
+
+  const output = [];
+  inflate
+    .on('error', (err) => {
+      assert.ifError(err);
+    })
+    .on('data', (chunk) => output.push(chunk))
+    .on('end', common.mustCall(
+      () => assert.deepStrictEqual(Buffer.concat(output).toString(), 'abc')));
+
+  fn(deflate, () => {
+    fn(inflate, () => {
+      deflate.write('abc');
+      deflate.end();
+    });
+  });
+}
