Skip to content

Commit

Permalink
net: Defer reading until listeners could be added
Browse files Browse the repository at this point in the history
Defer reading until user-land has a chance to add listeners. This
allows the TLS wrapper to listen for _tlsError and trigger a
clientError event if the socket already has data that could trigger.

Fixes: #1114
PR-URL: #1496
Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
  • Loading branch information
jameshartig authored and brendanashworth committed Jun 17, 2015
1 parent 7a3006e commit 061342a
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 11 deletions.
25 changes: 17 additions & 8 deletions lib/_tls_wrap.js
Expand Up @@ -209,6 +209,20 @@ function onocspresponse(resp) {
this.emit('OCSPResponse', resp);
}

function initRead(tls, wrapped) {
// If we were destroyed already don't bother reading
if (!tls._handle)
return;

// Socket already has some buffered data - emulate receiving it
if (wrapped && wrapped._readableState && wrapped._readableState.length) {
var buf;
while ((buf = wrapped.read()) !== null)
tls._handle.receive(buf);
}

tls.read(0);
}

/**
* Provides a wrap of socket stream to do encrypted communication.
Expand Down Expand Up @@ -257,7 +271,9 @@ function TLSSocket(socket, options) {
// starting the flow of the data
this.readable = true;
this.writable = true;
this.read(0);

// Read on next tick so the caller has a chance to setup listeners
process.nextTick(initRead, this, socket);
}
util.inherits(TLSSocket, net.Socket);
exports.TLSSocket = TLSSocket;
Expand Down Expand Up @@ -416,13 +432,6 @@ TLSSocket.prototype._init = function(socket, wrap) {
if (options.handshakeTimeout > 0)
this.setTimeout(options.handshakeTimeout, this._handleTimeout);

// Socket already has some buffered data - emulate receiving it
if (socket && socket._readableState && socket._readableState.length) {
var buf;
while ((buf = socket.read()) !== null)
ssl.receive(buf);
}

if (socket instanceof net.Socket) {
this._parent = socket;

Expand Down
46 changes: 46 additions & 0 deletions test/parallel/test-tls-delayed-attach-error.js
@@ -0,0 +1,46 @@
'use strict';
var common = require('../common');
var assert = require('assert');

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 bonkers = new Buffer(1024);
bonkers.fill(42);

var receivedError = false;
var options = {
key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'),
cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem')
};

var server = net.createServer(function(c) {
setTimeout(function() {
var s = new tls.TLSSocket(c, {
isServer: true,
secureContext: tls.createSecureContext(options)
});

s.on('_tlsError', function() {
receivedError = true;
});

s.on('close', function() {
server.close();
s.destroy();
});
}, 200);
}).listen(common.PORT, function() {
var c = net.connect({port: common.PORT}, function() {
c.write(bonkers);
});
});

process.on('exit', function() {
assert.ok(receivedError);
});
3 changes: 0 additions & 3 deletions test/parallel/test-tls-delayed-attach.js
Expand Up @@ -13,7 +13,6 @@ var net = require('net');

var sent = 'hello world';
var received = '';
var ended = 0;

var options = {
key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'),
Expand All @@ -32,7 +31,6 @@ var server = net.createServer(function(c) {
});

s.on('end', function() {
ended++;
server.close();
s.destroy();
});
Expand All @@ -47,5 +45,4 @@ var server = net.createServer(function(c) {

process.on('exit', function() {
assert.equal(received, sent);
assert.equal(ended, 1);
});

0 comments on commit 061342a

Please sign in to comment.