From 2a71f02988244b6299db8fe8ba3cc0491793acfc Mon Sep 17 00:00:00 2001 From: Malte-Thorben Bruns Date: Fri, 22 May 2015 21:12:59 +0200 Subject: [PATCH] tls: emit errors happening before handshake finish This fixes a race condition introduced in 80342f6. `socket.destroy(err)` only emits the passed error when `socket._writableState.errorEmitted === false`, `ssl.onerror` sets `errorEmitted = true` just before calling `socket.destroy()`. See: https://github.com/nodejs/io.js/issues/1119 See: https://github.com/nodejs/io.js/issues/1711 PR-URL: https://github.com/nodejs/io.js/pull/1769 Reviewed-By: Fedor Indutny --- lib/_tls_wrap.js | 3 +- test/parallel/test-tls-handshake-error.js | 46 +++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-tls-handshake-error.js diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 544fe95206ff34..b39cdb9a35c92b 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -359,7 +359,6 @@ TLSSocket.prototype._init = function(socket, wrap) { ssl.onerror = function(err) { if (self._writableState.errorEmitted) return; - self._writableState.errorEmitted = true; // Destroy socket if error happened before handshake's finish if (!self._secureEstablished) { @@ -373,6 +372,8 @@ TLSSocket.prototype._init = function(socket, wrap) { // Throw error self._emitTLSError(err); } + + self._writableState.errorEmitted = true; }; // If custom SNICallback was given, or if diff --git a/test/parallel/test-tls-handshake-error.js b/test/parallel/test-tls-handshake-error.js new file mode 100644 index 00000000000000..dbe91d0943c464 --- /dev/null +++ b/test/parallel/test-tls-handshake-error.js @@ -0,0 +1,46 @@ +'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'), + rejectUnauthorized: true +}, function(c) { +}).listen(common.PORT, function() { + var c = tls.connect({ + port: common.PORT, + ciphers: 'RC4' + }, function() { + assert(false, 'should not be called'); + }); + + c.on('error', function(err) { + errorCount++; + assert.notEqual(err.code, 'ECONNRESET'); + }); + + c.on('close', function(err) { + if (err) + closeCount++; + server.close(); + }); +}); + +process.on('exit', function() { + assert.equal(errorCount, 1); + assert.equal(closeCount, 1); +});