Skip to content

Commit 9ee86b7

Browse files
committed
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.
1 parent 9826b15 commit 9ee86b7

1 file changed

Lines changed: 33 additions & 22 deletions

File tree

lib/tls.js

Lines changed: 33 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,16 @@ CryptoStream.prototype._write = function write(data, encoding, cb) {
362362
return cb(this.pair.error(true));
363363
}
364364

365+
// Force SSL_read call to cycle some states/data inside OpenSSL
366+
this.pair.cleartext.read(0);
367+
368+
// Cycle encrypted data
369+
if (this.pair.encrypted._internallyPendingBytes())
370+
this.pair.encrypted.read(0);
371+
372+
// Get NPN and Server name when ready
373+
this.pair.maybeInitFinished();
374+
365375
// Whole buffer was written
366376
if (written === data.length) {
367377
if (this === this.pair.cleartext) {
@@ -377,19 +387,6 @@ CryptoStream.prototype._write = function write(data, encoding, cb) {
377387
} else {
378388
cb(null);
379389
}
380-
}
381-
382-
// Force SSL_read call to cycle some states/data inside OpenSSL
383-
this.pair.cleartext.read(0);
384-
385-
// Cycle encrypted data
386-
if (this.pair.encrypted._internallyPendingBytes())
387-
this.pair.encrypted.read(0);
388-
389-
// Get NPN and Server name when ready
390-
this.pair.maybeInitFinished();
391-
392-
if (written === data.length) {
393390
return;
394391
} else if (written !== 0 && written !== -1) {
395392
assert(!this._retryAfterPartial);
@@ -521,13 +518,15 @@ CryptoStream.prototype._read = function read(size) {
521518
this._halfRead = halfRead;
522519

523520
// Notify listeners about internal data end
524-
if (this === this.pair.cleartext) {
525-
debug('cleartext.sslOutEnd');
526-
} else {
527-
debug('encrypted.sslOutEnd');
528-
}
521+
if (!halfRead) {
522+
if (this === this.pair.cleartext) {
523+
debug('cleartext.sslOutEnd');
524+
} else {
525+
debug('encrypted.sslOutEnd');
526+
}
529527

530-
this.emit('sslOutEnd');
528+
this.emit('sslOutEnd');
529+
}
531530
}
532531
};
533532

@@ -640,10 +639,19 @@ CryptoStream.prototype.destroySoon = function(err) {
640639
if (this.writable)
641640
this.end();
642641

643-
if (this._writableState.finished && this._opposite._ended)
642+
if (this._writableState.finished && this._opposite._ended) {
644643
this.destroy();
645-
else
646-
this.once('finish', this.destroy);
644+
} else {
645+
// Wait for both `finish` and `end` events to ensure that all data that
646+
// was written on this side was read from the other side.
647+
var self = this;
648+
var waiting = 2;
649+
function finish() {
650+
if (--waiting === 0) self.destroy();
651+
}
652+
this._opposite.once('end', finish);
653+
this.once('finish', finish);
654+
}
647655
};
648656

649657

@@ -1379,6 +1387,9 @@ function pipe(pair, socket) {
13791387

13801388
pair.encrypted.on('close', function() {
13811389
process.nextTick(function() {
1390+
// Encrypted should be unpiped from socket to prevent possible
1391+
// write after destroy.
1392+
pair.encrypted.unpipe(socket);
13821393
socket.destroy();
13831394
});
13841395
});

0 commit comments

Comments
 (0)