zlib: Manage flush flags appropriately

If you call z.flush();z.write('foo'); then it would try to write 'foo'
before the flush was done, triggering an assertion in the zlib binding.

Closes #4950

3 files changed, 93 insertions, 17 deletions

diff --git a/doc/api/zlib.markdown b/doc/api/zlib.markdown
index 5dcde3248..6a194b9e1 100644
--- a/doc/api/zlib.markdown
+++ b/doc/api/zlib.markdown
@@ -228,9 +228,10 @@ Decompress a raw Buffer with Unzip.
 Each class takes an options object.  All options are optional.  (The
 convenience methods use the default settings for all options.)
 
-Note that some options are only
-relevant when compressing, and are ignored by the decompression classes.
+Note that some options are only relevant when compressing, and are
+ignored by the decompression classes.
 
+* flush (default: `zlib.Z_NO_FLUSH`)
 * chunkSize (default: 16*1024)
 * windowBits
 * level (compression only)
@@ -238,8 +239,8 @@ relevant when compressing, and are ignored by the decompression classes.
 * strategy (compression only)
 * dictionary (deflate/inflate only, empty dictionary by default)
 
-See the description of `deflateInit2` and `inflateInit2`
-at <http://zlib.net/manual.html#Advanced> for more information on these.
+See the description of `deflateInit2` and `inflateInit2` at
+<http://zlib.net/manual.html#Advanced> for more information on these.
 
 ## Memory Usage Tuning
 
diff --git a/lib/zlib.js b/lib/zlib.js
index dc0aeca30..c9263142d 100644
--- a/lib/zlib.js
+++ b/lib/zlib.js
@@ -226,8 +226,17 @@ function Zlib(opts, mode) {
 
   Transform.call(this, opts);
 
-  // means a different thing there.
-  this._readableState.chunkSize = null;
+  if (opts.flush) {
+    if (opts.flush !== binding.Z_NO_FLUSH &&
+        opts.flush !== binding.Z_PARTIAL_FLUSH &&
+        opts.flush !== binding.Z_SYNC_FLUSH &&
+        opts.flush !== binding.Z_FULL_FLUSH &&
+        opts.flush !== binding.Z_FINISH &&
+        opts.flush !== binding.Z_BLOCK) {
+      throw new Error('Invalid flush flag: ' + opts.flush);
+    }
+  }
+  this._flushFlag = opts.flush || binding.Z_NO_FLUSH;
 
   if (opts.chunkSize) {
     if (opts.chunkSize < exports.Z_MIN_CHUNK ||
@@ -308,22 +317,30 @@ Zlib.prototype.reset = function reset() {
   return this._binding.reset();
 };
 
+// This is the _flush function called by the transform class,
+// internally, when the last chunk has been written.
 Zlib.prototype._flush = function(callback) {
-  this._transform(null, '', callback);
+  this._transform(new Buffer(0), '', callback);
 };
 
 Zlib.prototype.flush = function(callback) {
   var ws = this._writableState;
-  var ts = this._transformState;
 
-  if (ws.writing) {
-    ws.needDrain = true;
+  if (ws.ended) {
+    if (callback)
+      process.nextTick(callback);
+  } else if (ws.ending) {
+    if (callback)
+      this.once('end', callback);
+  } else if (ws.needDrain) {
     var self = this;
     this.once('drain', function() {
-      self._flush(callback);
+      self.flush(callback);
     });
-  } else
-    this._flush(callback || function() {});
+  } else {
+    this._flushFlag = binding.Z_FULL_FLUSH;
+    this.write(new Buffer(0), '', callback);
+  }
 };
 
 Zlib.prototype.close = function(callback) {
@@ -358,10 +375,14 @@ Zlib.prototype._transform = function(chunk, encoding, cb) {
   // goodness.
   if (last)
     flushFlag = binding.Z_FINISH;
-  else if (chunk === null)
-    flushFlag = binding.Z_FULL_FLUSH;
-  else
-    flushFlag = binding.Z_NO_FLUSH;
+  else {
+    flushFlag = this._flushFlag;
+    // once we've flushed the last of the queue, stop flushing and
+    // go back to the normal behavior.
+    if (chunk.length >= ws.length) {
+      this._flushFlag = this._opts.flush || binding.Z_NO_FLUSH;
+    }
+  }
 
   var availInBefore = chunk && chunk.length;
   var availOutBefore = this._chunkSize - this._offset;
diff --git a/test/simple/test-zlib-write-after-flush.js b/test/simple/test-zlib-write-after-flush.js
new file mode 100644
index 000000000..e13871ecb
--- /dev/null
+++ b/test/simple/test-zlib-write-after-flush.js
@@ -0,0 +1,54 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var common = require('../common');
+var assert = require('assert');
+var zlib = require('zlib');
+var fs = require('fs');
+
+var gzip = zlib.createGzip();
+var gunz = zlib.createUnzip();
+
+gzip.pipe(gunz);
+
+var output = '';
+var input = 'A line of data\n';
+gunz.setEncoding('utf8');
+gunz.on('data', function(c) {
+  output += c;
+});
+
+process.on('exit', function() {
+  assert.equal(output, input);
+
+  // Make sure that the flush flag was set back to normal
+  assert.equal(gzip._flushFlag, zlib.Z_NO_FLUSH);
+
+  console.log('ok');
+});
+
+// make sure that flush/write doesn't trigger an assert failure
+gzip.flush(); write();
+function write() {
+  gzip.write(input);
+  gzip.end();
+  gunz.read(0);
+}