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: #1114 PR-URL: #1496 Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com> Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
nodejs · Jun 17, 2015 · 061342a · 061342a
1 parent 7a3006e
commit 061342a
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 11 deletions.
diff --git 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
@@ -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
@@ -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);
 });