diff --git a/DEPRECATED.md b/DEPRECATED.md index 91ccc6259f73c..fc0df1f0aed11 100644 --- a/DEPRECATED.md +++ b/DEPRECATED.md @@ -9,3 +9,4 @@ The following features have been DEPRECATED and will be removed in the specified * Config option `statsd_local_udp_port` has been deprecated and has been replaced with `statsd_udp_ip_address`. * `HttpFilterConfigFactory` filter API has been deprecated in favor of `NamedHttpFilterConfigFactory`. + * Config option `http_codec_options` has been deprecated and has been replaced with `http2_settings`. diff --git a/docs/configuration/cluster_manager/cluster.rst b/docs/configuration/cluster_manager/cluster.rst index 4db70834b0810..8ba4910e842fc 100644 --- a/docs/configuration/cluster_manager/cluster.rst +++ b/docs/configuration/cluster_manager/cluster.rst @@ -19,6 +19,7 @@ Cluster "ssl_context": "{...}", "features": "...", "http_codec_options": "...", + "http2_settings": "{...}", "dns_refresh_rate_ms": "...", "dns_lookup_family": "...", "outlier_detection": "{...}" @@ -137,6 +138,13 @@ http_codec_options an HTTP/2 mesh, if it's desired to disable HTTP/2 header compression the *no_compression* option should be specified both here as well as in the HTTP connection manager. +.. _config_cluster_manager_cluster_http2_settings: + +http2_settings + *(optional, object)* Additional HTTP/2 settings that are passed directly to the HTTP/2 codec when + initiating HTTP connection pool connections. These are the same options supported in the HTTP connection + manager :ref:`http2_settings ` option. + .. _config_cluster_manager_cluster_dns_refresh_rate_ms: dns_refresh_rate_ms diff --git a/docs/configuration/http_conn_man/http_conn_man.rst b/docs/configuration/http_conn_man/http_conn_man.rst index 7f71192df1dbb..c2b109207cf29 100644 --- a/docs/configuration/http_conn_man/http_conn_man.rst +++ b/docs/configuration/http_conn_man/http_conn_man.rst @@ -20,6 +20,7 @@ HTTP connection manager "add_user_agent": "...", "tracing": "{...}", "http_codec_options": "...", + "http2_settings": "{...}", "server_name": "...", "idle_timeout_s": "...", "drain_timeout_ms": "...", @@ -85,7 +86,9 @@ add_user_agent .. _config_http_conn_man_http_codec_options: -http_codec_options +http_codec_options (Warning: DEPRECATED and will be removed in 1.4.0) + **DEPRECATED**, use :ref:`http2_settings ` instead. + *(optional, string)* Additional options that are passed directly to the codec. Not all options are applicable to all codecs. Possible values are: @@ -97,6 +100,44 @@ http_codec_options ` option. See the comment there about disabling HTTP/2 header compression. +.. _config_http_conn_man_http2_settings: + +http2_settings + *(optional, object)* Additional HTTP/2 settings that are passed directly to the HTTP/2 codec. + Currently supported settings are: + + hpack_table_size + *(optional, integer)* `Maximum table size `_ + (in octets) that the encoder is permitted to use for + the dynamic HPACK table. Valid values range from 0 to 4294967295 (2^32 - 1) and defaults to 4096. + 0 effectively disables header compression. + + max_concurrent_streams + *(optional, integer)* `Maximum concurrent streams + `_ + allowed for peer on one HTTP/2 connection. + Valid values range from 1 to 2147483647 (2^31 - 1) and defaults to 2147483647. + +.. _config_http_conn_man_http2_settings_initial_stream_window_size: + + initial_stream_window_size + *(optional, integer)* `Initial stream-level flow-control window + `_ size. Valid values range from 65535 + (2^16 - 1, HTTP/2 default) to 2147483647 (2^31 - 1, HTTP/2 maximum) and defaults to 268435456 + (256 * 1024 * 1024). + + NOTE: 65535 is the initial window size from HTTP/2 spec. We only support increasing the default window + size now, so it's also the minimum. + + initial_connection_window_size + *(optional, integer)* Similar to :ref:`initial_stream_window_size + `, but for connection-level flow-control + window. Currently , this has the same minimum/maximum/default as :ref:`initial_stream_window_size + `. + + These are the same options available in the upstream cluster :ref:`http2_settings + ` option. + .. _config_http_conn_man_server_name: server_name diff --git a/include/envoy/http/codec.h b/include/envoy/http/codec.h index e0f8e803e195c..a1ced838c2075 100644 --- a/include/envoy/http/codec.h +++ b/include/envoy/http/codec.h @@ -148,11 +148,47 @@ class ConnectionCallbacks { }; /** - * A list of options that can be specified when creating a codec. + * HTTP/2 codec settings */ -class CodecOptions { -public: - static const uint64_t NoCompression = 0x1; +struct Http2Settings { + // TODO(jwfang): support other HTTP/2 settings + uint32_t hpack_table_size_{DEFAULT_HPACK_TABLE_SIZE}; + uint32_t max_concurrent_streams_{DEFAULT_MAX_CONCURRENT_STREAMS}; + uint32_t initial_stream_window_size_{DEFAULT_INITIAL_STREAM_WINDOW_SIZE}; + uint32_t initial_connection_window_size_{DEFAULT_INITIAL_CONNECTION_WINDOW_SIZE}; + + // disable HPACK compression + static const uint32_t MIN_HPACK_TABLE_SIZE = 0; + // initial value from HTTP/2 spec, same as NGHTTP2_DEFAULT_HEADER_TABLE_SIZE from nghttp2 + static const uint32_t DEFAULT_HPACK_TABLE_SIZE = (1 << 12); + // no maximum from HTTP/2 spec, use unsigned 32-bit maximum + static const uint32_t MAX_HPACK_TABLE_SIZE = (1UL << 32) - 1; + + // TODO(jwfang): make this 0, the HTTP/2 spec minimum + static const uint32_t MIN_MAX_CONCURRENT_STREAMS = 1; + // defaults to maximum, same as nghttp2 + static const uint32_t DEFAULT_MAX_CONCURRENT_STREAMS = (1U << 31) - 1; + // no maximum from HTTP/2 spec, total streams is unsigned 32-bit maximum, + // one-side (client/server) is half that, and we need to exclude stream 0. + // same as NGHTTP2_INITIAL_MAX_CONCURRENT_STREAMS from nghttp2 + static const uint32_t MAX_MAX_CONCURRENT_STREAMS = (1U << 31) - 1; + + // initial value from HTTP/2 spec, same as NGHTTP2_INITIAL_WINDOW_SIZE from nghttp2 + // NOTE: we only support increasing window size now, so this is also the minimum + // TODO(jwfang): make this 0 to support decrease window size + static const uint32_t MIN_INITIAL_STREAM_WINDOW_SIZE = (1 << 16) - 1; + // initial value from HTTP/2 spec is 65535, but we want more (256MiB) + static const uint32_t DEFAULT_INITIAL_STREAM_WINDOW_SIZE = 256 * 1024 * 1024; + // maximum from HTTP/2 spec, same as NGHTTP2_MAX_WINDOW_SIZE from nghttp2 + static const uint32_t MAX_INITIAL_STREAM_WINDOW_SIZE = (1U << 31) - 1; + + // CONNECTION_WINDOW_SIZE is similar to STREAM_WINDOW_SIZE, but for connection-level window + // TODO(jwfang): make this 0 to support decrease window size + static const uint32_t MIN_INITIAL_CONNECTION_WINDOW_SIZE = (1 << 16) - 1; + // nghttp2's default connection-level window equals to its stream-level, + // our default connection-level window also equals to our stream-level + static const uint32_t DEFAULT_INITIAL_CONNECTION_WINDOW_SIZE = 256 * 1024 * 1024; + static const uint32_t MAX_INITIAL_CONNECTION_WINDOW_SIZE = (1U << 31) - 1; }; /** diff --git a/include/envoy/upstream/BUILD b/include/envoy/upstream/BUILD index c63deb669e4f7..0b7071a3087fc 100644 --- a/include/envoy/upstream/BUILD +++ b/include/envoy/upstream/BUILD @@ -77,6 +77,7 @@ envoy_cc_library( ":load_balancer_type_interface", ":resource_manager_interface", "//include/envoy/common:optional", + "//include/envoy/http:codec_interface", "//include/envoy/network:connection_interface", "//include/envoy/ssl:context_interface", ], diff --git a/include/envoy/upstream/upstream.h b/include/envoy/upstream/upstream.h index fbc2ce337d2e5..1489e21386b1f 100644 --- a/include/envoy/upstream/upstream.h +++ b/include/envoy/upstream/upstream.h @@ -9,6 +9,7 @@ #include #include "envoy/common/optional.h" +#include "envoy/http/codec.h" #include "envoy/network/connection.h" #include "envoy/ssl/context.h" #include "envoy/upstream/load_balancer_type.h" @@ -243,10 +244,10 @@ class ClusterInfo { virtual uint64_t features() const PURE; /** - * @return uint64_t HTTP codec options for HTTP connections created on behalf of this cluster. - * @see Http::CodecOptions. + * @return const Http::Http2Settings& for HTTP/2 connections created on behalf of this cluster. + * @see Http::Http2Settings. */ - virtual uint64_t httpCodecOptions() const PURE; + virtual const Http::Http2Settings& http2Settings() const PURE; /** * @return the type of load balancing that the cluster should use. diff --git a/source/common/http/codec_client.cc b/source/common/http/codec_client.cc index 3c0a402921834..9092f4ce37e28 100644 --- a/source/common/http/codec_client.cc +++ b/source/common/http/codec_client.cc @@ -128,7 +128,7 @@ CodecClientProd::CodecClientProd(Type type, Network::ClientConnectionPtr&& conne } case Type::HTTP2: { codec_.reset(new Http2::ClientConnectionImpl(*connection_, *this, host->cluster().statsScope(), - host->cluster().httpCodecOptions())); + host->cluster().http2Settings())); break; } } diff --git a/source/common/http/http2/codec_impl.cc b/source/common/http/http2/codec_impl.cc index 02e286344a187..284ffa11b9ec2 100644 --- a/source/common/http/http2/codec_impl.cc +++ b/source/common/http/http2/codec_impl.cc @@ -506,24 +506,59 @@ void ConnectionImpl::sendPendingFrames() { } } -void ConnectionImpl::sendSettings(uint64_t codec_options) { - std::vector iv = { - {NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS, MAX_CONCURRENT_STREAMS}, - {NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE, DEFAULT_WINDOW_SIZE}}; +void ConnectionImpl::sendSettings(const Http2Settings& http2_settings) { + ASSERT(http2_settings.hpack_table_size_ <= Http2Settings::MAX_HPACK_TABLE_SIZE); + ASSERT(Http2Settings::MIN_MAX_CONCURRENT_STREAMS <= http2_settings.max_concurrent_streams_ && + http2_settings.max_concurrent_streams_ <= Http2Settings::MAX_MAX_CONCURRENT_STREAMS); + ASSERT( + Http2Settings::MIN_INITIAL_STREAM_WINDOW_SIZE <= http2_settings.initial_stream_window_size_ && + http2_settings.initial_stream_window_size_ <= Http2Settings::MAX_INITIAL_STREAM_WINDOW_SIZE); + ASSERT(Http2Settings::MIN_INITIAL_CONNECTION_WINDOW_SIZE <= + http2_settings.initial_connection_window_size_ && + http2_settings.initial_connection_window_size_ <= + Http2Settings::MAX_INITIAL_CONNECTION_WINDOW_SIZE); + + std::vector iv; + + if (http2_settings.hpack_table_size_ != NGHTTP2_DEFAULT_HEADER_TABLE_SIZE) { + iv.push_back({NGHTTP2_SETTINGS_HEADER_TABLE_SIZE, http2_settings.hpack_table_size_}); + conn_log_debug("setting HPACK table size to {}", connection_, http2_settings.hpack_table_size_); + } - if (codec_options & CodecOptions::NoCompression) { - iv.push_back({NGHTTP2_SETTINGS_HEADER_TABLE_SIZE, 0}); - conn_log_debug("disabling header compression", connection_); + if (http2_settings.max_concurrent_streams_ != NGHTTP2_INITIAL_MAX_CONCURRENT_STREAMS) { + iv.push_back({NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS, http2_settings.max_concurrent_streams_}); + conn_log_debug("setting max concurrent streams to {}", connection_, + http2_settings.max_concurrent_streams_); } - int rc = nghttp2_submit_settings(session_, NGHTTP2_FLAG_NONE, &iv[0], iv.size()); - ASSERT(rc == 0); - UNREFERENCED_PARAMETER(rc); + if (http2_settings.initial_stream_window_size_ != NGHTTP2_INITIAL_WINDOW_SIZE) { + iv.push_back( + {NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE, http2_settings.initial_stream_window_size_}); + conn_log_debug("setting stream-level initial window size to {}", connection_, + http2_settings.initial_stream_window_size_); + } + + if (!iv.empty()) { + int rc = nghttp2_submit_settings(session_, NGHTTP2_FLAG_NONE, &iv[0], iv.size()); + ASSERT(rc == 0); + UNREFERENCED_PARAMETER(rc); + } else { + // nghttp2_submit_settings need to be called at least once + int rc = nghttp2_submit_settings(session_, NGHTTP2_FLAG_NONE, 0, 0); + ASSERT(rc == 0); + UNREFERENCED_PARAMETER(rc); + } // Increase connection window size up to our default size. - rc = nghttp2_submit_window_update(session_, NGHTTP2_FLAG_NONE, 0, - DEFAULT_WINDOW_SIZE - NGHTTP2_INITIAL_CONNECTION_WINDOW_SIZE); - ASSERT(rc == 0); + if (http2_settings.initial_connection_window_size_ != NGHTTP2_INITIAL_CONNECTION_WINDOW_SIZE) { + conn_log_debug("updating connection-level initial window size to {}", connection_, + http2_settings.initial_connection_window_size_); + int rc = nghttp2_submit_window_update(session_, NGHTTP2_FLAG_NONE, 0, + http2_settings.initial_connection_window_size_ - + NGHTTP2_INITIAL_CONNECTION_WINDOW_SIZE); + ASSERT(rc == 0); + UNREFERENCED_PARAMETER(rc); + } } ConnectionImpl::Http2Callbacks::Http2Callbacks() { @@ -602,7 +637,7 @@ ConnectionImpl::Http2Options::Http2Options() { // Currently we do not do anything with stream priority. Setting the following option prevents // nghttp2 from keeping around closed streams for use during stream priority dependency graph // calculations. This saves a tremendous amount of memory in cases where there are a large number - // of kept alive http/2 connections. + // of kept alive HTTP/2 connections. nghttp2_option_set_no_closed_streams(options_, 1); } @@ -610,11 +645,11 @@ ConnectionImpl::Http2Options::~Http2Options() { nghttp2_option_del(options_); } ClientConnectionImpl::ClientConnectionImpl(Network::Connection& connection, ConnectionCallbacks& callbacks, Stats::Scope& stats, - uint64_t codec_options) + const Http2Settings& http2_settings) : ConnectionImpl(connection, stats), callbacks_(callbacks) { nghttp2_session_client_new2(&session_, http2_callbacks_.callbacks(), base(), http2_options_.options()); - sendSettings(codec_options); + sendSettings(http2_settings); } Http::StreamEncoder& ClientConnectionImpl::newStream(StreamDecoder& decoder) { @@ -648,11 +683,11 @@ int ClientConnectionImpl::onHeader(const nghttp2_frame* frame, HeaderString&& na ServerConnectionImpl::ServerConnectionImpl(Network::Connection& connection, Http::ServerConnectionCallbacks& callbacks, - Stats::Store& stats, uint64_t codec_options) + Stats::Store& stats, const Http2Settings& http2_settings) : ConnectionImpl(connection, stats), callbacks_(callbacks) { nghttp2_session_server_new2(&session_, http2_callbacks_.callbacks(), base(), http2_options_.options()); - sendSettings(codec_options); + sendSettings(http2_settings); } int ServerConnectionImpl::onBeginHeaders(const nghttp2_frame* frame) { diff --git a/source/common/http/http2/codec_impl.h b/source/common/http/http2/codec_impl.h index 3997de689d2ea..4e3041f779f3a 100644 --- a/source/common/http/http2/codec_impl.h +++ b/source/common/http/http2/codec_impl.h @@ -32,7 +32,7 @@ const std::string ALPN_STRING = "h2"; const std::string CLIENT_MAGIC_PREFIX = "PRI * HTTP/2"; /** - * All stats for the http/2 codec. @see stats_macros.h + * All stats for the HTTP/2 codec. @see stats_macros.h */ // clang-format off #define ALL_HTTP2_CODEC_STATS(COUNTER) \ @@ -44,7 +44,7 @@ const std::string CLIENT_MAGIC_PREFIX = "PRI * HTTP/2"; // clang-format on /** - * Wrapper struct for the http/2 codec stats. @see stats_macros.h + * Wrapper struct for the HTTP/2 codec stats. @see stats_macros.h */ struct CodecStats { ALL_HTTP2_CODEC_STATS(GENERATE_COUNTER_STRUCT) @@ -82,8 +82,6 @@ class ConnectionImpl : public virtual Connection, Logger::Loggable CONTINUE_HEADER; Network::Connection& connection_; @@ -233,7 +227,7 @@ class ConnectionImpl : public virtual Connection, Logger::Loggablevalue().c_str()); } -uint64_t Utility::parseCodecOptions(const Json::Object& config) { - uint64_t ret = 0; +Http2Settings Utility::parseHttp2Settings(const Json::Object& config) { + Http2Settings ret; + + Json::ObjectSharedPtr http2_settings = config.getObject("http2_settings", true); + ret.hpack_table_size_ = + http2_settings->getInteger("hpack_table_size", Http::Http2Settings::DEFAULT_HPACK_TABLE_SIZE); + ret.max_concurrent_streams_ = http2_settings->getInteger( + "max_concurrent_streams", Http::Http2Settings::DEFAULT_MAX_CONCURRENT_STREAMS); + ret.initial_stream_window_size_ = http2_settings->getInteger( + "initial_stream_window_size", Http::Http2Settings::DEFAULT_INITIAL_STREAM_WINDOW_SIZE); + ret.initial_connection_window_size_ = + http2_settings->getInteger("initial_connection_window_size", + Http::Http2Settings::DEFAULT_INITIAL_CONNECTION_WINDOW_SIZE); + + // http_codec_options config is DEPRECATED std::string options = config.getString("http_codec_options", ""); + if (options != "") { + spdlog::logger& logger = Logger::Registry::getLog(Logger::Id::config); + logger.warn("'http_codec_options' is DEPRECATED, please use 'http2_settings' instead"); + } + for (const std::string& option : StringUtil::split(options, ',')) { if (option == "no_compression") { - ret |= CodecOptions::NoCompression; + if (http2_settings->hasObject("hpack_table_size") && ret.hpack_table_size_ != 0) { + throw EnvoyException( + "'http_codec_options.no_compression' conflicts with 'http2_settings.hpack_table_size'"); + } + ret.hpack_table_size_ = 0; } else { throw EnvoyException(fmt::format("unknown http codec option '{}'", option)); } diff --git a/source/common/http/utility.h b/source/common/http/utility.h index 91ce05d82c426..e7d16d8aaaae9 100644 --- a/source/common/http/utility.h +++ b/source/common/http/utility.h @@ -64,10 +64,10 @@ class Utility { static bool isInternalRequest(const HeaderMap& headers); /** - * @return uint64_t parse a "http_codec_options" JSON field and turn it into a bitmask of - * CodecOption values. + * @return Http2Settings An Http2Settings populated from the "http_codec_options" and + * "http2_settings" JSON fields. */ - static uint64_t parseCodecOptions(const Json::Object& config); + static Http2Settings parseHttp2Settings(const Json::Object& config); /** * Create a locally generated response using filter callbacks. diff --git a/source/common/json/config_schemas.cc b/source/common/json/config_schemas.cc index dc90186bbc0f2..d2f47bb89c140 100644 --- a/source/common/json/config_schemas.cc +++ b/source/common/json/config_schemas.cc @@ -262,6 +262,31 @@ const std::string Json::Schema::HTTP_CONN_NETWORK_FILTER_SCHEMA(R"EOF( "type" : "string", "enum" : ["no_compression"] }, + "http2_settings" : { + "type" : "object", + "properties" : { + "hpack_table_size" : { + "type": "integer", + "minimum": 0, + "maximum" : 4294967295 + }, + "max_concurrent_streams" : { + "type": "integer", + "minimum": 1, + "maximum" : 2147483647 + }, + "initial_stream_window_size" : { + "type": "integer", + "minimum": 65535, + "maximum" : 2147483647 + }, + "initial_connection_window_size" : { + "type": "integer", + "minimum": 65535, + "maximum" : 2147483647 + } + } + }, "server_name" : {"type" : "string"}, "idle_timeout_s" : {"type" : "integer"}, "drain_timeout_ms" : {"type" : "integer"}, @@ -1205,7 +1230,35 @@ const std::string Json::Schema::CLUSTER_SCHEMA(R"EOF( "type" : "string", "enum" : ["http2"] }, - "http_codec_options" : {"type" : "string"}, + "http_codec_options" : { + "type" : "string", + "enum" : ["no_compression"] + }, + "http2_settings" : { + "type" : "object", + "properties" : { + "hpack_table_size" : { + "type": "integer", + "minimum": 0, + "maximum" : 4294967295 + }, + "max_concurrent_streams" : { + "type": "integer", + "minimum": 1, + "maximum" : 2147483647 + }, + "initial_stream_window_size" : { + "type": "integer", + "minimum": 65535, + "maximum" : 2147483647 + }, + "initial_connection_window_size" : { + "type": "integer", + "minimum": 65535, + "maximum" : 2147483647 + } + } + }, "dns_refresh_rate_ms" : { "type" : "integer", "minimum" : 0, diff --git a/source/common/router/router.cc b/source/common/router/router.cc index cd294e0b17532..90938261fb5c2 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -722,7 +722,7 @@ void Filter::UpstreamRequest::onPoolReady(Http::StreamEncoder& request_encoder, calling_encode_headers_ = false; // It is possible to get reset in the middle of an encodeHeaders() call. This happens for example - // in the http/2 codec if the frame cannot be encoded for some reason. This should never happen + // in the HTTP/2 codec if the frame cannot be encoded for some reason. This should never happen // but it's unclear if we have covered all cases so protect against it and test for it. One // specific example of a case where this happens is if we try to encode a total header size that // is too big in HTTP/2 (64K currently). diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 578a937686b8d..7998854b9be02 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -74,7 +74,7 @@ ClusterInfoImpl::ClusterInfoImpl(const Json::Object& config, Runtime::Loader& ru config.getInteger("per_connection_buffer_limit_bytes", 1024 * 1024)), stats_scope_(stats.createScope(fmt::format("cluster.{}.", name_))), stats_(generateStats(*stats_scope_)), features_(parseFeatures(config)), - http_codec_options_(Http::Utility::parseCodecOptions(config)), + http2_settings_(Http::Utility::parseHttp2Settings(config)), resource_managers_(config, runtime, name_), maintenance_mode_runtime_key_(fmt::format("upstream.maintenance_mode.{}", name_)) { diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index 1c62a19243edd..c8af3a2d96d31 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -176,7 +176,7 @@ class ClusterInfoImpl : public ClusterInfo { return per_connection_buffer_limit_bytes_; } uint64_t features() const override { return features_; } - uint64_t httpCodecOptions() const override { return http_codec_options_; } + const Http::Http2Settings& http2Settings() const override { return http2_settings_; } LoadBalancerType lbType() const override { return lb_type_; } bool maintenanceMode() const override; uint64_t maxRequestsPerConnection() const override { return max_requests_per_connection_; } @@ -209,7 +209,7 @@ class ClusterInfoImpl : public ClusterInfo { mutable ClusterStats stats_; Ssl::ClientContextPtr ssl_ctx_; const uint64_t features_; - const uint64_t http_codec_options_; + const Http::Http2Settings http2_settings_; mutable ResourceManagers resource_managers_; const std::string maintenance_mode_runtime_key_; LoadBalancerType lb_type_; diff --git a/source/server/config/network/http_connection_manager.cc b/source/server/config/network/http_connection_manager.cc index 307df3e90c492..8c3e4955a2d79 100644 --- a/source/server/config/network/http_connection_manager.cc +++ b/source/server/config/network/http_connection_manager.cc @@ -77,7 +77,7 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(const Json::Object& con stats_(Http::ConnectionManagerImpl::generateStats(stats_prefix_, server.stats())), tracing_stats_( Http::ConnectionManagerImpl::generateTracingStats(stats_prefix_, server.stats())), - codec_options_(Http::Utility::parseCodecOptions(config)), + http2_settings_(Http::Utility::parseHttp2Settings(config)), drain_timeout_(config.getInteger("drain_timeout_ms", 5000)), generate_request_id_(config.getBoolean("generate_request_id", true)), date_provider_(server.dispatcher(), server.threadLocal()) { @@ -193,12 +193,12 @@ HttpConnectionManagerConfig::createCodec(Network::Connection& connection, return Http::ServerConnectionPtr{new Http::Http1::ServerConnectionImpl(connection, callbacks)}; case CodecType::HTTP2: return Http::ServerConnectionPtr{new Http::Http2::ServerConnectionImpl( - connection, callbacks, server_.stats(), codec_options_)}; + connection, callbacks, server_.stats(), http2_settings_)}; case CodecType::AUTO: if (HttpConnectionManagerConfigUtility::determineNextProtocol(connection, data) == Http::Http2::ALPN_STRING) { return Http::ServerConnectionPtr{new Http::Http2::ServerConnectionImpl( - connection, callbacks, server_.stats(), codec_options_)}; + connection, callbacks, server_.stats(), http2_settings_)}; } else { return Http::ServerConnectionPtr{ new Http::Http1::ServerConnectionImpl(connection, callbacks)}; diff --git a/source/server/config/network/http_connection_manager.h b/source/server/config/network/http_connection_manager.h index 304874056d268..fc11648f29768 100644 --- a/source/server/config/network/http_connection_manager.h +++ b/source/server/config/network/http_connection_manager.h @@ -196,7 +196,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, Http::ConnectionManagerTracingStats tracing_stats_; bool use_remote_address_{}; CodecType codec_type_; - const uint64_t codec_options_; + const Http::Http2Settings http2_settings_; std::string server_name_; Http::TracingConnectionManagerConfigPtr tracing_config_; Optional user_agent_; diff --git a/test/common/http/http2/codec_impl_test.cc b/test/common/http/http2/codec_impl_test.cc index 58538fd632693..b8d5df88db964 100644 --- a/test/common/http/http2/codec_impl_test.cc +++ b/test/common/http/http2/codec_impl_test.cc @@ -16,6 +16,7 @@ #include "gtest/gtest.h" namespace Envoy { + using testing::_; using testing::AtLeast; using testing::InSequence; @@ -25,7 +26,21 @@ using testing::NiceMock; namespace Http { namespace Http2 { -class Http2CodecImplTest : public testing::TestWithParam { +typedef ::testing::tuple Http2SettingsTuple; +typedef ::testing::tuple Http2SettingsTestParam; + +namespace { +Http2Settings Http2SettingsFromTuple(const Http2SettingsTuple& tp) { + Http2Settings ret; + ret.hpack_table_size_ = ::testing::get<0>(tp); + ret.max_concurrent_streams_ = ::testing::get<1>(tp); + ret.initial_stream_window_size_ = ::testing::get<2>(tp); + ret.initial_connection_window_size_ = ::testing::get<3>(tp); + return ret; +} +} + +class Http2CodecImplTest : public testing::TestWithParam { public: struct ConnectionWrapper { void dispatch(const Buffer::Instance& data, ConnectionImpl& connection) { @@ -44,8 +59,10 @@ class Http2CodecImplTest : public testing::TestWithParam { }; Http2CodecImplTest() - : client_(client_connection_, client_callbacks_, stats_store_, GetParam()), - server_(server_connection_, server_callbacks_, stats_store_, GetParam()), + : client_http2settings_(Http2SettingsFromTuple(::testing::get<0>(GetParam()))), + client_(client_connection_, client_callbacks_, stats_store_, client_http2settings_), + server_http2settings_(Http2SettingsFromTuple(::testing::get<1>(GetParam()))), + server_(server_connection_, server_callbacks_, stats_store_, server_http2settings_), request_encoder_(client_.newStream(response_decoder_)) { setupDefaultConnectionMocks(); @@ -67,10 +84,12 @@ class Http2CodecImplTest : public testing::TestWithParam { } Stats::IsolatedStoreImpl stats_store_; + const Http2Settings client_http2settings_; NiceMock client_connection_; MockConnectionCallbacks client_callbacks_; ClientConnectionImpl client_; ConnectionWrapper client_wrapper_; + const Http2Settings server_http2settings_; NiceMock server_connection_; MockServerConnectionCallbacks server_callbacks_; ServerConnectionImpl server_; @@ -236,8 +255,30 @@ TEST_P(Http2CodecImplTest, DeferredReset) { server_wrapper_.dispatch(Buffer::OwnedImpl(), server_); } -INSTANTIATE_TEST_CASE_P(Http2CodecImplTest, Http2CodecImplTest, - testing::Values(0, CodecOptions::NoCompression)); +// we seperate default/edge cases here to avoid combinatorial explosion + +#define HTTP2SETTINGS_DEFAULT_COMBIME \ + ::testing::Combine(::testing::Values(Http2Settings::DEFAULT_HPACK_TABLE_SIZE), \ + ::testing::Values(Http2Settings::DEFAULT_MAX_CONCURRENT_STREAMS), \ + ::testing::Values(Http2Settings::DEFAULT_INITIAL_STREAM_WINDOW_SIZE), \ + ::testing::Values(Http2Settings::DEFAULT_INITIAL_CONNECTION_WINDOW_SIZE)) + +INSTANTIATE_TEST_CASE_P(Http2CodecImplTestDefaultSettings, Http2CodecImplTest, + ::testing::Combine(HTTP2SETTINGS_DEFAULT_COMBIME, + HTTP2SETTINGS_DEFAULT_COMBIME)); + +#define HTTP2SETTINGS_EDGE_COMBIME \ + ::testing::Combine( \ + ::testing::Values(Http2Settings::MIN_HPACK_TABLE_SIZE, Http2Settings::MAX_HPACK_TABLE_SIZE), \ + ::testing::Values(Http2Settings::MIN_MAX_CONCURRENT_STREAMS, \ + Http2Settings::MAX_MAX_CONCURRENT_STREAMS), \ + ::testing::Values(Http2Settings::MIN_INITIAL_STREAM_WINDOW_SIZE, \ + Http2Settings::MAX_INITIAL_STREAM_WINDOW_SIZE), \ + ::testing::Values(Http2Settings::MIN_INITIAL_CONNECTION_WINDOW_SIZE, \ + Http2Settings::MAX_INITIAL_CONNECTION_WINDOW_SIZE)) + +INSTANTIATE_TEST_CASE_P(Http2CodecImplTestEdgeSettings, Http2CodecImplTest, + ::testing::Combine(HTTP2SETTINGS_EDGE_COMBIME, HTTP2SETTINGS_EDGE_COMBIME)); TEST(Http2CodecUtility, reconstituteCrumbledCookies) { { diff --git a/test/common/http/utility_test.cc b/test/common/http/utility_test.cc index 19fb677241897..b70152abbab43 100644 --- a/test/common/http/utility_test.cc +++ b/test/common/http/utility_test.cc @@ -15,9 +15,6 @@ namespace Envoy { namespace Http { -// Satisfy linker -const uint64_t CodecOptions::NoCompression; - TEST(HttpUtility, parseQueryString) { EXPECT_EQ(Utility::QueryParams(), Utility::parseQueryString("/hello")); EXPECT_EQ(Utility::QueryParams(), Utility::parseQueryString("/hello?")); @@ -90,21 +87,62 @@ TEST(HttpUtility, createSslRedirectPath) { } } -TEST(HttpUtility, parseCodecOptions) { +TEST(HttpUtility, parseHttp2Settings) { + { + auto http2_settings = Utility::parseHttp2Settings(*Json::Factory::loadFromString("{}")); + EXPECT_EQ(Http2Settings::DEFAULT_HPACK_TABLE_SIZE, http2_settings.hpack_table_size_); + EXPECT_EQ(Http2Settings::DEFAULT_MAX_CONCURRENT_STREAMS, + http2_settings.max_concurrent_streams_); + EXPECT_EQ(Http2Settings::DEFAULT_INITIAL_STREAM_WINDOW_SIZE, + http2_settings.initial_stream_window_size_); + EXPECT_EQ(Http2Settings::DEFAULT_INITIAL_CONNECTION_WINDOW_SIZE, + http2_settings.initial_connection_window_size_); + } + + { + auto http2_settings = Utility::parseHttp2Settings(*Json::Factory::loadFromString(R"raw({ + "http2_settings" : { + "hpack_table_size": 1, + "max_concurrent_streams": 2, + "initial_stream_window_size": 3, + "initial_connection_window_size": 4 + } + })raw")); + EXPECT_EQ(1U, http2_settings.hpack_table_size_); + EXPECT_EQ(2U, http2_settings.max_concurrent_streams_); + EXPECT_EQ(3U, http2_settings.initial_stream_window_size_); + EXPECT_EQ(4U, http2_settings.initial_connection_window_size_); + } + { - Json::ObjectSharedPtr json = Json::Factory::loadFromString("{}"); - EXPECT_EQ(0UL, Utility::parseCodecOptions(*json)); + auto http2_settings = Utility::parseHttp2Settings(*Json::Factory::loadFromString(R"raw({ + "http_codec_options": "no_compression" + })raw")); + EXPECT_EQ(0, http2_settings.hpack_table_size_); + EXPECT_EQ(Http2Settings::DEFAULT_MAX_CONCURRENT_STREAMS, + http2_settings.max_concurrent_streams_); + EXPECT_EQ(Http2Settings::DEFAULT_INITIAL_STREAM_WINDOW_SIZE, + http2_settings.initial_stream_window_size_); + EXPECT_EQ(Http2Settings::DEFAULT_INITIAL_CONNECTION_WINDOW_SIZE, + http2_settings.initial_connection_window_size_); } { - Json::ObjectSharedPtr json = - Json::Factory::loadFromString("{\"http_codec_options\": \"no_compression\"}"); - EXPECT_EQ(CodecOptions::NoCompression, Utility::parseCodecOptions(*json)); + auto json = Json::Factory::loadFromString("{\"http_codec_options\": \"foo\"}"); + EXPECT_THROW_WITH_MESSAGE(Utility::parseHttp2Settings(*json), EnvoyException, + "unknown http codec option 'foo'"); } { - Json::ObjectSharedPtr json = Json::Factory::loadFromString("{\"http_codec_options\": \"foo\"}"); - EXPECT_THROW(Utility::parseCodecOptions(*json), EnvoyException); + auto json = Json::Factory::loadFromString(R"raw({ + "http_codec_options": "no_compression", + "http2_settings" : { + "hpack_table_size": 1 + } + })raw"); + EXPECT_THROW_WITH_MESSAGE( + Utility::parseHttp2Settings(*json), EnvoyException, + "'http_codec_options.no_compression' conflicts with 'http2_settings.hpack_table_size'"); } } diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index 0e5e42afab35c..e7186685f6da6 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -25,6 +25,7 @@ #include "gtest/gtest.h" namespace Envoy { + using testing::_; using testing::ContainerEq; using testing::Invoke; @@ -184,8 +185,7 @@ TEST(StrictDnsClusterImplTest, Basic) { EXPECT_EQ(3U, cluster.info()->resourceManager(ResourcePriority::High).requests().max()); EXPECT_EQ(4U, cluster.info()->resourceManager(ResourcePriority::High).retries().max()); EXPECT_EQ(3U, cluster.info()->maxRequestsPerConnection()); - EXPECT_EQ(static_cast(Http::CodecOptions::NoCompression), - cluster.info()->httpCodecOptions()); + EXPECT_EQ(0U, cluster.info()->http2Settings().hpack_table_size_); cluster.info()->stats().upstream_rq_total_.inc(); EXPECT_EQ(1UL, stats.counter("cluster.name.upstream_rq_total").value()); @@ -467,7 +467,8 @@ TEST(StaticClusterImplTest, UrlConfig) { EXPECT_EQ(1024U, cluster.info()->resourceManager(ResourcePriority::High).requests().max()); EXPECT_EQ(3U, cluster.info()->resourceManager(ResourcePriority::High).retries().max()); EXPECT_EQ(0U, cluster.info()->maxRequestsPerConnection()); - EXPECT_EQ(0U, cluster.info()->httpCodecOptions()); + EXPECT_EQ(Http::Http2Settings::DEFAULT_HPACK_TABLE_SIZE, + cluster.info()->http2Settings().hpack_table_size_); EXPECT_EQ(LoadBalancerType::Random, cluster.info()->lbType()); EXPECT_THAT(std::list({"10.0.0.1:11001", "10.0.0.2:11002"}), ContainerEq(hostListToAddresses(cluster.hosts()))); diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index 9ee1b2596c1b1..1ca2014e579c8 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -131,7 +131,8 @@ FakeHttpConnection::FakeHttpConnection(QueuedConnectionWrapperPtr connection_wra if (type == Type::HTTP1) { codec_.reset(new Http::Http1::ServerConnectionImpl(connection_, *this)); } else { - codec_.reset(new Http::Http2::ServerConnectionImpl(connection_, *this, store, 0)); + codec_.reset( + new Http::Http2::ServerConnectionImpl(connection_, *this, store, Http::Http2Settings())); ASSERT(type == Type::HTTP2); } diff --git a/test/mocks/upstream/cluster_info.h b/test/mocks/upstream/cluster_info.h index cf37107265ea1..c2cf7977da78a 100644 --- a/test/mocks/upstream/cluster_info.h +++ b/test/mocks/upstream/cluster_info.h @@ -28,7 +28,7 @@ class MockClusterInfo : public ClusterInfo { MOCK_CONST_METHOD0(connectTimeout, std::chrono::milliseconds()); MOCK_CONST_METHOD0(perConnectionBufferLimitBytes, uint32_t()); MOCK_CONST_METHOD0(features, uint64_t()); - MOCK_CONST_METHOD0(httpCodecOptions, uint64_t()); + MOCK_CONST_METHOD0(http2Settings, const Http::Http2Settings&()); MOCK_CONST_METHOD0(lbType, LoadBalancerType()); MOCK_CONST_METHOD0(maintenanceMode, bool()); MOCK_CONST_METHOD0(maxRequestsPerConnection, uint64_t()); @@ -39,6 +39,7 @@ class MockClusterInfo : public ClusterInfo { MOCK_CONST_METHOD0(statsScope, Stats::Scope&()); std::string name_{"fake_cluster"}; + Http::Http2Settings http2_settings_{}; uint64_t max_requests_per_connection_{}; NiceMock stats_store_; ClusterStats stats_; diff --git a/test/mocks/upstream/mocks.cc b/test/mocks/upstream/mocks.cc index dc5843503c0ae..aea9804e32f65 100644 --- a/test/mocks/upstream/mocks.cc +++ b/test/mocks/upstream/mocks.cc @@ -61,6 +61,7 @@ MockClusterInfo::MockClusterInfo() ON_CALL(*this, connectTimeout()).WillByDefault(Return(std::chrono::milliseconds(1))); ON_CALL(*this, name()).WillByDefault(ReturnRef(name_)); + ON_CALL(*this, http2Settings()).WillByDefault(ReturnRef(http2_settings_)); ON_CALL(*this, maxRequestsPerConnection()) .WillByDefault(ReturnPointee(&max_requests_per_connection_)); ON_CALL(*this, stats()).WillByDefault(ReturnRef(stats_)); diff --git a/test/test_common/BUILD b/test/test_common/BUILD index f66fb67d466b8..bfd363cae7fd2 100644 --- a/test/test_common/BUILD +++ b/test/test_common/BUILD @@ -59,6 +59,7 @@ envoy_cc_test_library( hdrs = ["utility.h"], deps = [ "//include/envoy/buffer:buffer_interface", + "//include/envoy/http:codec_interface", "//include/envoy/network:address_interface", "//source/common/common:empty_string", "//source/common/http:header_map_lib", diff --git a/test/test_common/utility.cc b/test/test_common/utility.cc index 020919e1f2c9a..f54a8feff8ee9 100644 --- a/test/test_common/utility.cc +++ b/test/test_common/utility.cc @@ -11,6 +11,7 @@ #include #include "envoy/buffer/buffer.h" +#include "envoy/http/codec.h" #include "common/common/empty_string.h" #include "common/network/address_impl.h" @@ -22,6 +23,7 @@ #include "spdlog/spdlog.h" namespace Envoy { + bool TestUtility::buffersEqual(const Buffer::Instance& lhs, const Buffer::Instance& rhs) { if (lhs.length() != rhs.length()) { return false; @@ -119,6 +121,12 @@ ScopedFdCloser::~ScopedFdCloser() { ::close(fd_); } namespace Http { +// Satisfy linker +const uint32_t Http2Settings::DEFAULT_HPACK_TABLE_SIZE; +const uint32_t Http2Settings::DEFAULT_MAX_CONCURRENT_STREAMS; +const uint32_t Http2Settings::DEFAULT_INITIAL_STREAM_WINDOW_SIZE; +const uint32_t Http2Settings::DEFAULT_INITIAL_CONNECTION_WINDOW_SIZE; + TestHeaderMapImpl::TestHeaderMapImpl() : HeaderMapImpl() {} TestHeaderMapImpl::TestHeaderMapImpl(