Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

uhv: add http-parser permissive parsing mode for h1 codec #20528

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
b3f1e77
add new http parser setting to control permissive parsing for uhv
ameily Mar 21, 2022
c0d29e2
add http-parser tests
ameily Mar 21, 2022
87ce705
update http-parser unit tests to cover permissive parsing
ameily Mar 23, 2022
1c6e8c9
fix permissive parsing setting
ameily Mar 23, 2022
71e5328
add initial support for http-parser permissive parsing mode; refs #19750
ameily Mar 25, 2022
40969d7
Merge branch 'main' into uhv-h1-permissive-parsing
ameily Mar 25, 2022
77cf3d3
clang-format
ameily Mar 25, 2022
e5f763d
fix integration fix
ameily Mar 25, 2022
d7f0f7e
split strict and permissive http_parser parsing libraries
ameily Mar 31, 2022
bacf4a9
rename
ameily Mar 31, 2022
2eb58c9
only support permissive parsing
ameily Mar 31, 2022
6fe76f9
rename
ameily Mar 31, 2022
4e0428d
revert legacy parser
ameily Apr 1, 2022
83de91f
remove strict parsing
ameily Apr 1, 2022
0d23845
rename
ameily Apr 1, 2022
b8cf594
revert
ameily Apr 1, 2022
5f1c18c
add legacy permissive parser that uses http_parser__permissive library
ameily Apr 1, 2022
29ced6f
update http_parser__permissive build rules
ameily Apr 1, 2022
684f190
ignore http_parser__permissive directory
ameily Apr 1, 2022
1ca510e
remove http_parser_permissive tests
ameily Apr 4, 2022
f606ce6
revert to stock http-parser with HTTP_PARSER_STRICT disabled
ameily Apr 4, 2022
a0f1ccd
switch http1 codec tests to parameterized to exercise strict and perm…
ameily Apr 5, 2022
543512b
separate legacy strict and permissive parsing into 2 targets
ameily Apr 6, 2022
e4503fd
format
ameily Apr 6, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bazel/external/http_parser/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ cc_library(
# This compiler flag is set to an arbtitrarily high number so
# as to effectively disables the http_parser header limit, as
# we do our own checks in the conn manager and codec.
copts = ["-DHTTP_MAX_HEADER_SIZE=0x2000000"],
copts = ["-DHTTP_MAX_HEADER_SIZE=0x2000000", "-DHTTP_PARSER_STRICT"],
includes = ["."],
visibility = ["//visibility:public"],
)
18 changes: 18 additions & 0 deletions bazel/external/http_parser__permissive/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
load("@rules_cc//cc:defs.bzl", "cc_library")

licenses(["notice"]) # Apache 2

