From 061342a50075a23e04465e0ac2f33124ab56ea32 Mon Sep 17 00:00:00 2001 From: James Hartig Date: Mon, 8 Jun 2015 18:25:06 -0400 Subject: [PATCH] net: Defer reading until listeners could be added 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: https://github.com/nodejs/io.js/issues/1114 PR-URL: https://github.com/nodejs/io.js/pull/1496 Reviewed-By: Chris Dickinson Reviewed-By: Fedor Indutny --- lib/_tls_wrap.js | 25 ++++++---- .../parallel/test-tls-delayed-attach-error.js | 46 +++++++++++++++++++ test/parallel/test-tls-delayed-attach.js | 3 -- 3 files changed, 63 insertions(+), 11 deletions(-) create mode 100644 test/parallel/test-tls-delayed-attach-error.js diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index fe404213c24077..8ccee2379d4309 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -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. @@ -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; @@ -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; diff --git a/test/parallel/test-tls-delayed-attach-error.js b/test/parallel/test-tls-delayed-attach-error.js new file mode 100644 index 00000000000000..e2dbc5815d201c --- /dev/null +++ b/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); +}); diff --git a/test/parallel/test-tls-delayed-attach.js b/test/parallel/test-tls-delayed-attach.js index 00731592445f04..a5b312d1633831 100644 --- a/test/parallel/test-tls-delayed-attach.js +++ b/test/parallel/test-tls-delayed-attach.js @@ -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'), @@ -32,7 +31,6 @@ var server = net.createServer(function(c) { }); s.on('end', function() { - ended++; server.close(); s.destroy(); }); @@ -47,5 +45,4 @@ var server = net.createServer(function(c) { process.on('exit', function() { assert.equal(received, sent); - assert.equal(ended, 1); });