tls: invoke write cb only after opposite read end

Stream's `._write()` callback should be invoked only after it's opposite
stream has finished processing incoming data, otherwise `finish` event
fires too early and connection might be closed while there's some data
to send to the client.

see #5544

1 files changed, 69 insertions, 27 deletions

diff --git a/lib/tls.js b/lib/tls.js
index 4441bd1d6..5335a3a2f 100644
--- a/lib/tls.js
+++ b/lib/tls.js
@@ -254,6 +254,8 @@ function CryptoStream(pair, options) {
   this._pendingCallback = null;
   this._doneFlag = false;
   this._retryAfterPartial = false;
+  this._halfRead = false;
+  this._sslOutCb = null;
   this._resumingSession = false;
   this._reading = true;
   this._destroyed = false;
@@ -319,6 +321,19 @@ function onCryptoStreamEnd() {
 }
 
 
+// NOTE: Called once `this._opposite` is set.
+CryptoStream.prototype.init = function init() {
+  var self = this;
+  this._opposite.on('sslOutEnd', function() {
+    if (self._sslOutCb) {
+      var cb = self._sslOutCb;
+      self._sslOutCb = null;
+      cb(null);
+    }
+  });
+};
+
+
 CryptoStream.prototype._write = function write(data, encoding, cb) {
   assert(this._pending === null);
 
@@ -347,28 +362,36 @@ CryptoStream.prototype._write = function write(data, encoding, cb) {
       return cb(this.pair.error(true));
     }
 
+    // Whole buffer was written
+    if (written === data.length) {
+      if (this === this.pair.cleartext) {
+        debug('cleartext.write succeed with ' + written + ' bytes');
+      } else {
+        debug('encrypted.write succeed with ' + written + ' bytes');
+      }
+
+      // Invoke callback only when all data read from opposite stream
+      if (this._opposite._halfRead) {
+        assert(this._sslOutCb === null);
+        this._sslOutCb = cb;
+      } else {
+        cb(null);
+      }
+    }
+
     // Force SSL_read call to cycle some states/data inside OpenSSL
     this.pair.cleartext.read(0);
 
     // Cycle encrypted data
-    if (this.pair.encrypted._internallyPendingBytes()) {
+    if (this.pair.encrypted._internallyPendingBytes())
       this.pair.encrypted.read(0);
-    }
 
     // Get NPN and Server name when ready
     this.pair.maybeInitFinished();
 
-    // Whole buffer was written
     if (written === data.length) {
-      if (this === this.pair.cleartext) {
-        debug('cleartext.write succeed with ' + data.length + ' bytes');
-      } else {
-        debug('encrypted.write succeed with ' + data.length + ' bytes');
-      }
-
-      return cb(null);
-    }
-    if (written !== 0 && written !== -1) {
+      return;
+    } else if (written !== 0 && written !== -1) {
       assert(!this._retryAfterPartial);
       this._retryAfterPartial = true;
       this._write(data.slice(written), encoding, cb);
@@ -470,25 +493,42 @@ CryptoStream.prototype._read = function read(size) {
         this._opposite._done();
 
       // EOF
-      return this.push(null);
+      this.push(null);
+    } else {
+      // Bail out
+      this.push('');
     }
-
-    // Bail out
-    return this.push('');
+  } else {
+    // Give them requested data
+    if (this.ondata) {
+      var self = this;
+      this.ondata(pool, start, start + bytesRead);
+
+      // Consume data automatically
+      // simple/test-https-drain fails without it
+      process.nextTick(function() {
+        self.read(bytesRead);
+      });
+    }
+    this.push(pool.slice(start, start + bytesRead));
   }
 
-  // Give them requested data
-  if (this.ondata) {
-    var self = this;
-    this.ondata(pool, start, start + bytesRead);
+  // Let users know that we've some internal data to read
+  var halfRead = this._internallyPendingBytes() !== 0;
 
-    // Consume data automatically
-    // simple/test-https-drain fails without it
-    process.nextTick(function() {
-      self.read(bytesRead);
-    });
+  // Smart check to avoid invoking 'sslOutEnd' in the most of the cases
+  if (this._halfRead !== halfRead) {
+    this._halfRead = halfRead;
+
+    // Notify listeners about internal data end
+    if (this === this.pair.cleartext) {
+      debug('cleartext.sslOutEnd');
+    } else {
+      debug('encrypted.sslOutEnd');
+    }
+
+    this.emit('sslOutEnd');
   }
-  return this.push(pool.slice(start, start + bytesRead));
 };
 
 
@@ -600,7 +640,7 @@ CryptoStream.prototype.destroySoon = function(err) {
   if (this.writable)
     this.end();
 
-  if (this._writableState.finished)
+  if (this._writableState.finished && this._opposite._ended)
     this.destroy();
   else
     this.once('finish', this.destroy);
@@ -870,6 +910,8 @@ function SecurePair(credentials, isServer, requestCert, rejectUnauthorized,
   /* Let streams know about each other */
   this.cleartext._opposite = this.encrypted;
   this.encrypted._opposite = this.cleartext;
+  this.cleartext.init();
+  this.encrypted.init();
 
   process.nextTick(function() {
     /* The Connection may be destroyed by an abort call */