http: move process.binding('http_parser') to internalBinding

Refs: nodejs#22160 PR-URL: nodejs#22329 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
ihiteshsharma · Aug 18, 2018 · 1744205 · 1744205
1 parent 4fa5448
commit 1744205
Show file tree

Hide file tree

Showing 14 changed files with 84 additions and 58 deletions.
diff --git a/benchmark/http/bench-parser.js b/benchmark/http/bench-parser.js
@@ -1,50 +1,54 @@
 'use strict';
 
 const common = require('../common');
-const HTTPParser = process.binding('http_parser').HTTPParser;
-const REQUEST = HTTPParser.REQUEST;
-const kOnHeaders = HTTPParser.kOnHeaders | 0;
-const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;
-const kOnBody = HTTPParser.kOnBody | 0;
-const kOnMessageComplete = HTTPParser.kOnMessageComplete | 0;
-const CRLF = '\r\n';
 
 const bench = common.createBenchmark(main, {
   len: [4, 8, 16, 32],
-  n: [1e5],
+  n: [1e5]
+}, {
+  flags: ['--expose-internals', '--no-warnings']
 });
 
 function main({ len, n }) {
-  var header = `GET /hello HTTP/1.1${CRLF}Content-Type: text/plain${CRLF}`;
-
-  for (var i = 0; i < len; i++) {
-    header += `X-Filler${i}: ${Math.random().toString(36).substr(2)}${CRLF}`;
+  const { internalBinding } = require('internal/test/binding');
+  const { HTTPParser } = internalBinding('http_parser');
+  const REQUEST = HTTPParser.REQUEST;
+  const kOnHeaders = HTTPParser.kOnHeaders | 0;
+  const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;
+  const kOnBody = HTTPParser.kOnBody | 0;
+  const kOnMessageComplete = HTTPParser.kOnMessageComplete | 0;
+  const CRLF = '\r\n';
+
+  function processHeader(header, n) {
+    const parser = newParser(REQUEST);
+
+    bench.start();
+    for (var i = 0; i < n; i++) {
+      parser.execute(header, 0, header.length);
+      parser.reinitialize(REQUEST);
+    }
+    bench.end(n);
   }
-  header += CRLF;
 
-  processHeader(Buffer.from(header), n);
-}
+  function newParser(type) {
+    const parser = new HTTPParser(type);
 
-function processHeader(header, n) {
-  const parser = newParser(REQUEST);
+    parser.headers = [];
 
-  bench.start();
-  for (var i = 0; i < n; i++) {
-    parser.execute(header, 0, header.length);
-    parser.reinitialize(REQUEST);
-  }
-  bench.end(n);
-}
+    parser[kOnHeaders] = function() { };
+    parser[kOnHeadersComplete] = function() { };
+    parser[kOnBody] = function() { };
+    parser[kOnMessageComplete] = function() { };
 
-function newParser(type) {
-  const parser = new HTTPParser(type);
+    return parser;
+  }
 
-  parser.headers = [];
+  let header = `GET /hello HTTP/1.1${CRLF}Content-Type: text/plain${CRLF}`;
 
-  parser[kOnHeaders] = function() { };
-  parser[kOnHeadersComplete] = function() { };
-  parser[kOnBody] = function() { };
-  parser[kOnMessageComplete] = function() { };
+  for (var i = 0; i < len; i++) {
+    header += `X-Filler${i}: ${Math.random().toString(36).substr(2)}${CRLF}`;
+  }
+  header += CRLF;
 
-  return parser;
+  processHeader(Buffer.from(header), n);
 }
diff --git a/lib/_http_client.js b/lib/_http_client.js
@@ -24,7 +24,8 @@
 const util = require('util');
 const net = require('net');
 const url = require('url');
-const { HTTPParser } = process.binding('http_parser');
+const { internalBinding } = require('internal/bootstrap/loaders');
+const { HTTPParser } = internalBinding('http_parser');
 const assert = require('assert').ok;
 const {
   _checkIsHttpToken: checkIsHttpToken,

diff --git a/lib/_http_common.js b/lib/_http_common.js
@@ -21,7 +21,8 @@
 
 'use strict';
 
-const { methods, HTTPParser } = process.binding('http_parser');
+const { internalBinding } = require('internal/bootstrap/loaders');
+const { methods, HTTPParser } = internalBinding('http_parser');
 
 const FreeList = require('internal/freelist');
 const { ondrain } = require('internal/http');

diff --git a/lib/_http_server.js b/lib/_http_server.js
@@ -23,7 +23,8 @@
 
 const util = require('util');
 const net = require('net');
-const { HTTPParser } = process.binding('http_parser');
+const { internalBinding } = require('internal/bootstrap/loaders');
+const { HTTPParser } = internalBinding('http_parser');
 const assert = require('assert').ok;
 const {
   parsers,

diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
@@ -344,7 +344,7 @@
     // that are whitelisted for access via process.binding()... this is used
     // to provide a transition path for modules that are being moved over to
     // internalBinding.
-    const internalBindingWhitelist = new SafeSet(['uv']);
+    const internalBindingWhitelist = new SafeSet(['uv', 'http_parser']);
     process.binding = function binding(name) {
       return internalBindingWhitelist.has(name) ?
         internalBinding(name) :

diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js
@@ -21,6 +21,8 @@ const dgram = require('dgram');
 const util = require('util');
 const assert = require('assert');
 
+const { internalBinding } = require('internal/bootstrap/loaders');
+
 const { Process } = process.binding('process_wrap');
 const { WriteWrap } = process.binding('stream_wrap');
 const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap');
@@ -32,12 +34,10 @@ const { owner_symbol } = require('internal/async_hooks').symbols;
 const { convertToValidSignal } = require('internal/util');
 const { isUint8Array } = require('internal/util/types');
 const spawn_sync = process.binding('spawn_sync');
-const { HTTPParser } = process.binding('http_parser');
+const { HTTPParser } = internalBinding('http_parser');
 const { freeParser } = require('_http_common');
 const { kStateSymbol } = require('internal/dgram');
 
-const { internalBinding } = require('internal/bootstrap/loaders');
-
 const {
   UV_EACCES,
   UV_EAGAIN,

diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc
@@ -775,4 +775,4 @@ void Initialize(Local<Object> target,
 }  // anonymous namespace
 }  // namespace node
 
-NODE_BUILTIN_MODULE_CONTEXT_AWARE(http_parser, node::Initialize)
+NODE_MODULE_CONTEXT_AWARE_INTERNAL(http_parser, node::Initialize)
diff --git a/test/async-hooks/test-httpparser.request.js b/test/async-hooks/test-httpparser.request.js
@@ -1,3 +1,4 @@
+// Flags: --expose-internals
 'use strict';
 
 const common = require('../common');
@@ -6,17 +7,21 @@ const tick = require('./tick');
 const initHooks = require('./init-hooks');
 const { checkInvocations } = require('./hook-checks');
 
-const binding = process.binding('http_parser');
-const HTTPParser = binding.HTTPParser;
+const hooks = initHooks();
+hooks.enable();
+
+// The hooks.enable() must come before require('internal/test/binding')
+// because internal/test/binding schedules a process warning on nextTick.
+// If this order is not preserved, the hooks check will fail because it
+// will not be notified about the nextTick creation but will see the
+// callback event.
+const { internalBinding } = require('internal/test/binding');
+const { HTTPParser } = internalBinding('http_parser');
 
 const REQUEST = HTTPParser.REQUEST;
 
 const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;
 
-const hooks = initHooks();
-
-hooks.enable();
-
 const request = Buffer.from(
   'GET /hello HTTP/1.1\r\n\r\n'
 );

diff --git a/test/async-hooks/test-httpparser.response.js b/test/async-hooks/test-httpparser.response.js
@@ -1,3 +1,4 @@
+// Flags: --expose-internals
 'use strict';
 
 const common = require('../common');
@@ -6,17 +7,22 @@ const tick = require('./tick');
 const initHooks = require('./init-hooks');
 const { checkInvocations } = require('./hook-checks');
 
-const binding = process.binding('http_parser');
-const HTTPParser = binding.HTTPParser;
+const hooks = initHooks();
+
+hooks.enable();
+
+// The hooks.enable() must come before require('internal/test/binding')
+// because internal/test/binding schedules a process warning on nextTick.
+// If this order is not preserved, the hooks check will fail because it
+// will not be notified about the nextTick creation but will see the
+// callback event.
+const { internalBinding } = require('internal/test/binding');
+const { HTTPParser } = internalBinding('http_parser');
 
 const RESPONSE = HTTPParser.RESPONSE;
 const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;
 const kOnBody = HTTPParser.kOnBody | 0;
 
-const hooks = initHooks();
-
-hooks.enable();
-
 const request = Buffer.from(
   'HTTP/1.1 200 OK\r\n' +
   'Content-Type: text/plain\r\n' +

diff --git a/test/parallel/test-http-parser-bad-ref.js b/test/parallel/test-http-parser-bad-ref.js
@@ -2,11 +2,12 @@
 // Run this program with valgrind or efence with --expose_gc to expose the
 // problem.
 
-// Flags: --expose_gc
+// Flags: --expose_gc --expose-internals
 
 require('../common');
 const assert = require('assert');
-const HTTPParser = process.binding('http_parser').HTTPParser;
+const { internalBinding } = require('internal/test/binding');
+const { HTTPParser } = internalBinding('http_parser');
 
 const kOnHeaders = HTTPParser.kOnHeaders | 0;
 const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;

diff --git a/test/parallel/test-http-parser.js b/test/parallel/test-http-parser.js
@@ -19,11 +19,14 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
+// Flags: --expose-internals
+
 'use strict';
 const { mustCall, mustNotCall } = require('../common');
 const assert = require('assert');
 
-const { methods, HTTPParser } = process.binding('http_parser');
+const { internalBinding } = require('internal/test/binding');
+const { methods, HTTPParser } = internalBinding('http_parser');
 const { REQUEST, RESPONSE } = HTTPParser;
 
 const kOnHeaders = HTTPParser.kOnHeaders | 0;

diff --git a/test/parallel/test-process-binding-internalbinding-whitelist.js b/test/parallel/test-process-binding-internalbinding-whitelist.js
@@ -7,3 +7,4 @@ const assert = require('assert');
 // Assert that whitelisted internalBinding modules are accessible via
 // process.binding().
 assert(process.binding('uv'));
+assert(process.binding('http_parser'));
diff --git a/test/sequential/test-async-wrap-getasyncid.js b/test/sequential/test-async-wrap-getasyncid.js
@@ -1,7 +1,8 @@
 'use strict';
-// Flags: --expose-gc
+// Flags: --expose-gc --expose-internals
 
 const common = require('../common');
+const { internalBinding } = require('internal/test/binding');
 const assert = require('assert');
 const fs = require('fs');
 const fsPromises = fs.promises;
@@ -146,7 +147,7 @@ if (common.hasCrypto) { // eslint-disable-line node-core/crypto-check
 
 
 {
-  const HTTPParser = process.binding('http_parser').HTTPParser;
+  const { HTTPParser } = internalBinding('http_parser');
   testInitialized(new HTTPParser(), 'HTTPParser');
 }
 

diff --git a/test/sequential/test-http-regr-gh-2928.js b/test/sequential/test-http-regr-gh-2928.js
@@ -1,11 +1,13 @@
 // This test is designed to fail with a segmentation fault in Node.js 4.1.0 and
 // execute without issues in Node.js 4.1.1 and up.
 
+// Flags: --expose-internals
 'use strict';
 const common = require('../common');
 const assert = require('assert');
 const httpCommon = require('_http_common');
-const HTTPParser = process.binding('http_parser').HTTPParser;
+const { internalBinding } = require('internal/test/binding');
+const { HTTPParser } = internalBinding('http_parser');
 const net = require('net');
 
 const COUNT = httpCommon.parsers.max + 1;