Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 2a462ba

Browse filesBrowse 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 <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
1 parent 4a63be0 commit 2a462ba
Copy full SHA for 2a462ba

File tree

Expand file treeCollapse file tree

3 files changed

+69
-10
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+69
-10
lines changed
Open diff view settings
Collapse file

‎benchmark/common.js‎

Copy file name to clipboardExpand all lines: benchmark/common.js
+3-5Lines changed: 3 additions & 5 deletions
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
};
Collapse file
+42Lines changed: 42 additions & 0 deletions
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+
}
Collapse file

‎lib/_http_common.js‎

Copy file name to clipboardExpand all lines: lib/_http_common.js
+24-5Lines changed: 24 additions & 5 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.