cc_library(
name = "http_parser__permissive",
srcs = [
"http_parser_permissive.cc",
"http_parser_permissive.h",
],
hdrs = ["http_parser_permissive.h"],
# This compiler flag is set to an arbtitrarily high number so
# as to effectively disables the http_parser header limit, as
# we do our own checks in the conn manager and codec.
copts = ["-DHTTP_MAX_HEADER_SIZE=0x2000000"],
includes = ["."],
visibility = ["//visibility:public"],
)
4 changes: 4 additions & 0 deletions bazel/external/http_parser__permissive/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
This is a clone of the [http-parser](https://github.com/nodejs/http-parser)
repo at commit `4f15b7d510dc7c6361a26a7c6d2f7c3a17f8d878`. See GitHub issue
[#19749](https://github.com/envoyproxy/envoy/issues/19749) for more
information.
2,579 changes: 2,579 additions & 0 deletions bazel/external/http_parser__permissive/http_parser_permissive.cc

Large diffs are not rendered by default.

445 changes: 445 additions & 0 deletions bazel/external/http_parser__permissive/http_parser_permissive.h

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions bazel/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,10 @@ def _com_github_nodejs_http_parser():
name = "http_parser",
actual = "@envoy//bazel/external/http_parser",
)
native.bind(
name = "http_parser__permissive",
actual = "@envoy//bazel/external/http_parser__permissive",
)

def _com_github_alibaba_hessian2_codec():
external_http_archive("com_github_alibaba_hessian2_codec")
Expand Down
8 changes: 8 additions & 0 deletions envoy/http/codec.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ const char MaxResponseHeadersCountOverrideKey[] =

class Stream;

/**
* Header validation mode that codecs should use internally.
*/
enum class CodecHeaderValidationMode {
Enabled,
Disabled,
};

/**
* Error codes used to convey the reason for a GOAWAY.
*/
Expand Down
2 changes: 1 addition & 1 deletion source/common/http/codec_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ NoConnectCodecClientProd::NoConnectCodecClientProd(CodecType type,
case CodecType::HTTP1: {
codec_ = std::make_unique<Http1::ClientConnectionImpl>(
*connection_, host->cluster().http1CodecStats(), *this, host->cluster().http1Settings(),
host->cluster().maxResponseHeadersCount());
host->cluster().maxResponseHeadersCount(), CodecHeaderValidationMode::Enabled);
break;
}
case CodecType::HTTP2: {
Expand Down
3 changes: 2 additions & 1 deletion source/common/http/conn_manager_utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ ServerConnectionPtr ConnectionManagerUtility::autoCreateCodec(
Http1::CodecStats& stats = Http1::CodecStats::atomicGet(http1_codec_stats, scope);
return std::make_unique<Http1::ServerConnectionImpl>(
connection, stats, callbacks, http1_settings, max_request_headers_kb,
max_request_headers_count, headers_with_underscores_action);
max_request_headers_count, headers_with_underscores_action,
CodecHeaderValidationMode::Enabled);
}
}

