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: https://github.com/nodejs/node/pull/24739
Refs: https://github.com/nodejs/node/issues/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>

10 files changed, 86 insertions, 34 deletions

diff --git a/src/inspector_socket.cc b/src/inspector_socket.cc
index 8c2d0a5a22..dcc42f25b4 100644
--- a/src/inspector_socket.cc
+++ b/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();
diff --git a/src/node_binding.cc b/src/node_binding.cc
index a2e5134bfa..6d70d0a501 100644
--- a/src/node_binding.cc
+++ b/src/node_binding.cc
@@ -36,6 +36,7 @@
   V(heap_utils)                                                                \
   V(http2)                                                                     \
   V(http_parser)                                                               \
+  V(http_parser_llhttp)                                                        \
   V(inspector)                                                                 \
   V(js_stream)                                                                 \
   V(messaging)                                                                 \
diff --git a/src/node_http_parser.cc b/src/node_http_parser_impl.h
index 4d683b4db2..4233ccbfca 100644
--- a/src/node_http_parser.cc
+++ b/src/node_http_parser_impl.h
@@ -19,6 +19,12 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
+// This file is included from 2 files, node_http_parser_traditional.cc
+// and node_http_parser_llhttp.cc.
+
+#ifndef SRC_NODE_HTTP_PARSER_IMPL_H_
+#define SRC_NODE_HTTP_PARSER_IMPL_H_
+
 #include "node.h"
 #include "node_buffer.h"
 #include "node_internals.h"
