Skip to content

Commit

Permalink
http: switch default parser to llhttp
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
addaleax committed Dec 9, 2018
1 parent 951b1c3 commit 2cb8f24
Show file tree
Hide file tree
Showing 7 changed files with 3 additions and 32 deletions.
5 changes: 1 addition & 4 deletions configure.py
Expand Up @@ -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',
Expand Down Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion doc/api/cli.md
Expand Up @@ -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.
Expand Down
1 change: 0 additions & 1 deletion node.gyp
Expand Up @@ -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',
Expand Down
4 changes: 0 additions & 4 deletions node.gypi
Expand Up @@ -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',
Expand Down
12 changes: 0 additions & 12 deletions 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"
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
4 changes: 0 additions & 4 deletions src/node_options.cc
Expand Up @@ -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",
Expand Down
7 changes: 1 addition & 6 deletions src/node_options.h
Expand Up @@ -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;
Expand Down

0 comments on commit 2cb8f24

Please sign in to comment.