From 83432bfff1e21797e497aacf4c473db1345f6334 Mon Sep 17 00:00:00 2001
From: Brian White <mscdex@mscdex.net>
Date: Sat, 7 May 2016 17:03:35 -0400
Subject: [PATCH] http: optimize checkInvalidHeaderChar()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This commit optimizes checkInvalidHeaderChar() by unrolling the
character checking loop a bit.

Additionally, some changes to the benchmark runner are needed in
order for the included benchmark to be run correctly. Specifically,
the regexp used to parse `key=value` parameters contained a greedy
quantifier that was causing the `key` to match part of the `value`
if `value` contained an equals sign.

PR-URL: https://github.com/nodejs/node/pull/6570
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
---
 benchmark/common.js                         |  8 ++--
 benchmark/http/check_invalid_header_char.js | 42 +++++++++++++++++++++
 lib/_http_common.js                         | 29 +++++++++++---
 3 files changed, 69 insertions(+), 10 deletions(-)
 create mode 100644 benchmark/http/check_invalid_header_char.js

diff --git a/benchmark/common.js b/benchmark/common.js
index 71b93d038a..d937e13d93 100644
--- a/benchmark/common.js
+++ b/benchmark/common.js
@@ -191,7 +191,7 @@ function parseOpts(options) {
   var num = keys.length;
   var conf = {};
   for (var i = 2; i < process.argv.length; i++) {
-    var match = process.argv[i].match(/^(.+)=(.*)$/);
+    var match = process.argv[i].match(/^(.+?)=([\s\S]*)$/);
     if (!match || !match[1] || !options[match[1]]) {
       return null;
     } else {
@@ -238,8 +238,6 @@ Benchmark.prototype.report = function(value) {
     console.log('%s: %s', heading, value.toFixed(5));
   else if (outputFormat == 'csv')
     console.log('%s,%s', heading, value.toFixed(5));
-
-  process.exit(0);
 };
 
 Benchmark.prototype.getHeading = function() {
@@ -247,11 +245,11 @@ Benchmark.prototype.getHeading = function() {
 
   if (outputFormat == 'default') {
     return this._name + ' ' + Object.keys(conf).map(function(key) {
-      return key + '=' + conf[key];
+      return key + '=' + JSON.stringify('' + conf[key]);
     }).join(' ');
   } else if (outputFormat == 'csv') {
     return this._name + ',' + Object.keys(conf).map(function(key) {
-      return conf[key];
+      return JSON.stringify('' + conf[key]);
     }).join(',');
   }
 };
diff --git a/benchmark/http/check_invalid_header_char.js b/benchmark/http/check_invalid_header_char.js
new file mode 100644
index 0000000000..bef44e6316
--- /dev/null
+++ b/benchmark/http/check_invalid_header_char.js
@@ -0,0 +1,42 @@
+'use strict';
+
+const common = require('../common.js');
+const _checkInvalidHeaderChar = require('_http_common')._checkInvalidHeaderChar;
+
+const bench = common.createBenchmark(main, {
+  key: [
+    // Valid
+    '',
+    '1',
+    '\t\t\t\t\t\t\t\t\t\tFoo bar baz',
+    'keep-alive',
+    'close',
+    'gzip',
+    '20091',
+    'private',
+    'text/html; charset=utf-8',
+    'text/plain',
+    'Sat, 07 May 2016 16:54:48 GMT',
+    'SAMEORIGIN',
+    'en-US',
+
+    // Invalid
+    'Here is a value that is really a folded header value\r\n  this should be \
+     supported, but it is not currently',
+    '中文呢', // unicode
+    'foo\nbar',
+    '\x7F'
+  ],
+  n: [5e8],
+});
+
+function main(conf) {
+  var n = +conf.n;
+  var key = conf.key;
+
+  bench.start();
+  for (var i = 0; i < n; i++) {
+    _checkInvalidHeaderChar(key);
+  }
+  bench.end(n);
+}
diff --git a/lib/_http_common.js b/lib/_http_common.js
index 150d424261..ad0389ba21 100644
--- a/lib/_http_common.js
+++ b/lib/_http_common.js
@@ -301,13 +301,32 @@ exports._checkIsHttpToken = checkIsHttpToken;
  *  field-value    = *( field-content / obs-fold )
  *  field-content  = field-vchar [ 1*( SP / HTAB ) field-vchar ]
  *  field-vchar    = VCHAR / obs-text
+ *
+ * checkInvalidHeaderChar() is currently designed to be inlinable by v8,
+ * so take care when making changes to the implementation so that the source
+ * code size does not exceed v8's default max_inlined_source_size setting.
  **/
 function checkInvalidHeaderChar(val) {
-  val = '' + val;
-  for (var i = 0; i < val.length; i++) {
-    const ch = val.charCodeAt(i);
-    if (ch === 9) continue;
-    if (ch <= 31 || ch > 255 || ch === 127) return true;
+  val += '';
+  if (val.length < 1)
+    return false;
+  var c = val.charCodeAt(0);
+  if ((c <= 31 && c !== 9) || c > 255 || c === 127)
+    return true;
+  if (val.length < 2)
+    return false;
+  c = val.charCodeAt(1);
+  if ((c <= 31 && c !== 9) || c > 255 || c === 127)
+    return true;
+  if (val.length < 3)
+    return false;
+  c = val.charCodeAt(2);
+  if ((c <= 31 && c !== 9) || c > 255 || c === 127)
+    return true;
+  for (var i = 3; i < val.length; ++i) {
+    c = val.charCodeAt(i);
+    if ((c <= 31 && c !== 9) || c > 255 || c === 127)
+      return true;
   }
   return false;
 }