@@ -47,7 +53,7 @@
 
 
 namespace node {
-namespace {
+namespace {  // NOLINT(build/namespaces)
 
 using v8::Array;
 using v8::Boolean;
@@ -910,10 +916,10 @@ const parser_settings_t Parser::settings = {
 };
 
 
-void Initialize(Local<Object> target,
-                Local<Value> unused,
-                Local<Context> context,
-                void* priv) {
+void InitializeHttpParser(Local<Object> target,
+                          Local<Value> unused,
+                          Local<Context> context,
+                          void* priv) {
   Environment* env = Environment::GetCurrent(context);
   Local<FunctionTemplate> t = env->NewFunctionTemplate(Parser::New);
   t->InstanceTemplate()->SetInternalFieldCount(1);
@@ -964,4 +970,4 @@ void Initialize(Local<Object> target,
 }  // anonymous namespace
 }  // namespace node
 
-NODE_MODULE_CONTEXT_AWARE_INTERNAL(http_parser, node::Initialize)
+#endif  // SRC_NODE_HTTP_PARSER_IMPL_H_
diff --git a/src/node_http_parser_llhttp.cc b/src/node_http_parser_llhttp.cc
new file mode 100644
index 0000000000..7b9999d13c
--- /dev/null
+++ b/src/node_http_parser_llhttp.cc
@@ -0,0 +1,17 @@
+#define NODE_EXPERIMENTAL_HTTP 1
+
+#include "node_http_parser_impl.h"
+
+namespace node {
+
+const char* llhttp_version =
+    NODE_STRINGIFY(LLHTTP_VERSION_MAJOR)
+    "."
+    NODE_STRINGIFY(LLHTTP_VERSION_MINOR)
+    "."
+    NODE_STRINGIFY(LLHTTP_VERSION_PATCH);
+
+}  // namespace node
+
+NODE_MODULE_CONTEXT_AWARE_INTERNAL(http_parser_llhttp,
+                                   node::InitializeHttpParser)
diff --git a/src/node_http_parser_traditional.cc b/src/node_http_parser_traditional.cc
new file mode 100644
index 0000000000..2bff20c165
--- /dev/null
+++ b/src/node_http_parser_traditional.cc
@@ -0,0 +1,18 @@
+#ifdef NODE_EXPERIMENTAL_HTTP
+#undef NODE_EXPERIMENTAL_HTTP
+#endif
+
+#include "node_http_parser_impl.h"
+
+namespace node {
+
+const char* http_parser_version =
+  NODE_STRINGIFY(HTTP_PARSER_VERSION_MAJOR)
+  "."
+  NODE_STRINGIFY(HTTP_PARSER_VERSION_MINOR)
+  "."
+  NODE_STRINGIFY(HTTP_PARSER_VERSION_PATCH);
+
+}  // namespace node
+
+NODE_MODULE_CONTEXT_AWARE_INTERNAL(http_parser, node::InitializeHttpParser)
diff --git a/src/node_internals.h b/src/node_internals.h
index 6cb40c9070..1d43d4b141 100644
--- a/src/node_internals.h
+++ b/src/node_internals.h
@@ -697,6 +697,9 @@ static inline const char* errno_string(int errorno) {
 
 extern double prog_start_time;
 
+extern const char* llhttp_version;
+extern const char* http_parser_version;
+
 void Abort(const v8::FunctionCallbackInfo<v8::Value>& args);
 void Chdir(const v8::FunctionCallbackInfo<v8::Value>& args);
 void CPUUsage(const v8::FunctionCallbackInfo<v8::Value>& args);
diff --git a/src/node_metadata.cc b/src/node_metadata.cc
index 1bf2fcfce9..fd37dbb97b 100644
--- a/src/node_metadata.cc
+++ b/src/node_metadata.cc
@@ -11,12 +11,6 @@
 #include "node_crypto.h"
 #endif
 
-#ifdef NODE_EXPERIMENTAL_HTTP
-#include "llhttp.h"
-#else /* !NODE_EXPERIMENTAL_HTTP */
-#include "http_parser.h"
-#endif /* NODE_EXPERIMENTAL_HTTP */
-
 namespace node {
 
 namespace per_process {
@@ -32,14 +26,8 @@ Metadata::Versions::Versions() {
   modules = NODE_STRINGIFY(NODE_MODULE_VERSION);
   nghttp2 = NGHTTP2_VERSION;
   napi = NODE_STRINGIFY(NAPI_VERSION);
-
-#ifdef NODE_EXPERIMENTAL_HTTP
-  llhttp = NODE_STRINGIFY(LLHTTP_VERSION_MAJOR) "." NODE_STRINGIFY(
-      LLHTTP_VERSION_MINOR) "." NODE_STRINGIFY(LLHTTP_VERSION_PATCH);
-#else  /* !NODE_EXPERIMENTAL_HTTP */
-  http_parser = NODE_STRINGIFY(HTTP_PARSER_VERSION_MAJOR) "." NODE_STRINGIFY(
-      HTTP_PARSER_VERSION_MINOR) "." NODE_STRINGIFY(HTTP_PARSER_VERSION_PATCH);
-#endif /* NODE_EXPERIMENTAL_HTTP */
+  llhttp = llhttp_version;
+  http_parser = http_parser_version;
 
 #if HAVE_OPENSSL
   openssl = crypto::GetOpenSSLVersion();
diff --git a/src/node_metadata.h b/src/node_metadata.h
index 0f32fcf21d..9f383be5f8 100644
--- a/src/node_metadata.h
+++ b/src/node_metadata.h
@@ -15,13 +15,9 @@ namespace node {
   V(ares)                                                                      \
   V(modules)                                                                   \
   V(nghttp2)                                                                   \
-  V(napi)
-
-#ifdef NODE_EXPERIMENTAL_HTTP
-#define NODE_VERSIONS_KEY_HTTP(V) V(llhttp)
-#else /* !NODE_EXPERIMENTAL_HTTP */
-#define NODE_VERSIONS_KEY_HTTP(V) V(http_parser)
-#endif /* NODE_EXPERIMENTAL_HTTP */
+  V(napi)                                                                      \
+  V(llhttp)                                                                    \
+  V(http_parser)                                                               \
 
 #if HAVE_OPENSSL
 #define NODE_VERSIONS_KEY_CRYPTO(V) V(openssl)
@@ -31,7 +27,6 @@ namespace node {
 
 #define NODE_VERSIONS_KEYS(V)                                                  \
   NODE_VERSIONS_KEYS_BASE(V)                                                   \
-  NODE_VERSIONS_KEY_HTTP(V)                                                    \
   NODE_VERSIONS_KEY_CRYPTO(V)
 
 class Metadata {
diff --git a/src/node_options.cc b/src/node_options.cc
index 2168b99075..885501839c 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -39,6 +39,11 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors) {
   if (syntax_check_only && has_eval_string) {
     errors->push_back("either --check or --eval can be used, not both");
   }
+
+  if (http_parser != "legacy" && http_parser != "llhttp") {
+    errors->push_back("invalid value for --http-parser");
+  }
+
   debug_options->CheckOptions(errors);
 }
 
@@ -102,6 +107,15 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             &EnvironmentOptions::experimental_worker,
             kAllowedInEnvironment);
   AddOption("--expose-internals", "", &EnvironmentOptions::expose_internals);
+  AddOption("--http-parser",
+            "Select which HTTP parser to use; either 'legacy' or 'llhttp' "
+#ifdef NODE_EXPERIMENTAL_HTTP_DEFAULT
+            "(default: llhttp).",
+#else
+            "(default: legacy).",
+#endif
+            &EnvironmentOptions::http_parser,
+            kAllowedInEnvironment);
   AddOption("--loader",
             "(with --experimental-modules) use the specified file as a "
             "custom loader",
diff --git a/src/node_options.h b/src/node_options.h
index 8e9ea6304e..dfe7ff6c5e 100644
--- a/src/node_options.h
+++ b/src/node_options.h
@@ -72,6 +72,12 @@ class EnvironmentOptions : public Options {
   bool experimental_vm_modules = false;
   bool experimental_worker = false;
   bool expose_internals = false;
+  std::string http_parser =
+#ifdef NODE_EXPERIMENTAL_HTTP_DEFAULT
+    "llhttp";
+#else
+    "legacy";
+#endif
   bool no_deprecation = false;
   bool no_force_async_hooks_checks = false;
   bool no_warnings = false;