Skip to content

Commit 43a77d4

Browse files
skenqbxandrewdeandrade
authored andcommitted
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: nodejs/node#1119 See: nodejs/node#1711 PR-URL: nodejs/node#1769 Reviewed-By: Fedor Indutny <fedor@indutny.com>
1 parent 92b288a commit 43a77d4

File tree

2 files changed

+48
-1
lines changed

2 files changed

+48
-1
lines changed

lib/_tls_wrap.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,6 @@ TLSSocket.prototype._init = function(socket, wrap) {
359359
ssl.onerror = function(err) {
360360
if (self._writableState.errorEmitted)
361361
return;
362-
self._writableState.errorEmitted = true;
363362

364363
// Destroy socket if error happened before handshake's finish
365364
if (!self._secureEstablished) {
@@ -373,6 +372,8 @@ TLSSocket.prototype._init = function(socket, wrap) {
373372
// Throw error
374373
self._emitTLSError(err);
375374
}
375+
376+
self._writableState.errorEmitted = true;
376377
};
377378

378379
// If custom SNICallback was given, or if
+46
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
'use strict';
2+
3+
var assert = require('assert');
4+
var common = require('../common');
5+
6+
if (!common.hasCrypto) {
7+
console.log('1..0 # Skipped: missing crypto');
8+
process.exit();
9+
}
10+
var tls = require('tls');
11+
12+
var fs = require('fs');
13+
var net = require('net');
14+
15+
var errorCount = 0;
16+
var closeCount = 0;
17+
18+
var server = tls.createServer({
19+
key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'),
20+
cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem'),
21+
rejectUnauthorized: true
22+
}, function(c) {
23+
}).listen(common.PORT, function() {
24+
var c = tls.connect({
25+
port: common.PORT,
26+
ciphers: 'RC4'
27+
}, function() {
28+
assert(false, 'should not be called');
29+
});
30+
31+
c.on('error', function(err) {
32+
errorCount++;
33+
assert.notEqual(err.code, 'ECONNRESET');
34+
});
35+
36+
c.on('close', function(err) {
37+
if (err)
38+
closeCount++;
39+
server.close();
40+
});
41+
});
42+
43+
process.on('exit', function() {
44+
assert.equal(errorCount, 1);
45+
assert.equal(closeCount, 1);
46+
});

0 commit comments

Comments
 (0)