Browse Source

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>
v2.3.1-release
Fedor Indutny 10 years ago
parent
commit
80342f649d
  1. 36
      lib/_tls_wrap.js
  2. 42
      test/parallel/test-tls-close-error.js

36
lib/_tls_wrap.js

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

42
test/parallel/test-tls-close-error.js

@ -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);
});
Loading…
Cancel
Save