diff options
author | Anna Henningsen <anna@addaleax.net> | 2018-11-30 09:39:02 +0300 |
---|---|---|
committer | Daniel Bevenius <daniel.bevenius@gmail.com> | 2018-12-06 07:21:36 +0300 |
commit | aa943d098e0299ea87485a607353d152f5ea5012 (patch) | |
tree | b3f2cd52efc4568d5f7deb6ca99684e2f5d22a62 /src | |
parent | 7bcbf044ddc864be9d6c711a75f3a33b6a3c652e (diff) |
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>
Diffstat (limited to 'src')
-rw-r--r-- | src/inspector_socket.cc | 16 | ||||
-rw-r--r-- | src/node_binding.cc | 1 | ||||
-rw-r--r-- | src/node_http_parser_impl.h (renamed from src/node_http_parser.cc) | 18 | ||||
-rw-r--r-- | src/node_http_parser_llhttp.cc | 17 | ||||
-rw-r--r-- | src/node_http_parser_traditional.cc | 18 | ||||
-rw-r--r-- | src/node_internals.h | 3 | ||||
-rw-r--r-- | src/node_metadata.cc | 16 | ||||
-rw-r--r-- | src/node_metadata.h | 11 | ||||
-rw-r--r-- | src/node_options.cc | 14 | ||||
-rw-r--r-- | src/node_options.h | 6 |
10 files changed, 86 insertions, 34 deletions
diff --git a/src/inspector_socket.cc b/src/inspector_socket.cc index 8c2d0a5a22d..dcc42f25b4a 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 a2e5134bfa1..6d70d0a501e 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 4d683b4db2c..4233ccbfcaa 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 00000000000..7b9999d13c7 --- /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 00000000000..2bff20c1656 --- /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 6cb40c9070e..1d43d4b1419 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 1bf2fcfce95..fd37dbb97bf 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 0f32fcf21d1..9f383be5f8b 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 2168b990754..885501839c6 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 8e9ea6304e6..dfe7ff6c5e1 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; |