Skip to content

Commit

Permalink
http: optimize checkInvalidHeaderChar()
Browse files Browse the repository at this point in the history
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: #6570
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
  • Loading branch information
mscdex committed Jun 14, 2016
1 parent 918159e commit 83432bf
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 10 deletions.
8 changes: 3 additions & 5 deletions benchmark/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -238,20 +238,18 @@ 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() {
var conf = this.config;

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(',');
}
};
Expand Down
42 changes: 42 additions & 0 deletions benchmark/http/check_invalid_header_char.js
Original file line number Diff line number Diff line change
@@ -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);
}
29 changes: 24 additions & 5 deletions lib/_http_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down

0 comments on commit 83432bf

Please sign in to comment.