tls: accept empty net.Sockets

Accept `new net.Socket()` as a `socket` option to `tls.connect()` without triggering an assertion error in C++. This is done by wrapping it into a JSStream to ensure that there will be a handle at the time of wrapping the socket into TLSSocket. Fix: #987 PR-URL: #1046 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Rod Vagg <[email protected]>
nodejs · Mar 3, 2015 · 7b3b8ac · 7b3b8ac
1 parent e1bf670
commit 7b3b8ac
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 11 deletions.
diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js
@@ -227,15 +227,20 @@ function TLSSocket(socket, options) {
   this.authorizationError = null;
 
   // Wrap plain JS Stream into StreamWrap
+  var wrap;
   if (!(socket instanceof net.Socket) && socket instanceof Duplex)
-    socket = new StreamWrap(socket);
+    wrap = new StreamWrap(socket);
+  else if ((socket instanceof net.Socket) && !socket._handle)
+    wrap = new StreamWrap(socket);
+  else
+    wrap = socket;
 
   // Just a documented property to make secure sockets
   // distinguishable from regular ones.
   this.encrypted = true;
 
   net.Socket.call(this, {
-    handle: this._wrapHandle(socket && socket._handle),
+    handle: this._wrapHandle(wrap && wrap._handle),
     allowHalfOpen: socket && socket.allowHalfOpen,
     readable: false,
     writable: false
@@ -246,7 +251,7 @@ function TLSSocket(socket, options) {
 
   this.on('error', this._tlsError);
 
-  this._init(socket);
+  this._init(socket, wrap);
 
   // Make sure to setup all required properties like: `_connecting` before
   // starting the flow of the data
@@ -302,7 +307,7 @@ TLSSocket.prototype._wrapHandle = function(handle) {
   return res;
 };
 
-TLSSocket.prototype._init = function(socket) {
+TLSSocket.prototype._init = function(socket, wrap) {
   var self = this;
   var options = this._tlsOptions;
   var ssl = this._handle;
@@ -394,24 +399,26 @@ TLSSocket.prototype._init = function(socket) {
       ssl.receive(buf);
   }
 
-  if (socket) {
+  if (socket instanceof net.Socket) {
     this._parent = socket;
 
     // To prevent assertion in afterConnect() and properly kick off readStart
-    this._connecting = socket._connecting;
+    this._connecting = socket._connecting || !socket._handle;
     socket.once('connect', function() {
       self._connecting = false;
       self.emit('connect');
     });
-
-    socket.on('error', function(err) {
-      self._tlsError(err);
-    });
   }
 
   // Assume `tls.connect()`
-  if (!socket)
+  if (wrap) {
+    wrap.on('error', function(err) {
+      self._tlsError(err);
+    });
+  } else {
+    assert(!socket);
     this._connecting = true;
+  }
 };
 
 TLSSocket.prototype.renegotiate = function(options, callback) {
@@ -506,6 +513,7 @@ TLSSocket.prototype._start = function() {
     return;
   }
 
+  debug('start');
   if (this._tlsOptions.requestOCSP)
     this._handle.requestOCSP();
   this._handle.start();

diff --git a/test/parallel/test-tls-on-empty-socket.js b/test/parallel/test-tls-on-empty-socket.js
@@ -0,0 +1,41 @@
+if (!process.versions.openssl) {
+  console.error('Skipping because node compiled without OpenSSL.');
+  process.exit(0);
+}
+
+var assert = require('assert');
+var fs = require('fs');
+var net = require('net');
+var tls = require('tls');
+
+var common = require('../common');
+
+var out = '';
+
+var server = tls.createServer({
+  key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'),
+  cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem')
+}, function(c) {
+  c.end('hello');
+}).listen(common.PORT, function() {
+  var socket = new net.Socket();
+
+  var s = tls.connect({
+    socket: socket,
+    rejectUnauthorized: false
+  }, function() {
+    s.on('data', function(chunk) {
+      out += chunk;
+    });
+    s.on('end', function() {
+      s.destroy();
+      server.close();
+    });
+  });
+
+  socket.connect(common.PORT);
+});
+
+process.on('exit', function() {
+  assert.equal(out, 'hello');
+});