http, https: don't depend on globalAgent

TooTallNate · TooTallNate · commit d6bbb19f1d1d · 2014-02-26T13:18:54.000-08:00
For the `request()` and `get()` functions. I could never really understand why these two functions go through agent first... Especially since the user could be passing `agent: false` or a different Agent instance completely, in which `globalAgent` will be completely bypassed. Moved the relevant logic from `Agent#request()` into the `ClientRequest` constructor. Incidentally, this commit fixes nodejs#7012 (which was the original intent of this commit).
diff --git a/lib/_http_client.js b/lib/_http_client.js
@@ -21,6 +21,7 @@
 
 var util = require('util');
 var net = require('net');
+var url = require('url');
 var EventEmitter = require('events').EventEmitter;
 var HTTPParser = process.binding('http_parser').HTTPParser;
 var assert = require('assert').ok;
@@ -35,17 +36,39 @@ var debug = common.debug;
 var IncomingMessage = require('_http_incoming').IncomingMessage;
 var OutgoingMessage = require('_http_outgoing').OutgoingMessage;
 
-var agent = require('_http_agent');
-var globalAgent = agent.globalAgent;
+var Agent = require('_http_agent');
 
 
 function ClientRequest(options, cb) {
   var self = this;
   OutgoingMessage.call(self);
 
-  options = util._extend({}, options);
+  if (util.isString(options)) {
+    options = url.parse(options);
+  } else {
+    options = util._extend({}, options);
+  }
 
-  self.agent = util.isUndefined(options.agent) ? globalAgent : options.agent;
+  var agent = options.agent;
+  var defaultAgent = options._defaultAgent || Agent.globalAgent;
+  if (agent === false) {
+    agent = new defaultAgent.constructor();
+  } else if (util.isNullOrUndefined(agent)) {
+    agent = defaultAgent;
+  }
+  self.agent = agent;
+
+  if (options.path && / /.test(options.path)) {
+    // The actual regex is more like /[^A-Za-z0-9\-._~!$&'()*+,;=/:@]/
+    // with an additional rule for ignoring percentage-escaped characters
+    // but that's a) hard to capture in a regular expression that performs
+    // well, and b) possibly too restrictive for real-world usage. That's
+    // why it only scans for spaces because those are guaranteed to create
+    // an invalid request.
+    throw new TypeError('Request path contains unescaped characters.');
+  } else if (options.protocol && options.protocol !== self.agent.protocol) {
+    throw new Error('Protocol:' + options.protocol + ' not supported.');
+  }
 
   var defaultPort = options.defaultPort || self.agent.defaultPort;
 
@@ -114,8 +137,7 @@ function ClientRequest(options, cb) {
     // but only if the Agent will actually reuse the connection!
     // If it's not a keepAlive agent, and the maxSockets==Infinity, then
     // there's never a case where this socket will actually be reused
-    var agent = self.agent;
-    if (!agent.keepAlive && !Number.isFinite(agent.maxSockets)) {
+    if (!self.agent.keepAlive && !Number.isFinite(self.agent.maxSockets)) {
       self._last = true;
       self.shouldKeepAlive = false;
     } else {
diff --git a/lib/http.js b/lib/http.js
@@ -49,11 +49,13 @@ var client = require('_http_client');
 var ClientRequest = exports.ClientRequest = client.ClientRequest;
 
 exports.request = function(options, cb) {
-  return globalAgent.request(options, cb);
+  return new ClientRequest(options, cb);
 };
 
 exports.get = function(options, cb) {
-  return globalAgent.get(options, cb);
+  var req = exports.request(options, cb);
+  req.end();
+  return req;
 };
 
 exports._connectionListener = server._connectionListener;
diff --git a/lib/https.js b/lib/https.js
@@ -20,6 +20,7 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 var tls = require('tls');
+var url = require('url');
 var http = require('http');
 var util = require('util');
 var inherits = require('util').inherits;
@@ -126,9 +127,17 @@ exports.globalAgent = globalAgent;
 exports.Agent = Agent;
 
 exports.request = function(options, cb) {
-  return globalAgent.request(options, cb);
+  if (util.isString(options)) {
+    options = url.parse(options);
+  } else {
+    options = util._extend({}, options);
+  }
+  options._defaultAgent = globalAgent;
+  return http.request(options, cb);
 };
 
 exports.get = function(options, cb) {
-  return globalAgent.get(options, cb);
+  var req = exports.request(options, cb);
+  req.end();
+  return req;
 };
