http: refactor outgoing headers processing

Use a shared function, for..in instead of Object.keys, do less work in
`setHeader` and instead defer some of it until later, and other minor
changes to improve clarity, as well as a slight boost in performance.

PR-URL: https://github.com/nodejs/node/pull/20250
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

benchmark/http/headers.js

@@ -0,0 +1,36 @@
+'use strict';
+
+const common = require('../common.js');
+const http = require('http');
+
+const bench = common.createBenchmark(main, {
+  duplicates: [1, 100],
+  n: [10, 1000],
+});
+
+function main({ duplicates, n }) {
+  const headers = {
+    'Connection': 'keep-alive',
+    'Transfer-Encoding': 'chunked',
+  };
+
+  for (var i = 0; i < n / duplicates; i++) {
+    headers[`foo${i}`] = [];
+    for (var j = 0; j < duplicates; j++) {
+      headers[`foo${i}`].push(`some header value ${i}`);
+    }
+  }
+
+  const server = http.createServer(function(req, res) {
+    res.writeHead(200, headers);
+    res.end();
+  });
+  server.listen(common.PORT, function() {
+    bench.http({
+      path: '/',
+      connections: 10
+    }, function() {
+      server.close();
+    });
+  });
+}

lib/_http_outgoing.js

@@ -52,6 +52,8 @@ const { utcDate } = internalHttp;
 
 const kIsCorked = Symbol('isCorked');
 
+const hasOwnProperty = Function.call.bind(Object.prototype.hasOwnProperty);
+
 var RE_CONN_CLOSE = /(?:^|\W)close(?:$|\W)/i;
 var RE_TE_CHUNKED = common.chunkExpression;
 
