Skip to content

Commit 2a462ba

Browse files
mscdexevanlucas
authored andcommitted
http: optimize checkInvalidHeaderChar()
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 <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Сковорода Никита Андреевич <[email protected]>
1 parent 4a63be0 commit 2a462ba

File tree

3 files changed

+69
-10
lines changed

3 files changed

+69
-10
lines changed

benchmark/common.js

+3-5
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ function parseOpts(options) {
191191
var num = keys.length;
192192
var conf = {};
193193
for (var i = 2; i < process.argv.length; i++) {
194-
var match = process.argv[i].match(/^(.+)=(.*)$/);
194+
var match = process.argv[i].match(/^(.+?)=([\s\S]*)$/);
195195
if (!match || !match[1] || !options[match[1]]) {
196196
return null;
197197
} else {
@@ -238,20 +238,18 @@ Benchmark.prototype.report = function(value) {
238238
console.log('%s: %s', heading, value.toFixed(5));
239239
else if (outputFormat == 'csv')
240240
console.log('%s,%s', heading, value.toFixed(5));
241-
242-
process.exit(0);
243241
};
244242

245243
Benchmark.prototype.getHeading = function() {
246244
var conf = this.config;
247245

248246
if (outputFormat == 'default') {
249247
return this._name + ' ' + Object.keys(conf).map(function(key) {
250-
return key + '=' + conf[key];
248+
return key + '=' + JSON.stringify('' + conf[key]);
251249
}).join(' ');
252250
} else if (outputFormat == 'csv') {
253251
return this._name + ',' + Object.keys(conf).map(function(key) {
254-
return conf[key];
252+
return JSON.stringify('' + conf[key]);
255253
}).join(',');
256254
}
257255
};
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
'use strict';
2+
3+
const common = require('../common.js');
4+
const _checkInvalidHeaderChar = require('_http_common')._checkInvalidHeaderChar;
5+
6+
const bench = common.createBenchmark(main, {
7+
key: [
8+
// Valid
9+
'',
10+
'1',
11+
'\t\t\t\t\t\t\t\t\t\tFoo bar baz',
12+
'keep-alive',
13+
'close',
14+
'gzip',
15+
'20091',
16+
'private',
17+
'text/html; charset=utf-8',
18+
'text/plain',
19+
'Sat, 07 May 2016 16:54:48 GMT',
20+
'SAMEORIGIN',
21+
'en-US',
22+
23+
// Invalid
24+
'Here is a value that is really a folded header value\r\n this should be \
25+
supported, but it is not currently',
26+
'中文呢', // unicode
27+
'foo\nbar',
28+
'\x7F'
29+
],
30+
n: [5e8],
31+
});
32+
33+
function main(conf) {
34+
var n = +conf.n;
35+
var key = conf.key;
36+
37+
bench.start();
38+
for (var i = 0; i < n; i++) {
39+
_checkInvalidHeaderChar(key);
40+
}
41+
bench.end(n);
42+
}

lib/_http_common.js

+24-5
Original file line numberDiff line numberDiff line change
@@ -301,13 +301,32 @@ exports._checkIsHttpToken = checkIsHttpToken;
301301
* field-value = *( field-content / obs-fold )
302302
* field-content = field-vchar [ 1*( SP / HTAB ) field-vchar ]
303303
* field-vchar = VCHAR / obs-text
304+
*
305+
* checkInvalidHeaderChar() is currently designed to be inlinable by v8,
306+
* so take care when making changes to the implementation so that the source
307+
* code size does not exceed v8's default max_inlined_source_size setting.
304308
**/
305309
function checkInvalidHeaderChar(val) {
306-
val = '' + val;
307-
for (var i = 0; i < val.length; i++) {
308-
const ch = val.charCodeAt(i);
309-
if (ch === 9) continue;
310-
if (ch <= 31 || ch > 255 || ch === 127) return true;
310+
val += '';
311+
if (val.length < 1)
312+
return false;
313+
var c = val.charCodeAt(0);
314+
if ((c <= 31 && c !== 9) || c > 255 || c === 127)
315+
return true;
316+
if (val.length < 2)
317+
return false;
318+
c = val.charCodeAt(1);
319+
if ((c <= 31 && c !== 9) || c > 255 || c === 127)
320+
return true;
321+
if (val.length < 3)
322+
return false;
323+
c = val.charCodeAt(2);
324+
if ((c <= 31 && c !== 9) || c > 255 || c === 127)
325+
return true;
326+
for (var i = 3; i < val.length; ++i) {
327+
c = val.charCodeAt(i);
328+
if ((c <= 31 && c !== 9) || c > 255 || c === 127)
329+
return true;
311330
}
312331
return false;
313332
}

0 commit comments

Comments
 (0)