http: make parser choice a runtime flag

Add a `--http-parser=llhttp` vs `--http-parser=traditional`
command line switch, to make testing and comparing the new
llhttp-based implementation easier.

PR-URL: #24739
Refs: #24730
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>

Author: addaleax

configure.py

@@ -187,7 +187,7 @@
 parser.add_option('--experimental-http-parser',
     action='store_true',
     dest='experimental_http_parser',
-    help='use llhttp instead of http_parser')
+    help='use llhttp instead of http_parser by default')
 
 shared_optgroup.add_option('--shared-http-parser',
     action='store_true',

doc/api/cli.md

@@ -119,6 +119,22 @@ added: v6.0.0
 Force FIPS-compliant crypto on startup. (Cannot be disabled from script code.)
 (Same requirements as `--enable-fips`.)
 
+### `--http-parser=library`
+<!-- YAML
+added: REPLACEME
+-->
+
+Chooses an HTTP parser library. Available values are:
+
+- `llhttp` for https://llhttp.org/
+- `legacy` for https://github.com/nodejs/http-parser
+
+The default is `legacy`, unless otherwise specified when building Node.js.
+
+This flag exists to aid in experimentation with the internal implementation of
+the Node.js http parser.
+This flag is likely to become a no-op and removed at some point in the future.
+
 ### `--icu-data-dir=file`
 <!-- YAML
 added: v0.11.15

doc/node.1

@@ -97,6 +97,12 @@ Enable experimental ES module support in VM module.
 .It Fl -experimental-worker
 Enable experimental worker threads using worker_threads module.
 .
+.It Fl -http-parser Ns = Ns Ar library
+Chooses an HTTP parser library. Available values are
+.Sy llhttp
+or
+.Sy legacy .
+.
 .It Fl -force-fips
 Force FIPS-compliant crypto on startup
 (Cannot be disabled from script code).

lib/_http_client.js

@@ -24,14 +24,14 @@
 const util = require('util');
 const net = require('net');
 const url = require('url');
-const { HTTPParser } = internalBinding('http_parser');
 const assert = require('assert').ok;
 const {
   _checkIsHttpToken: checkIsHttpToken,
   debug,
   freeParser,
   httpSocketSetup,
-  parsers
+  parsers,
+  HTTPParser,
 } = require('_http_common');
 const { OutgoingMessage } = require('_http_outgoing');
 const Agent = require('_http_agent');

lib/_http_common.js

@@ -21,7 +21,11 @@
 
 'use strict';
 
-const { methods, HTTPParser } = internalBinding('http_parser');
+const { getOptionValue } = require('internal/options');
+
+const { methods, HTTPParser } =
+  getOptionValue('--http-parser') === 'legacy' ?
+    internalBinding('http_parser') : internalBinding('http_parser_llhttp');
 
 const { FreeList } = require('internal/freelist');
 const { ondrain } = require('internal/http');
@@ -243,5 +247,6 @@ module.exports = {
   httpSocketSetup,
   methods,
   parsers,
-  kIncomingMessage
+  kIncomingMessage,
+  HTTPParser
 };

lib/_http_server.js

@@ -23,7 +23,6 @@
 
 const util = require('util');
 const net = require('net');
-const { HTTPParser } = internalBinding('http_parser');
 const assert = require('assert').ok;
 const {
   parsers,
@@ -34,6 +33,7 @@ const {
   chunkExpression,
   httpSocketSetup,
   kIncomingMessage,
+  HTTPParser,
   _checkInvalidHeaderChar: checkInvalidHeaderChar
 } = require('_http_common');
 const { OutgoingMessage } = require('_http_outgoing');

lib/internal/child_process.js

@@ -127,7 +127,7 @@ const handleConversion = {
         if (freeParser === undefined)
           freeParser = require('_http_common').freeParser;
         if (HTTPParser === undefined)
-          HTTPParser = internalBinding('http_parser').HTTPParser;
+          HTTPParser = require('_http_common').HTTPParser;
 
         // In case of an HTTP connection socket, release the associated
         // resources

node.gyp

@@ -352,7 +352,8 @@
         'src/node_encoding.cc',
         'src/node_errors.cc',
         'src/node_file.cc',
-        'src/node_http_parser.cc',
+        'src/node_http_parser_llhttp.cc',
+        'src/node_http_parser_traditional.cc',
         'src/node_http2.cc',
         'src/node_i18n.cc',
         'src/node_messaging.cc',
@@ -423,6 +424,7 @@
         'src/node_contextify.h',
         'src/node_errors.h',
         'src/node_file.h',
+        'src/node_http_parser_impl.h',
         'src/node_http2.h',
         'src/node_http2_state.h',
         'src/node_i18n.h',

node.gypi

@@ -164,12 +164,14 @@
     }],
 
     [ 'node_experimental_http_parser=="true"', {
-      'defines': [ 'NODE_EXPERIMENTAL_HTTP' ],
-      'dependencies': [ 'deps/llhttp/llhttp.gyp:llhttp' ],
-    }, {
-      'conditions': [ [ 'node_shared_http_parser=="false"', {
-        'dependencies': [ 'deps/http_parser/http_parser.gyp:http_parser' ],
-      } ] ],
+      'defines': [ 'NODE_EXPERIMENTAL_HTTP_DEFAULT' ],
+    } ],
+
+    [ 'node_shared_http_parser=="false"', {
+      'dependencies': [
+        'deps/http_parser/http_parser.gyp:http_parser',
+        'deps/llhttp/llhttp.gyp:llhttp'
+      ],
     } ],
 
     [ 'node_shared_cares=="false"', {

src/inspector_socket.cc

@@ -1,6 +1,10 @@
 #include "inspector_socket.h"
 
+#ifdef NODE_EXPERIMENTAL_HTTP_DEFAULT
+#define NODE_EXPERIMENTAL_HTTP
+#endif
 #include "http_parser_adaptor.h"
+
 #include "util-inl.h"
 
 #define NODE_WANT_INTERNALS 1
@@ -433,13 +437,13 @@ class HttpHandler : public ProtocolHandler {
   explicit HttpHandler(InspectorSocket* inspector, TcpHolder::Pointer tcp)
                        : ProtocolHandler(inspector, std::move(tcp)),
                          parsing_value_(false) {
-#ifdef NODE_EXPERIMENTAL_HTTP
+#ifdef NODE_EXPERIMENTAL_HTTP_DEFAULT
     llhttp_init(&parser_, HTTP_REQUEST, &parser_settings);
     llhttp_settings_init(&parser_settings);
-#else  /* !NODE_EXPERIMENTAL_HTTP */
+#else  /* !NODE_EXPERIMENTAL_HTTP_DEFAULT */
     http_parser_init(&parser_, HTTP_REQUEST);
     http_parser_settings_init(&parser_settings);
-#endif  /* NODE_EXPERIMENTAL_HTTP */
+#endif  /* NODE_EXPERIMENTAL_HTTP_DEFAULT */
     parser_settings.on_header_field = OnHeaderField;
     parser_settings.on_header_value = OnHeaderValue;
     parser_settings.on_message_complete = OnMessageComplete;
@@ -484,17 +488,17 @@ class HttpHandler : public ProtocolHandler {
 
   void OnData(std::vector<char>* data) override {
     parser_errno_t err;
-#ifdef NODE_EXPERIMENTAL_HTTP
+#ifdef NODE_EXPERIMENTAL_HTTP_DEFAULT
     err = llhttp_execute(&parser_, data->data(), data->size());
 
     if (err == HPE_PAUSED_UPGRADE) {
       err = HPE_OK;
       llhttp_resume_after_upgrade(&parser_);
     }
-#else  /* !NODE_EXPERIMENTAL_HTTP */
+#else  /* !NODE_EXPERIMENTAL_HTTP_DEFAULT */
     http_parser_execute(&parser_, &parser_settings, data->data(), data->size());
     err = HTTP_PARSER_ERRNO(&parser_);
-#endif  /* NODE_EXPERIMENTAL_HTTP */
+#endif  /* NODE_EXPERIMENTAL_HTTP_DEFAULT */
     data->clear();
     if (err != HPE_OK) {
       CancelHandshake();