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

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 */

test/simple/test-tls-client-destroy-soon.js

@@ -0,0 +1,75 @@
+// 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.
+
+// Create an ssl server.  First connection, validate that not resume.
+// Cache session and close connection.  Use session on second connection.
+// ASSERT resumption.
+
+if (!process.versions.openssl) {
+  console.error('Skipping because node compiled without OpenSSL.');
+  process.exit(0);
+}
+
+var common = require('../common');
+var assert = require('assert');
+var tls = require('tls');
+var fs = require('fs');
+
+var options = {
+  key: fs.readFileSync(common.fixturesDir + '/keys/agent2-key.pem'),
+  cert: fs.readFileSync(common.fixturesDir + '/keys/agent2-cert.pem')
+};
+
+var big = new Buffer(2 * 1024 * 1024);
+var connections = 0;
+var bytesRead = 0;
+
+big.fill('Y');
+
+// create server
+var server = tls.createServer(options, function(socket) {
+  socket.end(big);
+  socket.destroySoon();
+  connections++;
+});
+
+// start listening
+server.listen(common.PORT, function() {
+  var client = tls.connect({
+    port: common.PORT,
+    rejectUnauthorized: false
+  }, function() {
+    client.on('readable', function() {
+      var d = client.read();
+      if (d)
+        bytesRead += d.length;
+    });
+
+    client.on('end', function() {
+      server.close();
+    });
+  });
+});
+
+process.on('exit', function() {
+  assert.equal(1, connections);
+  assert.equal(big.length, bytesRead);
+});