http: Reuse more http/https Agent code

lib/_http_agent.js

@@ -24,6 +24,7 @@ var url = require('url');
 var util = require('util');
 var EventEmitter = require('events').EventEmitter;
 var ClientRequest = require('_http_client').ClientRequest;
+var debug = util.debuglog('http');
 
 // New Agent code.
 
@@ -44,7 +45,12 @@ function Agent(options) {
   EventEmitter.call(this);
 
   var self = this;
+
+  self.defaultPort = 80;
+  self.protocol = 'http:';
+
   self.options = util._extend({}, options);
+
   // don't confuse net and make it think that we're connecting to a pipe
   self.options.path = null;
   self.requests = {};
@@ -54,11 +60,9 @@ function Agent(options) {
   self.keepAlive = self.options.keepAlive || false;
   self.maxSockets = self.options.maxSockets || Agent.defaultMaxSockets;
 
-  self.on('free', function(socket, host, port, localAddress) {
+  self.on('free', function(socket, options) {
-    var name = host + ':' + port;
+    var name = self.getName(options);
-    if (localAddress) {
+    debug('agent.on(free)', name);
-      name += ':' + localAddress;
-    }
 
     if (!socket.destroyed &&
         self.requests[name] && self.requests[name].length) {
@@ -103,18 +107,38 @@ exports.Agent = Agent;
 Agent.defaultMaxSockets = Infinity;
 
 Agent.prototype.createConnection = net.createConnection;
-Agent.prototype.defaultPort = 80;
+
-Agent.prototype.protocol = 'http:';
+// Get the key for a given set of request options
-Agent.prototype.addRequest = function(req, host, port, localAddress) {
+Agent.prototype.getName = function(options) {
-  var name = host + ':' + port;
+  var name = '';
-  if (localAddress) {
+
-    name += ':' + localAddress;
+  if (options.host)
-  }
+    name += options.host;
+  else
+    name += 'localhost';
+
+  name += ':';
+  if (options.port)
+    name += options.port;
+  name += ':';
+  if (options.localAddress)
+    name += options.localAddress;
+  name += ':';
+  return name;
+};
+
+Agent.prototype.addRequest = function(req, options) {
+  var host = options.host;
+  var port = options.port;
+  var localAddress = options.localAddress;
+
+  var name = this.getName(options);
   if (!this.sockets[name]) {
     this.sockets[name] = [];
   }
 
   if (this.freeSockets[name] && this.freeSockets[name].length) {
+    debug('have free socket');
     // we have a free socket, so use that.
     var socket = this.freeSockets[name].shift();
 
@@ -125,9 +149,11 @@ Agent.prototype.addRequest = function(req, host, port, localAddress) {
     socket.ref();
     req.onSocket(socket);
   } else if (this.sockets[name].length < this.maxSockets) {
+    debug('call onSocket');
     // If we are under maxSockets create a new one.
-    req.onSocket(this.createSocket(name, host, port, localAddress, req));
+    req.onSocket(this.createSocket(req, options));
   } else {
+    debug('wait for socket');
     // We are over limit so we'll add it to the queue.
     if (!this.requests[name]) {
       this.requests[name] = [];
@@ -136,14 +162,12 @@ Agent.prototype.addRequest = function(req, host, port, localAddress) {
   }
 };
 
-Agent.prototype.createSocket = function(name, host, port, localAddress, req) {
+Agent.prototype.createSocket = function(req, options) {
   var self = this;
-  var options = util._extend({}, self.options);
+  options = util._extend({}, options);
-  options.port = port;
+  options = util._extend(options, self.options);
-  options.host = host;
-  options.localAddress = localAddress;
 
-  options.servername = host;
+  options.servername = options.host;
   if (req) {
     var hostHeader = req.getHeader('host');
     if (hostHeader) {
@@ -151,30 +175,36 @@ Agent.prototype.createSocket = function(name, host, port, localAddress, req) {
     }
   }
 
+  var name = self.getName(options);
+
+  debug('createConnection', name, options);
   var s = self.createConnection(options);
   if (!self.sockets[name]) {
     self.sockets[name] = [];
   }
   this.sockets[name].push(s);
+  debug('sockets', name, this.sockets[name].length);
 
   function onFree() {
-    self.emit('free', s, host, port, localAddress);
+    self.emit('free', s, options);
   }
   s.on('free', onFree);
 
   function onClose(err) {
+    debug('CLIENT socket onClose');
     // This is the only place where sockets get removed from the Agent.
     // If you want to remove a socket from the pool, just close it.
     // All socket errors end in a close event anyway.
-    self.removeSocket(s, name, host, port, localAddress);
+    self.removeSocket(s, options);
   }
   s.on('close', onClose);
 
   function onRemove() {
     // We need this function for cases like HTTP 'upgrade'
-    // (defined by WebSockets) where we need to remove a socket from the pool
+    // (defined by WebSockets) where we need to remove a socket from the
-    //  because it'll be locked up indefinitely
+    // pool because it'll be locked up indefinitely
-    self.removeSocket(s, name, host, port, localAddress);
+    debug('CLIENT socket onRemove');
+    self.removeSocket(s, options);
     s.removeListener('close', onClose);
     s.removeListener('free', onFree);
     s.removeListener('agentRemove', onRemove);
@@ -183,7 +213,9 @@ Agent.prototype.createSocket = function(name, host, port, localAddress, req) {
   return s;
 };
 
-Agent.prototype.removeSocket = function(s, name, host, port, localAddress) {
+Agent.prototype.removeSocket = function(s, options) {
+  var name = this.getName(options);
+  debug('removeSocket', name);
   if (this.sockets[name]) {
     var index = this.sockets[name].indexOf(s);
     if (index !== -1) {
@@ -195,9 +227,10 @@ Agent.prototype.removeSocket = function(s, name, host, port, localAddress) {
     }
   }
   if (this.requests[name] && this.requests[name].length) {
+    debug('removeSocket, have a request, make a socket');
     var req = this.requests[name][0];
-    // If we have pending requests and a socket gets closed a new one
+    // If we have pending requests and a socket gets closed make a new one
-    this.createSocket(name, host, port, localAddress, req).emit('free');
+    this.createSocket(req, options).emit('free');
   }
 };
 
@@ -216,6 +249,10 @@ Agent.prototype.request = function(options, cb) {
   if (typeof options === 'string') {
     options = url.parse(options);
   }
+  // don't try to do dns lookups of foo.com:8080, just foo.com
+  if (options.hostname) {
+    options.host = options.hostname;
+  }
 
   if (options && options.path && / /.test(options.path)) {
     // The actual regex is more like /[^A-Za-z0-9\-._~!$&'()*+,;=/:@]/
@@ -229,11 +266,16 @@ Agent.prototype.request = function(options, cb) {
     throw new Error('Protocol:' + options.protocol + ' not supported.');
   }
 
-  options = util._extend({ agent: this, keepAlive: false }, options);
+  options = util._extend({
+    agent: this,
+    keepAlive: this.keepAlive
+  }, options);
 
+  // if it's false, then make a new one, just like this one.
   if (options.agent === false)
-    options.agent = new Agent(options);
+    options.agent = new this.constructor(options);
 
+  debug('agent.request', options);
   return new ClientRequest(options, cb);
 };
 

lib/_http_client.js

@@ -99,33 +99,26 @@ function ClientRequest(options, cb) {
     self._storeHeader(self.method + ' ' + self.path + ' HTTP/1.1\r\n',
                       self._renderHeaders());
   }
+
   if (self.socketPath) {
     self._last = true;
     self.shouldKeepAlive = false;
-    if (options.createConnection) {
+    var conn = self.agent.createConnection({ path: self.socketPath });
-      self.onSocket(options.createConnection(self.socketPath));
+    self.onSocket(conn);
-    } else {
-      self.onSocket(net.createConnection(self.socketPath));
-    }
   } else if (self.agent) {
     // If there is an agent we should default to Connection:keep-alive.
     self._last = false;
     self.shouldKeepAlive = true;
-    self.agent.addRequest(self, host, port, options.localAddress);
+    self.agent.addRequest(self, options);
   } else {
     // No agent, default to Connection:close.
     self._last = true;
     self.shouldKeepAlive = false;
     if (options.createConnection) {
-      options.port = port;
-      options.host = host;
       var conn = options.createConnection(options);
     } else {
-      var conn = net.createConnection({
+      debug('CLIENT use net.createConnection', options);
-        port: port,
+      var conn = net.createConnection(options);
-        host: host,
-        localAddress: options.localAddress
-      });
     }
     self.onSocket(conn);
   }
@@ -134,8 +127,8 @@ function ClientRequest(options, cb) {
     self._flush();
     self = null;
   });
-
 }
+
 util.inherits(ClientRequest, OutgoingMessage);
 
 exports.ClientRequest = ClientRequest;

lib/https.js

@@ -24,6 +24,7 @@ var http = require('http');
 var util = require('util');
 var url = require('url');
 var inherits = require('util').inherits;
+var debug = util.debuglog('https');
 
 function Server(opts, requestListener) {
   if (!(this instanceof Server)) return new Server(opts, requestListener);
@@ -77,56 +78,58 @@ function createConnection(port, host, options) {
     options.host = host;
   }
 
+  debug('createConnection', options);
   return tls.connect(options);
 }
 
 
 function Agent(options) {
   http.Agent.call(this, options);
+  this.defaultPort = 443;
+  this.protocol = 'https:';
 }
 inherits(Agent, http.Agent);
-Agent.prototype.defaultPort = 443;
-Agent.prototype.protocol = 'https:';
 Agent.prototype.createConnection = createConnection;
 
+Agent.prototype.getName = function(options) {
+  var name = http.Agent.prototype.getName.call(this, options);
+
+  name += ':';
+  if (options.ca)
+    name += options.ca;
+
+  name += ':';
+  if (options.cert)
+    name += options.cert;
+
+  name += ':';
+  if (options.ciphers)
+    name += options.ciphers;
+
+  name += ':';
+  if (options.key)
+    name += options.key;
+
+  name += ':';
+  if (options.pfx)
+    name += options.pfx;
+
+  name += ':';
+  if (options.rejectUnauthorized !== undefined)
+    name += options.rejectUnauthorized;
+
+  return name;
+};
+
 var globalAgent = new Agent();
 
 exports.globalAgent = globalAgent;
 exports.Agent = Agent;
 
 exports.request = function(options, cb) {
-  if (typeof options === 'string') {
+  return globalAgent.request(options, cb);
-    options = url.parse(options);
-  }
-
-  if (options.protocol && options.protocol !== 'https:') {
-    throw new Error('Protocol:' + options.protocol + ' not supported.');
-  }
-
-  options = util._extend({
-    createConnection: createConnection,
-    defaultPort: 443
-  }, options);
-
-  if (typeof options.agent === 'undefined') {
-    if (typeof options.ca === 'undefined' &&
-        typeof options.cert === 'undefined' &&
-        typeof options.ciphers === 'undefined' &&
-        typeof options.key === 'undefined' &&
-        typeof options.passphrase === 'undefined' &&
-        typeof options.pfx === 'undefined' &&
-        typeof options.rejectUnauthorized === 'undefined') {
-      options.agent = globalAgent;
-    } else {
-      options.agent = new Agent(options);
-    }
-  }
-
-  return new http.ClientRequest(options, cb);
 };
 
 exports.get = function(options, cb) {
-  var req = exports.request(options, cb);
+  return globalAgent.get(options, cb);
-  req.end();
-  return req;
 };

test/simple/test-http-agent-destroyed-socket.js

@@ -43,7 +43,7 @@ var requestOptions = {
 
 var request1 = http.get(requestOptions, function(response) {
   // assert request2 is queued in the agent
-  var key = 'localhost:' + common.PORT;
+  var key = agent.getName(requestOptions);
   assert(agent.requests[key].length === 1);
   console.log('got response1');
   request1.socket.on('close', function() {

test/simple/test-http-client-agent.js

@@ -23,7 +23,7 @@ var common = require('../common');
 var assert = require('assert');
 var http = require('http');
 
-var name = 'localhost:' + common.PORT;
+var name = http.globalAgent.getName({ port: common.PORT });
 var max = 3;
 var count = 0;
 

test/simple/test-http-keep-alive-close-on-header.js

@@ -38,6 +38,7 @@ var connectCount = 0;
 
 server.listen(common.PORT, function() {
   var agent = new http.Agent({ maxSockets: 1 });
+  var name = agent.getName({ port: common.PORT });
   var request = http.request({
     method: 'GET',
     path: '/',
@@ -45,7 +46,7 @@ server.listen(common.PORT, function() {
     port: common.PORT,
     agent: agent
   }, function(res) {
-    assert.equal(1, agent.sockets['localhost:' + common.PORT].length);
+    assert.equal(1, agent.sockets[name].length);
     res.resume();
   });
   request.on('socket', function(s) {
@@ -62,7 +63,7 @@ server.listen(common.PORT, function() {
     port: common.PORT,
     agent: agent
   }, function(res) {
-    assert.equal(1, agent.sockets['localhost:' + common.PORT].length);
+    assert.equal(1, agent.sockets[name].length);
     res.resume();
   });
   request.on('socket', function(s) {
@@ -79,7 +80,7 @@ server.listen(common.PORT, function() {
     agent: agent
   }, function(response) {
     response.on('end', function() {
-      assert.equal(1, agent.sockets['localhost:' + common.PORT].length);
+      assert.equal(1, agent.sockets[name].length);
       server.close();
     });
     response.resume();

test/simple/test-http-keep-alive.js

@@ -32,9 +32,9 @@ var server = http.createServer(function(req, res) {
 });
 
 var connectCount = 0;
-var name = 'localhost:' + common.PORT;
 var agent = new http.Agent({maxSockets: 1});
 var headers = {'connection': 'keep-alive'};
+var name = agent.getName({ port: common.PORT });
 
 server.listen(common.PORT, function() {
   http.get({

test/simple/test-regress-GH-1531.js

@@ -42,22 +42,25 @@ var server = https.createServer(options, function(req, res) {
 });
 
 server.listen(common.PORT, function() {
+  console.error('listening');
   https.get({
     agent: false,
     path: '/',
     port: common.PORT,
     rejectUnauthorized: false
   }, function(res) {
-    console.error(res.statusCode);
+    console.error(res.statusCode, res.headers);
     gotCallback = true;
+    res.resume();
     server.close();
   }).on('error', function(e) {
-    console.error(e.message);
+    console.error(e.stack);
     process.exit(1);
   });
 });
 
 process.on('exit', function() {
   assert.ok(gotCallback);
+  console.log('ok');
 });
 

test/simple/test-regress-GH-877.js

@@ -35,7 +35,7 @@ var server = http.createServer(function(req, res) {
   res.end('Hello World\n');
 });
 
-var addrString = '127.0.0.1:' + common.PORT;
+var addrString = agent.getName({ host: '127.0.0.1', port: common.PORT });
 
 server.listen(common.PORT, '127.0.0.1', function() {
   for (var i = 0; i < N; i++) {
@@ -55,7 +55,7 @@ server.listen(common.PORT, '127.0.0.1', function() {
 
     console.log('Socket: ' + agent.sockets[addrString].length + '/' +
                 agent.maxSockets + ' queued: ' + (agent.requests[addrString] ?
-                agent.requests['127.0.0.1:' + common.PORT].length : 0));
+                agent.requests[addrString].length : 0));
 
     var agentRequests = agent.requests[addrString] ?
         agent.requests[addrString].length : 0;