Skip to content

Commit

Permalink
tls: do not hang without newSession handler
Browse files Browse the repository at this point in the history
When listening for client hello parser events (like OCSP requests), do
not hang if `newSession` event handler is not present.

Fix: nodejs/node-v0.x-archive#8660
Fix: nodejs/node-v0.x-archive#25735

Reviewed-By: Fedor Indutny <fedor@indutny.com>
PR-URL: nodejs/node-v0.x-archive#25739
  • Loading branch information
indutny committed Jul 20, 2015
1 parent 39e0563 commit 7569711
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 11 deletions.
7 changes: 5 additions & 2 deletions lib/_tls_legacy.js
Original file line number Diff line number Diff line change
Expand Up @@ -662,14 +662,17 @@ function onnewsession(key, session) {
var self = this;
var once = false;

self.server.emit('newSession', key, session, function() {
if (!self.server.emit('newSession', key, session, done))
done();

function done() {
if (once)
return;
once = true;

if (self.ssl)
self.ssl.newSessionDone();
});
};
}


Expand Down
7 changes: 5 additions & 2 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,10 @@ function onnewsession(key, session) {
var once = false;

this._newSessionPending = true;
this.server.emit('newSession', key, session, function() {
if (!this.server.emit('newSession', key, session, done))
done();

function done() {
if (once)
return;
once = true;
Expand All @@ -212,7 +215,7 @@ function onnewsession(key, session) {
if (self._securePending)
self._finishInit();
self._securePending = false;
});
}
}


Expand Down
28 changes: 21 additions & 7 deletions test/simple/test-tls-ocsp-callback.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,19 @@ if (!common.opensslCli) {
process.exit(0);
}

var assert = require('assert');
var tls = require('tls');
var constants = require('constants');
var fs = require('fs');
var join = require('path').join;

test({ response: false }, function() {
test({ response: 'hello world' });
test({ response: 'hello world' }, function() {
test({ ocsp: false });
});
});

function test(testOptions, cb) {
var assert = require('assert');
var tls = require('tls');
var fs = require('fs');
var join = require('path').join;
var spawn = require('child_process').spawn;

var keyFile = join(common.fixturesDir, 'keys', 'agent1-key.pem');
var certFile = join(common.fixturesDir, 'keys', 'agent1-cert.pem');
Expand All @@ -54,6 +57,7 @@ function test(testOptions, cb) {
ca: [ca]
};
var requestCount = 0;
var clientSecure = 0;
var ocspCount = 0;
var ocspResponse;
var session;
Expand Down Expand Up @@ -83,9 +87,12 @@ function test(testOptions, cb) {
server.listen(common.PORT, function() {
var client = tls.connect({
port: common.PORT,
requestOCSP: true,
requestOCSP: testOptions.ocsp !== false,
secureOptions: testOptions.ocsp === false ?
constants.SSL_OP_NO_TICKET : 0,
rejectUnauthorized: false
}, function() {
clientSecure++;
});
client.on('OCSPResponse', function(resp) {
ocspResponse = resp;
Expand All @@ -98,12 +105,19 @@ function test(testOptions, cb) {
});

process.on('exit', function() {
if (testOptions.ocsp === false) {
assert.equal(requestCount, clientSecure);
assert.equal(requestCount, 1);
return;
}

if (testOptions.response) {
assert.equal(ocspResponse.toString(), testOptions.response);
} else {
assert.ok(ocspResponse === null);
}
assert.equal(requestCount, testOptions.response ? 0 : 1);
assert.equal(clientSecure, requestCount);
assert.equal(ocspCount, 1);
});
}

0 comments on commit 7569711

Please sign in to comment.