http: throw an error for unexpected agent values

As per nodejs#9069
unexpected things can happen when supplying an
unexpected value to agent. Beings as the docs
clearly state the expected values, this throws
an error on an unexpected value.

PR-URL: nodejs#10053
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>

lib/_http_client.js

@@ -62,9 +62,16 @@ function ClientRequest(options, cb) {
   var defaultAgent = options._defaultAgent || Agent.globalAgent;
   if (agent === false) {
     agent = new defaultAgent.constructor();
-  } else if ((agent === null || agent === undefined) &&
-             typeof options.createConnection !== 'function') {
-    agent = defaultAgent;
+  } else if (agent === null || agent === undefined) {
+    if (typeof options.createConnection !== 'function') {
+      agent = defaultAgent;
+    }
+    // Explicitly pass through this statement as agent will not be used
+    // when createConnection is provided.
+  } else if (!(agent instanceof Agent.Agent)) {
+    throw new TypeError(
+      'Agent option must be an instance of http.Agent, undefined or false.'
+    );
   }
   self.agent = agent;
 

test/parallel/test-http-client-reject-unexpected-agent.js

@@ -0,0 +1,64 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const http = require('http');
+
+const baseOptions = {
+  method: 'GET',
+  port: undefined,
+  host: common.localhostIPv4,
+};
+
+const failingAgentOptions = [
+  true,
+  'agent',
+  {},
+  1,
+  () => null,
+  Symbol(),
+];
+
+const acceptableAgentOptions = [
+  false,
+  undefined,
+  null,
+  new http.Agent(),
+];
+
+const server = http.createServer((req, res) => {
+  res.end('hello');
+});
+
+let numberOfResponses = 0;
+
+function createRequest(agent) {
+  const options = Object.assign(baseOptions, {agent});
+  const request = http.request(options);
+  request.end();
+  request.on('response', common.mustCall(() => {
+    numberOfResponses++;
+    if (numberOfResponses === acceptableAgentOptions.length) {
+      server.close();
+    }
+  }));
+}
+
+server.listen(0, baseOptions.host, common.mustCall(function() {
+  baseOptions.port = this.address().port;
+
+  failingAgentOptions.forEach((agent) => {
+    assert.throws(
+      () => createRequest(agent),
+      /^TypeError: Agent option must be an instance of http.Agent/,
+      `Expected typeof agent: ${typeof agent} to be rejected`
+    );
+  });
+
+  acceptableAgentOptions.forEach((agent) => {
+    assert.doesNotThrow(() => createRequest(agent));
+  });
+}));
+
+process.on('exit', () => {
+  assert.strictEqual(numberOfResponses, acceptableAgentOptions.length);
+});