From 9ee86b718c8079474b2647385404042fd94fd116 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Thu, 30 May 2013 13:35:24 +0400 Subject: tls: proper .destroySoon 1. Emit `sslOutEnd` only when `_internallyPendingBytes() === 0`. 2. Read before checking `._halfRead`, otherwise we'll see only previous value, and will invoke `._write` callback improperly. 3. Wait for both `end` and `finish` events in `.destroySoon`. 4. Unpipe encrypted stream from socket to prevent write after destroy. --- lib/tls.js | 55 +++++++++++++++++++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/lib/tls.js b/lib/tls.js index 5335a3a2f..4d222f352 100644 --- a/lib/tls.js +++ b/lib/tls.js @@ -362,6 +362,16 @@ CryptoStream.prototype._write = function write(data, encoding, cb) { return cb(this.pair.error(true)); } + // Force SSL_read call to cycle some states/data inside OpenSSL + this.pair.cleartext.read(0); + + // Cycle encrypted data + 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) { @@ -377,19 +387,6 @@ CryptoStream.prototype._write = function write(data, encoding, 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()) - this.pair.encrypted.read(0); - - // Get NPN and Server name when ready - this.pair.maybeInitFinished(); - - if (written === data.length) { return; } else if (written !== 0 && written !== -1) { assert(!this._retryAfterPartial); @@ -521,13 +518,15 @@ CryptoStream.prototype._read = function read(size) { this._halfRead = halfRead; // Notify listeners about internal data end - if (this === this.pair.cleartext) { - debug('cleartext.sslOutEnd'); - } else { - debug('encrypted.sslOutEnd'); - } + if (!halfRead) { + if (this === this.pair.cleartext) { + debug('cleartext.sslOutEnd'); + } else { + debug('encrypted.sslOutEnd'); + } - this.emit('sslOutEnd'); + this.emit('sslOutEnd'); + } } }; @@ -640,10 +639,19 @@ CryptoStream.prototype.destroySoon = function(err) { if (this.writable) this.end(); - if (this._writableState.finished && this._opposite._ended) + if (this._writableState.finished && this._opposite._ended) { this.destroy(); - else - this.once('finish', this.destroy); + } else { + // Wait for both `finish` and `end` events to ensure that all data that + // was written on this side was read from the other side. + var self = this; + var waiting = 2; + function finish() { + if (--waiting === 0) self.destroy(); + } + this._opposite.once('end', finish); + this.once('finish', finish); + } }; @@ -1379,6 +1387,9 @@ function pipe(pair, socket) { pair.encrypted.on('close', function() { process.nextTick(function() { + // Encrypted should be unpiped from socket to prevent possible + // write after destroy. + pair.encrypted.unpipe(socket); socket.destroy(); }); }); -- cgit v1.2.1