http: process headers after setting up agent

Added tests to clarify the implicit behaviour of array header setting vs
object header setting

PR-URL: https://github.com/nodejs/node/pull/16568
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

1 files changed, 48 insertions, 43 deletions

diff --git a/lib/_http_client.js b/lib/_http_client.js
index a9ee686c69..ebfd809e21 100644
--- a/lib/_http_client.js
+++ b/lib/_http_client.js
@@ -132,6 +132,40 @@ function ClientRequest(options, cb) {
     this.once('response', cb);
   }
 
+  if (method === 'GET' ||
+      method === 'HEAD' ||
+      method === 'DELETE' ||
+      method === 'OPTIONS' ||
+      method === 'CONNECT') {
+    this.useChunkedEncodingByDefault = false;
+  } else {
+    this.useChunkedEncodingByDefault = true;
+  }
+
+  this._ended = false;
+  this.res = null;
+  this.aborted = undefined;
+  this.timeoutCb = null;
+  this.upgradeOrConnect = false;
+  this.parser = null;
+  this.maxHeadersCount = null;
+
+  var called = false;
+
+  if (this.agent) {
+    // If there is an agent we should default to Connection:keep-alive,
+    // 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
+    if (!this.agent.keepAlive && !Number.isFinite(this.agent.maxSockets)) {
+      this._last = true;
+      this.shouldKeepAlive = false;
+    } else {
+      this._last = false;
+      this.shouldKeepAlive = true;
+    }
+  }
+
   var headersArray = Array.isArray(options.headers);
   if (!headersArray) {
     if (options.headers) {
@@ -141,6 +175,7 @@ function ClientRequest(options, cb) {
         this.setHeader(key, options.headers[key]);
       }
     }
+
     if (host && !this.getHeader('host') && setHost) {
       var hostHeader = host;
 
@@ -159,45 +194,25 @@ function ClientRequest(options, cb) {
       }
       this.setHeader('Host', hostHeader);
     }
-  }
 
-  if (options.auth && !this.getHeader('Authorization')) {
-    this.setHeader('Authorization', 'Basic ' +
-                   Buffer.from(options.auth).toString('base64'));
-  }
+    if (options.auth && !this.getHeader('Authorization')) {
+      this.setHeader('Authorization', 'Basic ' +
+                     Buffer.from(options.auth).toString('base64'));
+    }
 
-  if (method === 'GET' ||
-      method === 'HEAD' ||
-      method === 'DELETE' ||
-      method === 'OPTIONS' ||
-      method === 'CONNECT') {
-    this.useChunkedEncodingByDefault = false;
-  } else {
-    this.useChunkedEncodingByDefault = true;
-  }
+    if (this.getHeader('expect')) {
+      if (this._header) {
+        throw new errors.Error('ERR_HTTP_HEADERS_SENT', 'render');
+      }
 
-  if (headersArray) {
-    this._storeHeader(this.method + ' ' + this.path + ' HTTP/1.1\r\n',
-                      options.headers);
-  } else if (this.getHeader('expect')) {
-    if (this._header) {
-      throw new errors.Error('ERR_HTTP_HEADERS_SENT', 'render');
+      this._storeHeader(this.method + ' ' + this.path + ' HTTP/1.1\r\n',
+                        this[outHeadersKey]);
     }
-
+  } else {
     this._storeHeader(this.method + ' ' + this.path + ' HTTP/1.1\r\n',
-                      this[outHeadersKey]);
+                      options.headers);
   }
 
-  this._ended = false;
-  this.res = null;
-  this.aborted = undefined;
-  this.timeoutCb = null;
-  this.upgradeOrConnect = false;
-  this.parser = null;
-  this.maxHeadersCount = null;
-
-  var called = false;
-
   var oncreate = (err, socket) => {
     if (called)
       return;
@@ -210,18 +225,8 @@ function ClientRequest(options, cb) {
     this._deferToConnect(null, null, () => this._flush());
   };
 
+  // initiate connection
   if (this.agent) {
-    // If there is an agent we should default to Connection:keep-alive,
-    // 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
-    if (!this.agent.keepAlive && !Number.isFinite(this.agent.maxSockets)) {
-      this._last = true;
-      this.shouldKeepAlive = false;
-    } else {
-      this._last = false;
-      this.shouldKeepAlive = true;
-    }
     this.agent.addRequest(this, options);
   } else {
     // No agent, default to Connection:close.