Control: reassign -1 node-tunnel-agent 0.3.1-1

On Fri, Mar 25 2016, Gerald Turner wrote:
> Sorry that I haven't found a solution, but I believe I'm onto
> something, an API incompatibility perhaps?

Looks like nodejs changed their internal API almost three years ago (see
attached diff) which was released as 0.11.4.

Looks like tunnel-agent fixed this API inconsistency two years ago (see
attached diff) which was eventually released in version 0.4.2 a few
months ago.

-- 
Gerald Turner <gtur...@unzane.com>        Encrypted mail preferred!
OpenPGP: 4096R / CA89 B27A 30FA 66C5 1B80  3858 EC94 2276 FDB8 716D
commit 49519f121787d51394f00c871f854794e409bdda
Author: isaacs <i...@izs.me>
Date:   Wed May 22 18:44:24 2013 -0700

    http: Reuse more http/https Agent code

diff --git a/lib/_http_agent.js b/lib/_http_agent.js
index 20fa1b6..07e8f4c 100644
--- a/lib/_http_agent.js
+++ b/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) {
-    var name = host + ':' + port;
-    if (localAddress) {
-      name += ':' + localAddress;
-    }
+  self.on('free', function(socket, options) {
+    var name = self.getName(options);
+    debug('agent.on(free)', name);
 
     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:';
-Agent.prototype.addRequest = function(req, host, port, localAddress) {
-  var name = host + ':' + port;
-  if (localAddress) {
-    name += ':' + localAddress;
-  }
+
+// Get the key for a given set of request options
+Agent.prototype.getName = function(options) {
+  var name = '';
+
+  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.port = port;
-  options.host = host;
-  options.localAddress = localAddress;
+  options = util._extend({}, options);
+  options = util._extend(options, self.options);
 
-  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
-    //  because it'll be locked up indefinitely
-    self.removeSocket(s, name, host, port, localAddress);
+    // (defined by WebSockets) where we need to remove a socket from the
+    // pool because it'll be locked up indefinitely
+    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
-    this.createSocket(name, host, port, localAddress, req).emit('free');
+    // If we have pending requests and a socket gets closed make a new one
+    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);
 };
 
diff --git a/lib/_http_client.js b/lib/_http_client.js
index 0f55148..0bc78ab 100644
--- a/lib/_http_client.js
+++ b/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) {
-      self.onSocket(options.createConnection(self.socketPath));
-    } else {
-      self.onSocket(net.createConnection(self.socketPath));
-    }
+    var conn = self.agent.createConnection({ path: self.socketPath });
+    self.onSocket(conn);
   } 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({
-        port: port,
-        host: host,
-        localAddress: options.localAddress
-      });
+      debug('CLIENT use net.createConnection', options);
+      var conn = net.createConnection(options);
     }
     self.onSocket(conn);
   }
@@ -134,8 +127,8 @@ function ClientRequest(options, cb) {
     self._flush();
     self = null;
   });
-
 }
+
 util.inherits(ClientRequest, OutgoingMessage);
 
 exports.ClientRequest = ClientRequest;
diff --git a/lib/https.js b/lib/https.js
index 18789ad..afbdef1 100644
--- a/lib/https.js
+++ b/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') {
-    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);
+  return globalAgent.request(options, cb);
 };
 
 exports.get = function(options, cb) {
-  var req = exports.request(options, cb);
-  req.end();
-  return req;
+  return globalAgent.get(options, cb);
 };
diff --git a/test/simple/test-http-agent-destroyed-socket.js b/test/simple/test-http-agent-destroyed-socket.js
index 960b3a3..be90bc6 100644
--- a/test/simple/test-http-agent-destroyed-socket.js
+++ b/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() {
diff --git a/test/simple/test-http-client-agent.js b/test/simple/test-http-client-agent.js
index aedf64b..ad5c149 100644
--- a/test/simple/test-http-client-agent.js
+++ b/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;
 
diff --git a/test/simple/test-http-keep-alive-close-on-header.js b/test/simple/test-http-keep-alive-close-on-header.js
index 53c73ae..4318bd9 100644
--- a/test/simple/test-http-keep-alive-close-on-header.js
+++ b/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();
diff --git a/test/simple/test-http-keep-alive.js b/test/simple/test-http-keep-alive.js
index 4e8a6e8..75049c9 100644
--- a/test/simple/test-http-keep-alive.js
+++ b/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({
diff --git a/test/simple/test-regress-GH-1531.js b/test/simple/test-regress-GH-1531.js
index 60cd949..53bc2a3 100644
--- a/test/simple/test-regress-GH-1531.js
+++ b/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');
 });
 
diff --git a/test/simple/test-regress-GH-877.js b/test/simple/test-regress-GH-877.js
index 1cdca9f..30e1f80 100644
--- a/test/simple/test-regress-GH-877.js
+++ b/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;
commit 3d979c3f26a6562f3744c4ca80c8b0716ec4a69f
Author: Michael Gorven <mgor...@fb.com>
Date:   Fri Apr 18 09:33:49 2014 -0700

    Fix various bugs with connection pool handling.
    
    * Adding requests to the queue used wrong parameters.
    * onCloseOrRemove() didn't remove sockets from the pool.
    * When using TLS, the pool contained the underlying connection instead
      of the TLS wrapped connection, causing connections to never be
      removed.
    * When a connection was closed and removeSocket() created a new socket
      to process requests in the queue, the event handlers were not attached
      to the new socket, causing the new socket to not be removed from the
      pool when it closed.

diff --git a/index.js b/index.js
index 13c0427..da516ec 100644
--- a/index.js
+++ b/index.js
@@ -81,23 +81,29 @@ TunnelingAgent.prototype.addRequest = function addRequest(req, options) {
 
   if (self.sockets.length >= this.maxSockets) {
     // We are over limit so we'll add it to the queue.
-    self.requests.push({host: host, port: port, request: req})
+    self.requests.push({host: options.host, port: options.port, request: req})
     return
   }
 
   // If we are under maxSockets create a new one.
-  self.createSocket({host: options.host, port: options.port, request: req}, function(socket) {
+  self.createConnection({host: options.host, port: options.port, request: req})
+}
+
+TunnelingAgent.prototype.createConnection = function createConnection(pending) {
+  var self = this
+
+  self.createSocket(pending, function(socket) {
     socket.on('free', onFree)
     socket.on('close', onCloseOrRemove)
     socket.on('agentRemove', onCloseOrRemove)
-    req.onSocket(socket)
+    pending.request.onSocket(socket)
 
     function onFree() {
-      self.emit('free', socket, options.host, options.port)
+      self.emit('free', socket, pending.host, pending.port)
     }
 
     function onCloseOrRemove(err) {
-      self.removeSocket()
+      self.removeSocket(socket)
       socket.removeListener('free', onFree)
       socket.removeListener('close', onCloseOrRemove)
       socket.removeListener('agentRemove', onCloseOrRemove)
@@ -182,9 +188,7 @@ TunnelingAgent.prototype.removeSocket = function removeSocket(socket) {
   if (pending) {
     // If we have pending requests and a socket gets closed a new one
     // needs to be created to take over in the pool for the one that closed.
-    this.createSocket(pending, function(socket) {
-      pending.request.onSocket(socket)
-    })
+    this.createConnection(pending)
   }
 }
 
@@ -197,6 +201,7 @@ function createSecureSocket(options, cb) {
       , socket: socket
       }
     ))
+    self.sockets[self.sockets.indexOf(socket)] = secureSocket
     cb(secureSocket)
   })
 }

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Pkg-javascript-devel mailing list
Pkg-javascript-devel@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-javascript-devel

Reply via email to