Expand Down
32 changes: 29 additions & 3 deletions source/common/http/http1/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ envoy_cc_library(
":codec_stats_lib",
":header_formatter_lib",
":legacy_parser_lib",
":legacy_permissive_parser_lib",
":parser_interface",
"//envoy/buffer:buffer_interface",
"//envoy/common:scope_tracker_interface",
Expand Down Expand Up @@ -108,11 +109,36 @@ envoy_cc_library(

envoy_cc_library(
name = "legacy_parser_lib",
srcs = ["legacy_parser_impl.cc"],
hdrs = ["legacy_parser_impl.h"],
external_deps = ["http_parser"],
srcs = [
"legacy_parser_impl.cc",
],
hdrs = [
"legacy_parser_impl.h",
],
external_deps = [
"http_parser",
],
deps = [
":parser_interface",
"//envoy/http:codec_interface",
"//source/common/common:assert_lib",
],
)

envoy_cc_library(
name = "legacy_permissive_parser_lib",
srcs = [
"legacy_permissive_parser_impl.cc",
],
hdrs = [
"legacy_permissive_parser_impl.h",
],
external_deps = [
"http_parser__permissive",
],
deps = [
":parser_interface",
"//envoy/http:codec_interface",
"//source/common/common:assert_lib",
],
)
24 changes: 16 additions & 8 deletions source/common/http/http1/codec_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "source/common/http/headers.h"
#include "source/common/http/http1/header_formatter.h"
#include "source/common/http/http1/legacy_parser_impl.h"
#include "source/common/http/http1/legacy_permissive_parser_impl.h"
#include "source/common/http/utility.h"
#include "source/common/runtime/runtime_features.h"

Expand Down Expand Up @@ -472,7 +473,8 @@ int ConnectionImpl::setAndCheckCallbackStatusOr(Envoy::StatusOr<ParserStatus>&&

ConnectionImpl::ConnectionImpl(Network::Connection& connection, CodecStats& stats,
const Http1Settings& settings, MessageType type,
uint32_t max_headers_kb, const uint32_t max_headers_count)
uint32_t max_headers_kb, const uint32_t max_headers_count,
const CodecHeaderValidationMode codec_header_validation_mode)
: connection_(connection), stats_(stats), codec_settings_(settings),
encode_only_header_key_formatter_(encodeOnlyFormatterFromSettings(settings)),
processing_trailers_(false), handling_upgrade_(false), reset_stream_called_(false),
Expand All @@ -483,7 +485,11 @@ ConnectionImpl::ConnectionImpl(Network::Connection& connection, CodecStats& stat
[]() -> void { /* TODO(adisuissa): Handle overflow watermark */ })),
max_headers_kb_(max_headers_kb), max_headers_count_(max_headers_count) {
output_buffer_->setWatermarks(connection.bufferLimit());
parser_ = std::make_unique<LegacyHttpParserImpl>(type, this);
if (codec_header_validation_mode == CodecHeaderValidationMode::Enabled) {
parser_ = std::make_unique<LegacyHttpParserImpl>(type, this);
} else {
parser_ = std::make_unique<LegacyPermissiveHttpParserImpl>(type, this);
}
}

Status ConnectionImpl::completeLastHeader() {
Expand Down Expand Up @@ -957,9 +963,10 @@ ServerConnectionImpl::ServerConnectionImpl(
const Http1Settings& settings, uint32_t max_request_headers_kb,
const uint32_t max_request_headers_count,
envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
headers_with_underscores_action)
headers_with_underscores_action,
const CodecHeaderValidationMode codec_header_validation_mode)
: ConnectionImpl(connection, stats, settings, MessageType::Request, max_request_headers_kb,
max_request_headers_count),
max_request_headers_count, codec_header_validation_mode),
callbacks_(callbacks),
response_buffer_releasor_([this](const Buffer::OwnedBufferFragmentImpl* fragment) {
releaseOutboundResponse(fragment);
Expand Down Expand Up @@ -1269,11 +1276,12 @@ void ServerConnectionImpl::ActiveRequest::dumpState(std::ostream& os, int indent
os << DUMP_MEMBER(response_encoder_.local_end_stream_);
}

ClientConnectionImpl::ClientConnectionImpl(Network::Connection& connection, CodecStats& stats,
ConnectionCallbacks&, const Http1Settings& settings,
const uint32_t max_response_headers_count)
ClientConnectionImpl::ClientConnectionImpl(
Network::Connection& connection, CodecStats& stats, ConnectionCallbacks&,
const Http1Settings& settings, const uint32_t max_response_headers_count,
const CodecHeaderValidationMode codec_header_validation_mode)
: ConnectionImpl(connection, stats, settings, MessageType::Response, MAX_RESPONSE_HEADERS_KB,
max_response_headers_count) {}
max_response_headers_count, codec_header_validation_mode) {}

bool ClientConnectionImpl::cannotHaveBody() {
if (pending_response_.has_value() && pending_response_.value().encoder_.headRequest()) {
Expand Down
9 changes: 6 additions & 3 deletions source/common/http/http1/codec_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,8 @@ class ConnectionImpl : public virtual Connection,

protected:
ConnectionImpl(Network::Connection& connection, CodecStats& stats, const Http1Settings& settings,
MessageType type, uint32_t max_headers_kb, const uint32_t max_headers_count);
MessageType type, uint32_t max_headers_kb, const uint32_t max_headers_count,
CodecHeaderValidationMode codec_header_validation_mode);

bool resetStreamCalled() { return reset_stream_called_; }

Expand Down Expand Up @@ -434,7 +435,8 @@ class ServerConnectionImpl : public ServerConnection, public ConnectionImpl {
ServerConnectionCallbacks& callbacks, const Http1Settings& settings,
uint32_t max_request_headers_kb, const uint32_t max_request_headers_count,
envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
headers_with_underscores_action);
headers_with_underscores_action,
const CodecHeaderValidationMode codec_header_validation_mode);
bool supportsHttp10() override { return codec_settings_.accept_http_10_; }

protected:
Expand Down Expand Up @@ -550,7 +552,8 @@ class ClientConnectionImpl : public ClientConnection, public ConnectionImpl {
public:
ClientConnectionImpl(Network::Connection& connection, CodecStats& stats,
ConnectionCallbacks& callbacks, const Http1Settings& settings,
const uint32_t max_response_headers_count);
const uint32_t max_response_headers_count,
const CodecHeaderValidationMode codec_header_validation_mode);
// Http::ClientConnection
RequestEncoder& newStream(ResponseDecoder& response_decoder) override;

Expand Down
Loading