Skip to content

Commit 1744205

Browse files
jasnelljasnell
committedAug 18, 2018
http: move process.binding('http_parser') to internalBinding
Refs: #22160 PR-URL: #22329 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Jon Moss <me@jonathanmoss.me> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent 4fa5448 commit 1744205

14 files changed

+84
-58
lines changed
 

‎benchmark/http/bench-parser.js

+36-32
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,54 @@
11
'use strict';
22

33
const common = require('../common');
4-
const HTTPParser = process.binding('http_parser').HTTPParser;
5-
const REQUEST = HTTPParser.REQUEST;
6-
const kOnHeaders = HTTPParser.kOnHeaders | 0;
7-
const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;
8-
const kOnBody = HTTPParser.kOnBody | 0;
9-
const kOnMessageComplete = HTTPParser.kOnMessageComplete | 0;
10-
const CRLF = '\r\n';
114

125
const bench = common.createBenchmark(main, {
136
len: [4, 8, 16, 32],
14-
n: [1e5],
7+
n: [1e5]
8+
}, {
9+
flags: ['--expose-internals', '--no-warnings']
1510
});
1611

1712
function main({ len, n }) {
18-
var header = `GET /hello HTTP/1.1${CRLF}Content-Type: text/plain${CRLF}`;
19-
20-
for (var i = 0; i < len; i++) {
21-
header += `X-Filler${i}: ${Math.random().toString(36).substr(2)}${CRLF}`;
13+
const { internalBinding } = require('internal/test/binding');
14+
const { HTTPParser } = internalBinding('http_parser');
15+
const REQUEST = HTTPParser.REQUEST;
16+
const kOnHeaders = HTTPParser.kOnHeaders | 0;
17+
const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;
18+
const kOnBody = HTTPParser.kOnBody | 0;
19+
const kOnMessageComplete = HTTPParser.kOnMessageComplete | 0;
20+
const CRLF = '\r\n';
21+
22+
function processHeader(header, n) {
23+
const parser = newParser(REQUEST);
24+
25+
bench.start();
26+
for (var i = 0; i < n; i++) {
27+
parser.execute(header, 0, header.length);
28+
parser.reinitialize(REQUEST);
29+
}
30+
bench.end(n);
2231
}
23-
header += CRLF;
2432

25-
processHeader(Buffer.from(header), n);
26-
}
33+
function newParser(type) {
34+
const parser = new HTTPParser(type);
2735

28-
function processHeader(header, n) {
29-
const parser = newParser(REQUEST);
36+
parser.headers = [];
3037

31-
bench.start();
32-
for (var i = 0; i < n; i++) {
33-
parser.execute(header, 0, header.length);
34-
parser.reinitialize(REQUEST);
35-
}
36-
bench.end(n);
37-
}
38+
parser[kOnHeaders] = function() { };
39+
parser[kOnHeadersComplete] = function() { };
40+
parser[kOnBody] = function() { };
41+
parser[kOnMessageComplete] = function() { };
3842

39-
function newParser(type) {
40-
const parser = new HTTPParser(type);
43+
return parser;
44+
}
4145

42-
parser.headers = [];
46+
let header = `GET /hello HTTP/1.1${CRLF}Content-Type: text/plain${CRLF}`;
4347

44-
parser[kOnHeaders] = function() { };
45-
parser[kOnHeadersComplete] = function() { };
46-
parser[kOnBody] = function() { };
47-
parser[kOnMessageComplete] = function() { };
48+
for (var i = 0; i < len; i++) {
49+
header += `X-Filler${i}: ${Math.random().toString(36).substr(2)}${CRLF}`;
50+
}
51+
header += CRLF;
4852

49-
return parser;
53+
processHeader(Buffer.from(header), n);
5054
}

‎lib/_http_client.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@
2424
const util = require('util');
2525
const net = require('net');
2626
const url = require('url');
27-
const { HTTPParser } = process.binding('http_parser');
27+
const { internalBinding } = require('internal/bootstrap/loaders');
28+
const { HTTPParser } = internalBinding('http_parser');
2829
const assert = require('assert').ok;
2930
const {
3031
_checkIsHttpToken: checkIsHttpToken,

‎lib/_http_common.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@
2121

2222
'use strict';
2323

24-
const { methods, HTTPParser } = process.binding('http_parser');
24+
const { internalBinding } = require('internal/bootstrap/loaders');
25+
const { methods, HTTPParser } = internalBinding('http_parser');
2526

2627
const FreeList = require('internal/freelist');
2728
const { ondrain } = require('internal/http');

‎lib/_http_server.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@
2323

2424
const util = require('util');
2525
const net = require('net');
26-
const { HTTPParser } = process.binding('http_parser');
26+
const { internalBinding } = require('internal/bootstrap/loaders');
27+
const { HTTPParser } = internalBinding('http_parser');
2728
const assert = require('assert').ok;
2829
const {
2930
parsers,

‎lib/internal/bootstrap/node.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@
344344
// that are whitelisted for access via process.binding()... this is used
345345
// to provide a transition path for modules that are being moved over to
346346
// internalBinding.
347-
const internalBindingWhitelist = new SafeSet(['uv']);
347+
const internalBindingWhitelist = new SafeSet(['uv', 'http_parser']);
348348
process.binding = function binding(name) {
349349
return internalBindingWhitelist.has(name) ?
350350
internalBinding(name) :

‎lib/internal/child_process.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ const dgram = require('dgram');
2121
const util = require('util');
2222
const assert = require('assert');
2323

24+
const { internalBinding } = require('internal/bootstrap/loaders');
25+
2426
const { Process } = process.binding('process_wrap');
2527
const { WriteWrap } = process.binding('stream_wrap');
2628
const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap');
@@ -32,12 +34,10 @@ const { owner_symbol } = require('internal/async_hooks').symbols;
3234
const { convertToValidSignal } = require('internal/util');
3335
const { isUint8Array } = require('internal/util/types');
3436
const spawn_sync = process.binding('spawn_sync');
35-
const { HTTPParser } = process.binding('http_parser');
37+
const { HTTPParser } = internalBinding('http_parser');
3638
const { freeParser } = require('_http_common');
3739
const { kStateSymbol } = require('internal/dgram');
3840

39-
const { internalBinding } = require('internal/bootstrap/loaders');
40-
4141
const {
4242
UV_EACCES,
4343
UV_EAGAIN,

‎src/node_http_parser.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -775,4 +775,4 @@ void Initialize(Local<Object> target,
775775
} // anonymous namespace
776776
} // namespace node
777777

778-
NODE_BUILTIN_MODULE_CONTEXT_AWARE(http_parser, node::Initialize)
778+
NODE_MODULE_CONTEXT_AWARE_INTERNAL(http_parser, node::Initialize)

‎test/async-hooks/test-httpparser.request.js

+11-6
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// Flags: --expose-internals
12
'use strict';
23

34
const common = require('../common');
@@ -6,17 +7,21 @@ const tick = require('./tick');
67
const initHooks = require('./init-hooks');
78
const { checkInvocations } = require('./hook-checks');
89

9-
const binding = process.binding('http_parser');
10-
const HTTPParser = binding.HTTPParser;
10+
const hooks = initHooks();
11+
hooks.enable();
12+
13+
// The hooks.enable() must come before require('internal/test/binding')
14+
// because internal/test/binding schedules a process warning on nextTick.
15+
// If this order is not preserved, the hooks check will fail because it
16+
// will not be notified about the nextTick creation but will see the
17+
// callback event.
18+
const { internalBinding } = require('internal/test/binding');
19+
const { HTTPParser } = internalBinding('http_parser');
1120

1221
const REQUEST = HTTPParser.REQUEST;
1322

1423
const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;
1524

16-
const hooks = initHooks();
17-
18-
hooks.enable();
19-
2025
const request = Buffer.from(
2126
'GET /hello HTTP/1.1\r\n\r\n'
2227
);

‎test/async-hooks/test-httpparser.response.js

+12-6
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// Flags: --expose-internals
12
'use strict';
23

34
const common = require('../common');
@@ -6,17 +7,22 @@ const tick = require('./tick');
67
const initHooks = require('./init-hooks');
78
const { checkInvocations } = require('./hook-checks');
89

9-
const binding = process.binding('http_parser');
10-
const HTTPParser = binding.HTTPParser;
10+
const hooks = initHooks();
11+
12+
hooks.enable();
13+
14+
// The hooks.enable() must come before require('internal/test/binding')
15+
// because internal/test/binding schedules a process warning on nextTick.
16+
// If this order is not preserved, the hooks check will fail because it
17+
// will not be notified about the nextTick creation but will see the
18+
// callback event.
19+
const { internalBinding } = require('internal/test/binding');
20+
const { HTTPParser } = internalBinding('http_parser');
1121

1222
const RESPONSE = HTTPParser.RESPONSE;
1323
const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;
1424
const kOnBody = HTTPParser.kOnBody | 0;
1525

16-
const hooks = initHooks();
17-
18-
hooks.enable();
19-
2026
const request = Buffer.from(
2127
'HTTP/1.1 200 OK\r\n' +
2228
'Content-Type: text/plain\r\n' +

‎test/parallel/test-http-parser-bad-ref.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,12 @@
22
// Run this program with valgrind or efence with --expose_gc to expose the
33
// problem.
44

5-
// Flags: --expose_gc
5+
// Flags: --expose_gc --expose-internals
66

77
require('../common');
88
const assert = require('assert');
9-
const HTTPParser = process.binding('http_parser').HTTPParser;
9+
const { internalBinding } = require('internal/test/binding');
10+
const { HTTPParser } = internalBinding('http_parser');
1011

1112
const kOnHeaders = HTTPParser.kOnHeaders | 0;
1213
const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;

‎test/parallel/test-http-parser.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,14 @@
1919
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
2020
// USE OR OTHER DEALINGS IN THE SOFTWARE.
2121

22+
// Flags: --expose-internals
23+
2224
'use strict';
2325
const { mustCall, mustNotCall } = require('../common');
2426
const assert = require('assert');
2527

26-
const { methods, HTTPParser } = process.binding('http_parser');
28+
const { internalBinding } = require('internal/test/binding');
29+
const { methods, HTTPParser } = internalBinding('http_parser');
2730
const { REQUEST, RESPONSE } = HTTPParser;
2831

2932
const kOnHeaders = HTTPParser.kOnHeaders | 0;

‎test/parallel/test-process-binding-internalbinding-whitelist.js

+1
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,4 @@ const assert = require('assert');
77
// Assert that whitelisted internalBinding modules are accessible via
88
// process.binding().
99
assert(process.binding('uv'));
10+
assert(process.binding('http_parser'));

‎test/sequential/test-async-wrap-getasyncid.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
'use strict';
2-
// Flags: --expose-gc
2+
// Flags: --expose-gc --expose-internals
33

44
const common = require('../common');
5+
const { internalBinding } = require('internal/test/binding');
56
const assert = require('assert');
67
const fs = require('fs');
78
const fsPromises = fs.promises;
@@ -146,7 +147,7 @@ if (common.hasCrypto) { // eslint-disable-line node-core/crypto-check
146147

147148

148149
{
149-
const HTTPParser = process.binding('http_parser').HTTPParser;
150+
const { HTTPParser } = internalBinding('http_parser');
150151
testInitialized(new HTTPParser(), 'HTTPParser');
151152
}
152153

‎test/sequential/test-http-regr-gh-2928.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
// This test is designed to fail with a segmentation fault in Node.js 4.1.0 and
22
// execute without issues in Node.js 4.1.1 and up.
33

4+
// Flags: --expose-internals
45
'use strict';
56
const common = require('../common');
67
const assert = require('assert');
78
const httpCommon = require('_http_common');
8-
const HTTPParser = process.binding('http_parser').HTTPParser;
9+
const { internalBinding } = require('internal/test/binding');
10+
const { HTTPParser } = internalBinding('http_parser');
911
const net = require('net');
1012

1113
const COUNT = httpCommon.parsers.max + 1;

0 commit comments

Comments
 (0)