Skip to content

Commit 6ce8b24

Browse files
sethbrenithMylesBorins
authored andcommitted
http: simplify checkInvalidHeaderChar
In the spirit of [17399](#17399), we can also simplify checkInvalidHeaderChar to use regex matching instead of a loop. This makes it faster on long matches and slower on short matches or non-matches. This change also includes some sample data from an AcmeAir benchmark run, as a rough proxy for real-world data. PR-URL: #18381 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Benedikt Meurer <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
1 parent 849f5c3 commit 6ce8b24

File tree

2 files changed

+54
-73
lines changed

2 files changed

+54
-73
lines changed

benchmark/http/check_invalid_header_char.js

+52-29
Original file line numberDiff line numberDiff line change
@@ -3,43 +3,66 @@
33
const common = require('../common.js');
44
const _checkInvalidHeaderChar = require('_http_common')._checkInvalidHeaderChar;
55

6-
// Put it here so the benchmark result lines will not be super long.
7-
const LONG_AND_INVALID = 'Here is a value that is really a folded header ' +
8-
'value\r\n this should be supported, but it is not currently';
6+
const groupedInputs = {
7+
// Representative set of inputs from an AcmeAir benchmark run:
8+
// all valid strings, average length 14.4, stdev 13.0
9+
group_acmeair: [
10+
'W/"2-d4cbb29"', 'OK', 'Express', 'X-HTTP-Method-Override', 'Express',
11+
'application/json', 'application/json; charset=utf-8', '206', 'OK',
12+
'sessionid=; Path=/', 'text/html; charset=utf-8',
13+
'text/html; charset=utf-8', '10', 'W/"a-eda64de5"', 'OK', 'Express',
14+
'application/json', 'application/json; charset=utf-8', '2', 'W/"2-d4cbb29"',
15+
'OK', 'Express', 'X-HTTP-Method-Override', 'sessionid=; Path=/', 'Express',
16+
'sessionid=; Path=/,sessionid=6b059402-d62f-4e6f-b3dd-ce5b9e487c39; Path=/',
17+
'text/html; charset=utf-8', 'text/html; charset=utf-8', '9', 'OK',
18+
'sessionid=; Path=/', 'text/html; charset=utf-8',
19+
'text/html; charset=utf-8', '10', 'W/"a-eda64de5"', 'OK', 'Express',
20+
'Express', 'X-HTTP-Method-Override', 'sessionid=; Path=/',
21+
'application/json'
22+
],
23+
24+
// Put it here so the benchmark result lines will not be super long.
25+
LONG_AND_INVALID: ['Here is a value that is really a folded header ' +
26+
'value\r\n this should be supported, but it is not currently']
27+
};
28+
29+
const inputs = [
30+
// Valid
31+
'',
32+
'1',
33+
'\t\t\t\t\t\t\t\t\t\tFoo bar baz',
34+
'keep-alive',
35+
'close',
36+
'gzip',
37+
'20091',
38+
'private',
39+
'text/html; charset=utf-8',
40+
'text/plain',
41+
'Sat, 07 May 2016 16:54:48 GMT',
42+
'SAMEORIGIN',
43+
'en-US',
44+
45+
// Invalid
46+
'中文呢', // unicode
47+
'foo\nbar',
48+
'\x7F'
49+
];
950

1051
const bench = common.createBenchmark(main, {
11-
key: [
12-
// Valid
13-
'',
14-
'1',
15-
'\t\t\t\t\t\t\t\t\t\tFoo bar baz',
16-
'keep-alive',
17-
'close',
18-
'gzip',
19-
'20091',
20-
'private',
21-
'text/html; charset=utf-8',
22-
'text/plain',
23-
'Sat, 07 May 2016 16:54:48 GMT',
24-
'SAMEORIGIN',
25-
'en-US',
26-
27-
// Invalid
28-
'LONG_AND_INVALID',
29-
'中文呢', // unicode
30-
'foo\nbar',
31-
'\x7F'
32-
],
52+
input: inputs.concat(Object.keys(groupedInputs)),
3353
n: [1e6],
3454
});
3555

36-
function main({ n, key }) {
37-
if (key === 'LONG_AND_INVALID') {
38-
key = LONG_AND_INVALID;
56+
function main({ n, input }) {
57+
let inputs = [input];
58+
if (groupedInputs.hasOwnProperty(input)) {
59+
inputs = groupedInputs[input];
3960
}
61+
62+
const len = inputs.length;
4063
bench.start();
4164
for (var i = 0; i < n; i++) {
42-
_checkInvalidHeaderChar(key);
65+
_checkInvalidHeaderChar(inputs[i % len]);
4366
}
4467
bench.end(n);
4568
}

lib/_http_common.js

+2-44
Original file line numberDiff line numberDiff line change
@@ -242,57 +242,15 @@ function checkIsHttpToken(val) {
242242
return tokenRegExp.test(val);
243243
}
244244

245+
const headerCharRegex = /[^\t\x20-\x7e\x80-\xff]/;
245246
/**
246247
* True if val contains an invalid field-vchar
247248
* field-value = *( field-content / obs-fold )
248249
* field-content = field-vchar [ 1*( SP / HTAB ) field-vchar ]
249250
* field-vchar = VCHAR / obs-text
250-
*
251-
* checkInvalidHeaderChar() is currently designed to be inlinable by v8,
252-
* so take care when making changes to the implementation so that the source
253-
* code size does not exceed v8's default max_inlined_source_size setting.
254251
**/
255-
var validHdrChars = [
256-
0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, // 0 - 15
257-
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 16 - 31
258-
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 32 - 47
259-
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 48 - 63
260-
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 64 - 79
261-
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 80 - 95
262-
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 96 - 111
263-
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, // 112 - 127
264-
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 128 ...
265-
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
266-
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
267-
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
268-
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
269-
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
270-
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
271-
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1 // ... 255
272-
];
273252
function checkInvalidHeaderChar(val) {
274-
val += '';
275-
if (val.length < 1)
276-
return false;
277-
if (!validHdrChars[val.charCodeAt(0)])
278-
return true;
279-
if (val.length < 2)
280-
return false;
281-
if (!validHdrChars[val.charCodeAt(1)])
282-
return true;
283-
if (val.length < 3)
284-
return false;
285-
if (!validHdrChars[val.charCodeAt(2)])
286-
return true;
287-
if (val.length < 4)
288-
return false;
289-
if (!validHdrChars[val.charCodeAt(3)])
290-
return true;
291-
for (var i = 4; i < val.length; ++i) {
292-
if (!validHdrChars[val.charCodeAt(i)])
293-
return true;
294-
}
295-
return false;
253+
return headerCharRegex.test(val);
296254
}
297255

298256
module.exports = {

0 commit comments

Comments
 (0)