@@ -116,7 +118,7 @@ Object.defineProperty(OutgoingMessage.prototype, '_headers', {
     if (val == null) {
       this[outHeadersKey] = null;
     } else if (typeof val === 'object') {
-      const headers = this[outHeadersKey] = {};
+      const headers = this[outHeadersKey] = Object.create(null);
       const keys = Object.keys(val);
       for (var i = 0; i < keys.length; ++i) {
         const name = keys[i];
@@ -129,7 +131,7 @@ Object.defineProperty(OutgoingMessage.prototype, '_headers', {
 Object.defineProperty(OutgoingMessage.prototype, '_headerNames', {
   get: function() {
     const headers = this[outHeadersKey];
-    if (headers) {
+    if (headers !== null) {
       const out = Object.create(null);
       const keys = Object.keys(headers);
       for (var i = 0; i < keys.length; ++i) {
@@ -138,9 +140,8 @@ Object.defineProperty(OutgoingMessage.prototype, '_headerNames', {
         out[key] = val;
       }
       return out;
-    } else {
-      return headers;
     }
+    return null;
   },
   set: function(val) {
     if (typeof val === 'object' && val !== null) {
@@ -164,14 +165,14 @@ OutgoingMessage.prototype._renderHeaders = function _renderHeaders() {
   }
 
   var headersMap = this[outHeadersKey];
-  if (!headersMap) return {};
+  const headers = {};
 
-  var headers = {};
+  if (headersMap !== null) {
-  var keys = Object.keys(headersMap);
+    const keys = Object.keys(headersMap);
-
+    for (var i = 0, l = keys.length; i < l; i++) {
-  for (var i = 0, l = keys.length; i < l; i++) {
+      const key = keys[i];
-    var key = keys[i];
+      headers[headersMap[key][0]] = headersMap[key][1];
-    headers[headersMap[key][0]] = headersMap[key][1];
+    }
   }
   return headers;
 };
@@ -285,72 +286,40 @@ OutgoingMessage.prototype._storeHeader = _storeHeader;
 function _storeHeader(firstLine, headers) {
   // firstLine in the case of request is: 'GET /index.html HTTP/1.1\r\n'
   // in the case of response it is: 'HTTP/1.1 200 OK\r\n'
-  var state = {
+  const state = {
     connection: false,
     contLen: false,
     te: false,
     date: false,
     expect: false,
     trailer: false,
-    upgrade: false,
     header: firstLine
   };
 
-  var field;
   var key;
-  var value;
-  var i;
-  var j;
   if (headers === this[outHeadersKey]) {
     for (key in headers) {
-      var entry = headers[key];
+      const entry = headers[key];
-      field = entry[0];
+      processHeader(this, state, entry[0], entry[1], false);
-      value = entry[1];
-
-      if (value instanceof Array) {
-        if (value.length < 2 || !isCookieField(field)) {
-          for (j = 0; j < value.length; j++)
-            storeHeader(this, state, field, value[j], false);
-          continue;
-        }
-        value = value.join('; ');
-      }
-      storeHeader(this, state, field, value, false);
     }
-  } else if (headers instanceof Array) {
+  } else if (Array.isArray(headers)) {
-    for (i = 0; i < headers.length; i++) {
+    for (var i = 0; i < headers.length; i++) {
-      field = headers[i][0];
+      const entry = headers[i];
-      value = headers[i][1];
+      processHeader(this, state, entry[0], entry[1], true);
-
-      if (value instanceof Array) {
-        for (j = 0; j < value.length; j++) {
-          storeHeader(this, state, field, value[j], true);
-        }
-      } else {
-        storeHeader(this, state, field, value, true);
-      }
     }
   } else if (headers) {
-    var keys = Object.keys(headers);
+    for (key in headers) {
-    for (i = 0; i < keys.length; i++) {
+      if (hasOwnProperty(headers, key)) {
-      field = keys[i];
+        processHeader(this, state, key, headers[key], true);
-      value = headers[field];
-
-      if (value instanceof Array) {
-        if (value.length < 2 || !isCookieField(field)) {
-          for (j = 0; j < value.length; j++)
-            storeHeader(this, state, field, value[j], true);
-          continue;
-        }
-        value = value.join('; ');
       }
-      storeHeader(this, state, field, value, true);
     }
   }
 
+  let { header } = state;
+
   // Date header
   if (this.sendDate && !state.date) {
-    state.header += 'Date: ' + utcDate() + CRLF;
+    header += 'Date: ' + utcDate() + CRLF;
   }
 
   // Force the connection to close when the response is a 204 No Content or
@@ -364,9 +333,9 @@ function _storeHeader(firstLine, headers) {
   // It was pointed out that this might confuse reverse proxies to the point
   // of creating security liabilities, so suppress the zero chunk and force
   // the connection to close.
-  var statusCode = this.statusCode;
+  if (this.chunkedEncoding && (this.statusCode === 204 ||
-  if ((statusCode === 204 || statusCode === 304) && this.chunkedEncoding) {
+                               this.statusCode === 304)) {
-    debug(statusCode + ' response should not use chunked encoding,' +
+    debug(this.statusCode + ' response should not use chunked encoding,' +
           ' closing connection.');
     this.chunkedEncoding = false;
     this.shouldKeepAlive = false;
@@ -377,13 +346,13 @@ function _storeHeader(firstLine, headers) {
     this._last = true;
     this.shouldKeepAlive = false;
   } else if (!state.connection) {
-    var shouldSendKeepAlive = this.shouldKeepAlive &&
+    const shouldSendKeepAlive = this.shouldKeepAlive &&
         (state.contLen || this.useChunkedEncodingByDefault || this.agent);
     if (shouldSendKeepAlive) {
-      state.header += 'Connection: keep-alive\r\n';
+      header += 'Connection: keep-alive\r\n';
     } else {
       this._last = true;
-      state.header += 'Connection: close\r\n';
+      header += 'Connection: close\r\n';
     }
   }
 
@@ -396,9 +365,9 @@ function _storeHeader(firstLine, headers) {
     } else if (!state.trailer &&
                !this._removedContLen &&
                typeof this._contentLength === 'number') {
-      state.header += 'Content-Length: ' + this._contentLength + CRLF;
+      header += 'Content-Length: ' + this._contentLength + CRLF;
     } else if (!this._removedTE) {
-      state.header += 'Transfer-Encoding: chunked\r\n';
+      header += 'Transfer-Encoding: chunked\r\n';
       this.chunkedEncoding = true;
     } else {
       // We should only be able to get here if both Content-Length and
@@ -416,7 +385,7 @@ function _storeHeader(firstLine, headers) {
     throw new ERR_HTTP_TRAILER_INVALID();
   }
 
-  this._header = state.header + CRLF;
+  this._header = header + CRLF;
   this._headerSent = false;
 
   // wait until the first body chunk, or close(), is sent to flush,
@@ -424,10 +393,23 @@ function _storeHeader(firstLine, headers) {
   if (state.expect) this._send('');
 }
 
-function storeHeader(self, state, key, value, validate) {
+function processHeader(self, state, key, value, validate) {
-  if (validate) {
+  if (validate)
-    validateHeader(key, value);
+    validateHeaderName(key);
+  if (Array.isArray(value)) {
+    if (value.length < 2 || !isCookieField(key)) {
+      for (var i = 0; i < value.length; i++)
+        storeHeader(self, state, key, value[i], validate);
+      return;
+    }
+    value = value.join('; ');
   }
+  storeHeader(self, state, key, value, validate);
+}
+
+function storeHeader(self, state, key, value, validate) {
+  if (validate)
+    validateHeaderValue(key, value);
   state.header += key + ': ' + escapeHeaderValue(value) + CRLF;
   matchHeader(self, state, key, value);
 }
@@ -439,6 +421,7 @@ function matchHeader(self, state, field, value) {
   switch (field) {
     case 'connection':
       state.connection = true;
+      self._removedConnection = false;
       if (RE_CONN_CLOSE.test(value))
         self._last = true;
       else
@@ -446,32 +429,39 @@ function matchHeader(self, state, field, value) {
       break;
     case 'transfer-encoding':
       state.te = true;
+      self._removedTE = false;
       if (RE_TE_CHUNKED.test(value)) self.chunkedEncoding = true;
       break;
     case 'content-length':
       state.contLen = true;
+      self._removedContLen = false;
       break;
     case 'date':
     case 'expect':
     case 'trailer':
-    case 'upgrade':
       state[field] = true;
       break;
   }
 }
 
-function validateHeader(name, value) {
+function validateHeaderName(name) {
-  let err;
   if (typeof name !== 'string' || !name || !checkIsHttpToken(name)) {
-    err = new ERR_INVALID_HTTP_TOKEN('Header name', name);
+    const err = new ERR_INVALID_HTTP_TOKEN('Header name', name);
-  } else if (value === undefined) {
+    Error.captureStackTrace(err, validateHeaderName);
+    throw err;
+  }
+}
+
+function validateHeaderValue(name, value) {
+  let err;
+  if (value === undefined) {
     err = new ERR_HTTP_INVALID_HEADER_VALUE(value, name);
   } else if (checkInvalidHeaderChar(value)) {
     debug('Header "%s" contains invalid characters', name);
     err = new ERR_INVALID_CHAR('header content', name);
   }
   if (err !== undefined) {
-    Error.captureStackTrace(err, validateHeader);
+    Error.captureStackTrace(err, validateHeaderValue);
     throw err;
   }
 }
@@ -480,25 +470,14 @@ OutgoingMessage.prototype.setHeader = function setHeader(name, value) {
   if (this._header) {
     throw new ERR_HTTP_HEADERS_SENT('set');
   }
-  validateHeader(name, value);
+  validateHeaderName(name);
+  validateHeaderValue(name, value);
 
-  if (!this[outHeadersKey])
+  let headers = this[outHeadersKey];
-    this[outHeadersKey] = {};
+  if (headers === null)
+    this[outHeadersKey] = headers = Object.create(null);
 
-  const key = name.toLowerCase();
+  headers[name.toLowerCase()] = [name, value];
-  this[outHeadersKey][key] = [name, value];
-
-  switch (key) {
-    case 'connection':
-      this._removedConnection = false;
-      break;
-    case 'content-length':
-      this._removedContLen = false;
-      break;
-    case 'transfer-encoding':
-      this._removedTE = false;
-      break;
-  }
 };
 
 
@@ -507,18 +486,18 @@ OutgoingMessage.prototype.getHeader = function getHeader(name) {
     throw new ERR_INVALID_ARG_TYPE('name', 'string', name);
   }
 
-  if (!this[outHeadersKey]) return;
+  const headers = this[outHeadersKey];
-
+  if (headers === null)
-  var entry = this[outHeadersKey][name.toLowerCase()];
-  if (!entry)
     return;
-  return entry[1];
+
+  const entry = headers[name.toLowerCase()];
+  return entry && entry[1];
 };
 
 
 // Returns an array of the names of the current outgoing headers.
 OutgoingMessage.prototype.getHeaderNames = function getHeaderNames() {
-  return (this[outHeadersKey] ? Object.keys(this[outHeadersKey]) : []);
+  return this[outHeadersKey] !== null ? Object.keys(this[outHeadersKey]) : [];
 };
 
 
@@ -543,7 +522,8 @@ OutgoingMessage.prototype.hasHeader = function hasHeader(name) {
     throw new ERR_INVALID_ARG_TYPE('name', 'string', name);
   }
 
-  return !!(this[outHeadersKey] && this[outHeadersKey][name.toLowerCase()]);
+  return this[outHeadersKey] !== null &&
+    !!this[outHeadersKey][name.toLowerCase()];
 };
 
 
@@ -573,7 +553,7 @@ OutgoingMessage.prototype.removeHeader = function removeHeader(name) {
       break;
   }
 
-  if (this[outHeadersKey]) {
+  if (this[outHeadersKey] !== null) {
     delete this[outHeadersKey][key];
   }
 };

test/parallel/test-http-outgoing-internal-headers.js

@@ -21,8 +21,10 @@ const { OutgoingMessage } = require('http');
     Origin: 'localhost'
   };
 
-  assert.deepStrictEqual(outgoingMessage[outHeadersKey], {
+  assert.deepStrictEqual(
-    host: ['host', 'risingstack.com'],
+    Object.entries(outgoingMessage[outHeadersKey]),
-    origin: ['Origin', 'localhost']
+    Object.entries({
-  });
+      host: ['host', 'risingstack.com'],
+      origin: ['Origin', 'localhost']
+    }));
 }

test/sequential/test-benchmark-http.js

@@ -18,6 +18,7 @@ runBenchmark('http',
                'chunkedEnc=true',
                'chunks=0',
                'dur=0.1',
+               'duplicates=1',
                'input=keep-alive',
                'key=""',
                'len=1',