benchmark: fix shadowing that broke h=20 on incoming_headers benchmark

Previously the inner 'headers' variable shadowed the outer, meaning that
headers=20 sends the same 7 headers as headers=0.

Signed-off-by: Tim Perry <pimterry@gmail.com>
PR-URL: #66257
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Author: pimterry

benchmark/http/incoming_headers.js

@@ -6,16 +6,20 @@ const bench = common.createBenchmark(main, {
   connections: [50], // Concurrent connections
   headers: [20], // Number of header lines to append after the common headers
   w: [0, 6], // Amount of trailing whitespace
+  read: [0, 1], // Whether the handler reads req.headers
   duration: 5,
 });
 
-function main({ connections, headers, w, duration }) {
+function main({ connections, headers, w, read, duration }) {
   const server = http.createServer((req, res) => {
+    if (read && req.headers.host === undefined) {
+      throw new Error('Missing Host header');
+    }
     res.end();
   });
 
   server.listen(0, () => {
-    const headers = {
+    const requestHeaders = {
       'Content-Type': 'text/plain',
       'Accept': 'text/plain',
       'User-Agent': 'nodejs-benchmark',
@@ -28,12 +32,12 @@ function main({ connections, headers, w, duration }) {
       // - wrk can only send trailing OWS. This is a side-effect of wrk
       // processing requests with http-parser before sending them, causing
       // leading OWS to be stripped.
-      headers[`foo${i}`] = `some header value ${i}${' \t'.repeat(w / 2)}`;
+      requestHeaders[`foo${i}`] = `some header value ${i}${' \t'.repeat(w / 2)}`;
     }
     bench.http({
       path: '/',
       connections,
-      headers,
+      headers: requestHeaders,
       duration,
       port: server.address().port,
     }, () => {

lib/_http_incoming.js

@@ -334,109 +334,161 @@ function _addHeaderLines(headers, n) {
 }
 
 
-// This function is used to help avoid the lowercasing of a field name if it
-// matches a 'traditional cased' version of a field name. It then returns the
-// lowercased name to both avoid calling toLowerCase() a second time and to
-// indicate whether the field was a 'no duplicates' field. If a field is not a
-// 'no duplicates' field, a `0` byte is prepended as a flag. The one exception
-// to this is the Set-Cookie header which is indicated by a `1` byte flag, since
-// it is an 'array' field and thus is treated differently in _addHeaderLines().
+// How repeated instances of a header field are combined in the headers
+// object. Fields that are not known are joined with ', '.
+const kFirstWins = 0;
+const kJoinComma = 1;
+const kJoinSemicolon = 2;
+const kArray = 3;
+
+function knownField(name, merge) {
+  return { name, merge };
+}
+
+// Later values are dropped, unless joinDuplicateHeaders is set.
+const kAge = knownField('age', kFirstWins);
+const kHost = knownField('host', kFirstWins);
+const kFrom = knownField('from', kFirstWins);
+const kETag = knownField('etag', kFirstWins);
+const kServer = knownField('server', kFirstWins);
+const kReferer = knownField('referer', kFirstWins);
+const kExpires = knownField('expires', kFirstWins);
+const kLocation = knownField('location', kFirstWins);
+const kUserAgent = knownField('user-agent', kFirstWins);
+const kRetryAfter = knownField('retry-after', kFirstWins);
+const kContentType = knownField('content-type', kFirstWins);
+const kMaxForwards = knownField('max-forwards', kFirstWins);
+const kAuthorization = knownField('authorization', kFirstWins);
+const kLastModified = knownField('last-modified', kFirstWins);
+const kContentLength = knownField('content-length', kFirstWins);
+const kIfModifiedSince = knownField('if-modified-since', kFirstWins);
+const kProxyAuthorization = knownField('proxy-authorization', kFirstWins);
+const kIfUnmodifiedSince = knownField('if-unmodified-since', kFirstWins);
+
+// Values are joined with ', '.
+const kDate = knownField('date', kJoinComma);
+const kVary = knownField('vary', kJoinComma);
+const kOrigin = knownField('origin', kJoinComma);
+const kExpect = knownField('expect', kJoinComma);
+const kAccept = knownField('accept', kJoinComma);
+const kUpgrade = knownField('upgrade', kJoinComma);
+const kIfMatch = knownField('if-match', kJoinComma);
+const kConnection = knownField('connection', kJoinComma);
+const kCacheControl = knownField('cache-control', kJoinComma);
+const kIfNoneMatch = knownField('if-none-match', kJoinComma);
+const kAcceptEncoding = knownField('accept-encoding', kJoinComma);
+const kAcceptLanguage = knownField('accept-language', kJoinComma);
+const kXForwardedFor = knownField('x-forwarded-for', kJoinComma);
+const kContentEncoding = knownField('content-encoding', kJoinComma);
+const kXForwardedHost = knownField('x-forwarded-host', kJoinComma);
+const kTransferEncoding = knownField('transfer-encoding', kJoinComma);
+const kXForwardedProto = knownField('x-forwarded-proto', kJoinComma);
+
+// Values are joined with '; '.
+const kCookie = knownField('cookie', kJoinSemicolon);
+
+// Values are collected into an array.
+const kSetCookie = knownField('set-cookie', kArray);
+
+// Returns the descriptor of a known field, or the lowercased name of any other
+// field. The 'traditional cased' and lowercase spellings of known fields are
+// matched first to avoid calling toLowerCase() for them.
 // TODO: perhaps http_parser could be returning both raw and lowercased versions
 // of known header names to avoid us having to call toLowerCase() for those
 // headers.
 function matchKnownFields(field, lowercased) {
   switch (field.length) {
     case 3:
-      if (field === 'Age' || field === 'age') return 'age';
+      if (field === 'Age' || field === 'age') return kAge;
       break;
     case 4:
-      if (field === 'Host' || field === 'host') return 'host';
-      if (field === 'From' || field === 'from') return 'from';
-      if (field === 'ETag' || field === 'etag') return 'etag';
-      if (field === 'Date' || field === 'date') return '\u0000date';
-      if (field === 'Vary' || field === 'vary') return '\u0000vary';
+      if (field === 'Host' || field === 'host') return kHost;
+      if (field === 'From' || field === 'from') return kFrom;
+      if (field === 'ETag' || field === 'etag') return kETag;
+      if (field === 'Date' || field === 'date') return kDate;
+      if (field === 'Vary' || field === 'vary') return kVary;
       break;
     case 6:
-      if (field === 'Server' || field === 'server') return 'server';
-      if (field === 'Cookie' || field === 'cookie') return '\u0002cookie';
-      if (field === 'Origin' || field === 'origin') return '\u0000origin';
-      if (field === 'Expect' || field === 'expect') return '\u0000expect';
-      if (field === 'Accept' || field === 'accept') return '\u0000accept';
+      if (field === 'Server' || field === 'server') return kServer;
+      if (field === 'Cookie' || field === 'cookie') return kCookie;
+      if (field === 'Origin' || field === 'origin') return kOrigin;
+      if (field === 'Expect' || field === 'expect') return kExpect;
+      if (field === 'Accept' || field === 'accept') return kAccept;
       break;
     case 7:
-      if (field === 'Referer' || field === 'referer') return 'referer';
-      if (field === 'Expires' || field === 'expires') return 'expires';
-      if (field === 'Upgrade' || field === 'upgrade') return '\u0000upgrade';
+      if (field === 'Referer' || field === 'referer') return kReferer;
+      if (field === 'Expires' || field === 'expires') return kExpires;
+      if (field === 'Upgrade' || field === 'upgrade') return kUpgrade;
       break;
     case 8:
       if (field === 'Location' || field === 'location')
-        return 'location';
+        return kLocation;
       if (field === 'If-Match' || field === 'if-match')
-        return '\u0000if-match';
+        return kIfMatch;
       break;
     case 10:
       if (field === 'User-Agent' || field === 'user-agent')
-        return 'user-agent';
+        return kUserAgent;
       if (field === 'Set-Cookie' || field === 'set-cookie')
-        return '\u0001';
+        return kSetCookie;
       if (field === 'Connection' || field === 'connection')
-        return '\u0000connection';
+        return kConnection;
       break;
     case 11:
       if (field === 'Retry-After' || field === 'retry-after')
-        return 'retry-after';
+        return kRetryAfter;
       break;
     case 12:
       if (field === 'Content-Type' || field === 'content-type')
-        return 'content-type';
+        return kContentType;
       if (field === 'Max-Forwards' || field === 'max-forwards')
-        return 'max-forwards';
+        return kMaxForwards;
       break;
     case 13:
       if (field === 'Authorization' || field === 'authorization')
-        return 'authorization';
+        return kAuthorization;
       if (field === 'Last-Modified' || field === 'last-modified')
-        return 'last-modified';
+        return kLastModified;
       if (field === 'Cache-Control' || field === 'cache-control')
-        return '\u0000cache-control';
+        return kCacheControl;
       if (field === 'If-None-Match' || field === 'if-none-match')
-        return '\u0000if-none-match';
+        return kIfNoneMatch;
       break;
     case 14:
       if (field === 'Content-Length' || field === 'content-length')
-        return 'content-length';
+        return kContentLength;
       break;
     case 15:
       if (field === 'Accept-Encoding' || field === 'accept-encoding')
-        return '\u0000accept-encoding';
+        return kAcceptEncoding;
       if (field === 'Accept-Language' || field === 'accept-language')
-        return '\u0000accept-language';
+        return kAcceptLanguage;
       if (field === 'X-Forwarded-For' || field === 'x-forwarded-for')
-        return '\u0000x-forwarded-for';
+        return kXForwardedFor;
       break;
     case 16:
       if (field === 'Content-Encoding' || field === 'content-encoding')
-        return '\u0000content-encoding';
+        return kContentEncoding;
       if (field === 'X-Forwarded-Host' || field === 'x-forwarded-host')
-        return '\u0000x-forwarded-host';
+        return kXForwardedHost;
       break;
     case 17:
       if (field === 'If-Modified-Since' || field === 'if-modified-since')
-        return 'if-modified-since';
+        return kIfModifiedSince;
       if (field === 'Transfer-Encoding' || field === 'transfer-encoding')
-        return '\u0000transfer-encoding';
+        return kTransferEncoding;
       if (field === 'X-Forwarded-Proto' || field === 'x-forwarded-proto')
-        return '\u0000x-forwarded-proto';
+        return kXForwardedProto;
       break;
     case 19:
       if (field === 'Proxy-Authorization' || field === 'proxy-authorization')
-        return 'proxy-authorization';
+        return kProxyAuthorization;
       if (field === 'If-Unmodified-Since' || field === 'if-unmodified-since')
-        return 'if-unmodified-since';
+        return kIfUnmodifiedSince;
       break;
   }
   if (lowercased) {
-    return '\u0000' + field;
+    return field;
   }
   return matchKnownFields(field.toLowerCase(), true);
 }
@@ -447,21 +499,25 @@ function matchKnownFields(field, lowercased) {
 // multiple values this way. The one exception to this is the Cookie header,
 // which has multiple values joined with a '; ' instead. If a header's values
 // cannot be joined in either of these ways, we declare the first instance the
-// winner and drop the second. Extended header fields (those beginning with
-// 'x-') are always joined.
+// winner and drop the second. Fields that are not known are always joined.
 IncomingMessage.prototype._addHeaderLine = _addHeaderLine;
 function _addHeaderLine(field, value, dest) {
-  field = matchKnownFields(field);
-  const flag = field.charCodeAt(0);
-  if (flag === 0 || flag === 2) {
-    field = field.slice(1);
+  const known = matchKnownFields(field);
+  let merge = kJoinComma;
+  if (typeof known === 'string') {
+    field = known;
+  } else {
+    field = known.name;
+    merge = known.merge;
+  }
+  if (merge === kJoinComma || merge === kJoinSemicolon) {
     // Make a delimited list
     if (typeof dest[field] === 'string') {
-      dest[field] += (flag === 0 ? ', ' : '; ') + value;
+      dest[field] += (merge === kJoinComma ? ', ' : '; ') + value;
     } else {
       dest[field] = value;
     }
-  } else if (flag === 1) {
+  } else if (merge === kArray) {
     // Array header -- only Set-Cookie at the moment
     if (dest['set-cookie'] !== undefined) {
       dest['set-cookie'].push(value);

test/parallel/test-http-incoming-matchKnownFields.js

@@ -91,3 +91,21 @@ checkDest('X-Forwarded-Proto', { 'x-forwarded-proto': undefined });
 checkDest('x-forwarded-proto', { 'x-forwarded-proto': 'test, value' }, 'value');
 checkDest('X-Foo', { 'x-foo': undefined });
 checkDest('x-foo', { 'x-foo': 'test, value' }, 'value');
+
+// Known fields in other casings are merged by the same rules as their usual
+// spellings.
+checkDest('CONTENT-TYPE', { 'content-type': 'test' }, 'value');
+checkDest('CONNECTION', { connection: 'test, value' }, 'value');
+checkDest('COOKIE', { cookie: 'test; value' }, 'value');
+checkDest('SET-COOKIE', { 'set-cookie': ['test', 'value'] }, 'value');
+checkDest('X-FOO', { 'x-foo': 'test, value' }, 'value');
+
+// joinDuplicateHeaders also applies to first-wins fields in other casings.
+{
+  const incomingMessage = new IncomingMessage();
+  incomingMessage.joinDuplicateHeaders = true;
+  const dest = {};
+  incomingMessage._addHeaderLine('AUTHORIZATION', 'a', dest);
+  incomingMessage._addHeaderLine('Authorization', 'b', dest);
+  assert.deepStrictEqual(dest, { authorization: 'a, b' });
+}