tls: use `.destroy(err)` instead of destroy+emit

Emit errors using `.destroy(err)` instead of `.destroy()` and
`.emit('error', err)`. Otherwise `close` event is emitted with the
`error` argument set to `false`, even if the connection was torn down
because of the error.

See: https://github.com/nodejs/io.js/issues/1119
PR-URL: https://github.com/nodejs/io.js/pull/1711
Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com>
pull/1658/merge
Fedor Indutny 2015-05-15 21:21:06 +02:00
parent 2b1c01c2cc
commit 80342f649d
2 changed files with 64 additions and 14 deletions

View File

@ -38,7 +38,7 @@ function onhandshakestart() {
// callback to destroy the connection right now, it would crash and burn.
setImmediate(function() {
var err = new Error('TLS session renegotiation attack detected.');
self._tlsError(err);
self._emitTLSError(err);
});
}
}
@ -233,7 +233,7 @@ function TLSSocket(socket, options) {
// Proxy for API compatibility
this.ssl = this._handle;
this.on('error', this._tlsError);
this.on('error', this._emitTLSError);
this._init(socket, wrap);
@ -363,8 +363,7 @@ TLSSocket.prototype._init = function(socket, wrap) {
// Destroy socket if error happened before handshake's finish
if (!self._secureEstablished) {
self._tlsError(err);
self.destroy();
self.destroy(self._tlsError(err));
} else if (options.isServer &&
rejectUnauthorized &&
/peer did not return a certificate/.test(err.message)) {
@ -372,7 +371,7 @@ TLSSocket.prototype._init = function(socket, wrap) {
self.destroy();
} else {
// Throw error
self._tlsError(err);
self._emitTLSError(err);
}
};
@ -416,7 +415,7 @@ TLSSocket.prototype._init = function(socket, wrap) {
// Assume `tls.connect()`
if (wrap) {
wrap.on('error', function(err) {
self._tlsError(err);
self._emitTLSError(err);
});
} else {
assert(!socket);
@ -472,20 +471,27 @@ TLSSocket.prototype.getTLSTicket = function getTLSTicket() {
};
TLSSocket.prototype._handleTimeout = function() {
this._tlsError(new Error('TLS handshake timeout'));
this._emitTLSError(new Error('TLS handshake timeout'));
};
TLSSocket.prototype._emitTLSError = function(err) {
var e = this._tlsError(err);
if (e)
this.emit('error', e);
};
TLSSocket.prototype._tlsError = function(err) {
this.emit('_tlsError', err);
if (this._controlReleased)
this.emit('error', err);
return err;
return null;
};
TLSSocket.prototype._releaseControl = function() {
if (this._controlReleased)
return false;
this._controlReleased = true;
this.removeListener('error', this._tlsError);
this.removeListener('error', this._emitTLSError);
return true;
};
@ -717,7 +723,11 @@ function Server(/* [options], listener */) {
});
var errorEmitted = false;
socket.on('close', function() {
socket.on('close', function(err) {
// Closed because of error - no need to emit it twice
if (err)
return;
// Emit ECONNRESET
if (!socket._controlReleased && !errorEmitted) {
errorEmitted = true;
@ -936,8 +946,7 @@ exports.connect = function(/* [port, host], options, cb */) {
socket.authorizationError = verifyError.code || verifyError.message;
if (options.rejectUnauthorized) {
socket.emit('error', verifyError);
socket.destroy();
socket.destroy(verifyError);
return;
} else {
socket.emit('secureConnect');
@ -957,8 +966,7 @@ exports.connect = function(/* [port, host], options, cb */) {
socket._hadError = true;
var error = new Error('socket hang up');
error.code = 'ECONNRESET';
socket.destroy();
socket.emit('error', error);
socket.destroy(error);
}
}
socket.once('end', onHangUp);

View File

@ -0,0 +1,42 @@
'use strict';
var assert = require('assert');
var common = require('../common');
if (!common.hasCrypto) {
console.log('1..0 # Skipped: missing crypto');
process.exit();
}
var tls = require('tls');
var fs = require('fs');
var net = require('net');
var errorCount = 0;
var closeCount = 0;
var server = tls.createServer({
key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'),
cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem')
}, function(c) {
}).listen(common.PORT, function() {
var c = tls.connect(common.PORT, function() {
assert(false, 'should not be called');
});
c.on('error', function(err) {
errorCount++;
});
c.on('close', function(err) {
if (err)
closeCount++;
server.close();
});
});
process.on('exit', function() {
assert.equal(errorCount, 1);
assert.equal(closeCount, 1);
});