http: switch default parser to llhttp

Refs: #24739 Fixes: #24730 PR-URL: #24870 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
nodejs · Dec 9, 2018 · 2cb8f24 · 2cb8f24
1 parent 951b1c3
commit 2cb8f24
Show file tree

Hide file tree

Showing 7 changed files with 3 additions and 32 deletions.
diff --git a/configure.py b/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 by default')
+    help='(no-op)')
 
 shared_optgroup.add_option('--shared-http-parser',
     action='store_true',
@@ -1118,9 +1118,6 @@ def configure_node(o):
   else:
     o['variables']['node_target_type'] = 'executable'
 
-  o['variables']['node_experimental_http_parser'] = \
-      b(options.experimental_http_parser)
-
 def configure_library(lib, output):
   shared_lib = 'shared_' + lib
   output['variables']['node_' + shared_lib] = b(getattr(options, shared_lib))

diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -129,7 +129,7 @@ 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.
+The default is `llhttp`, unless otherwise specified when building Node.js.
 
 This flag exists to aid in experimentation with the internal implementation of
 the Node.js http parser.

diff --git a/node.gyp b/node.gyp
@@ -12,7 +12,6 @@
     'force_dynamic_crt%': 0,
     'node_module_version%': '',
     'node_shared_zlib%': 'false',
-    'node_experimental_http_parser%': 'false',
     'node_shared_http_parser%': 'false',
     'node_shared_cares%': 'false',
     'node_shared_libuv%': 'false',

diff --git a/node.gypi b/node.gypi
@@ -163,10 +163,6 @@
       ],
     }],
 
-    [ 'node_experimental_http_parser=="true"', {
-      'defines': [ 'NODE_EXPERIMENTAL_HTTP_DEFAULT' ],
-    } ],
-
     [ 'node_shared_http_parser=="false"', {
       'dependencies': [
         'deps/http_parser/http_parser.gyp:http_parser',

diff --git a/src/inspector_socket.cc b/src/inspector_socket.cc
@@ -1,8 +1,6 @@
 #include "inspector_socket.h"
 
-#ifdef NODE_EXPERIMENTAL_HTTP_DEFAULT
 #define NODE_EXPERIMENTAL_HTTP
-#endif
 #include "http_parser_adaptor.h"
 
 #include "util-inl.h"
@@ -437,13 +435,8 @@ class HttpHandler : public ProtocolHandler {
   explicit HttpHandler(InspectorSocket* inspector, TcpHolder::Pointer tcp)
                        : ProtocolHandler(inspector, std::move(tcp)),
                          parsing_value_(false) {
-#ifdef NODE_EXPERIMENTAL_HTTP_DEFAULT
     llhttp_init(&parser_, HTTP_REQUEST, &parser_settings);
     llhttp_settings_init(&parser_settings);
-#else  /* !NODE_EXPERIMENTAL_HTTP_DEFAULT */
-    http_parser_init(&parser_, HTTP_REQUEST);
-    http_parser_settings_init(&parser_settings);
-#endif  /* NODE_EXPERIMENTAL_HTTP_DEFAULT */
     parser_settings.on_header_field = OnHeaderField;
     parser_settings.on_header_value = OnHeaderValue;
     parser_settings.on_message_complete = OnMessageComplete;
@@ -488,17 +481,12 @@ class HttpHandler : public ProtocolHandler {
 
   void OnData(std::vector<char>* data) override {
     parser_errno_t err;
-#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_DEFAULT */
-    http_parser_execute(&parser_, &parser_settings, data->data(), data->size());
-    err = HTTP_PARSER_ERRNO(&parser_);
-#endif  /* NODE_EXPERIMENTAL_HTTP_DEFAULT */
     data->clear();
     if (err != HPE_OK) {
       CancelHandshake();

diff --git a/src/node_options.cc b/src/node_options.cc
@@ -111,11 +111,7 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
   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",

diff --git a/src/node_options.h b/src/node_options.h
@@ -97,12 +97,7 @@ 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
+  std::string http_parser = "llhttp";
   bool no_deprecation = false;
   bool no_force_async_hooks_checks = false;
   bool no_warnings = false;