From 1744205ff565b490f8db72000028b074cce23d5d Mon Sep 17 00:00:00 2001
From: James M Snell <jasnell@gmail.com>
Date: Tue, 14 Aug 2018 17:01:06 -0700
Subject: [PATCH] http: move process.binding('http_parser') to internalBinding

Refs: https://github.com/nodejs/node/issues/22160

PR-URL: https://github.com/nodejs/node/pull/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>
---
 benchmark/http/bench-parser.js                | 68 ++++++++++---------
 lib/_http_client.js                           |  3 +-
 lib/_http_common.js                           |  3 +-
 lib/_http_server.js                           |  3 +-
 lib/internal/bootstrap/node.js                |  2 +-
 lib/internal/child_process.js                 |  6 +-
 src/node_http_parser.cc                       |  2 +-
 test/async-hooks/test-httpparser.request.js   | 17 +++--
 test/async-hooks/test-httpparser.response.js  | 18 +++--
 test/parallel/test-http-parser-bad-ref.js     |  5 +-
 test/parallel/test-http-parser.js             |  5 +-
 ...ocess-binding-internalbinding-whitelist.js |  1 +
 test/sequential/test-async-wrap-getasyncid.js |  5 +-
 test/sequential/test-http-regr-gh-2928.js     |  4 +-
 14 files changed, 84 insertions(+), 58 deletions(-)

diff --git a/benchmark/http/bench-parser.js b/benchmark/http/bench-parser.js
index d629fe67e59e76..087616f44e71d1 100644
--- 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
index 7d28cda681d810..372ac0f953ff51 100644
--- 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
index faa6fe629ae416..09f9aebe9b2ff2 100644
--- 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
index 3d5a1f8f6242f7..6e1b2c90e5351d 100644
--- 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
index 26ab0c3198cd56..e6c4da25b00d9b 100644
--- 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
index c34bc996de9664..d212bbeaa7bca2 100644
--- 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
index 318ad0b8e6bf73..bab15a8c4bdf6c 100644
--- 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
index c6e18be5a63d38..71fccc0a858921 100644
--- 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
index d56e3e1fecae2d..eea40b14d16b68 100644
--- 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
index 6e9fb2f9ac6033..60151c63488143 100644
--- 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
index bc9c6920bc36a2..0bdaa22b8ade5a 100644
--- 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
index ece967a0b76ab9..bfb265a2994952 100644
--- 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
index 1f77267922689d..9ee32b646cd955 100644
--- 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
index 55e3a93bc98eaa..0950b84bbe093d 100644
--- 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;