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

http2: support custom SETTINGS parameters #9964

Merged
merged 34 commits into from
Mar 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
9691ae7
Refactor H2 codec Http2Settings config processing.
AndresGuedez Jan 31, 2020
28cdf4d
Support custom H/2 settings parameters.
AndresGuedez Feb 6, 2020
bad389c
Add Http2ProtocolOptions cluster config test.
AndresGuedez Feb 6, 2020
4b78a16
Cleanup.
AndresGuedez Feb 7, 2020
0f50401
Cleanup.
AndresGuedez Feb 7, 2020
16b8ba7
Fix comment.
AndresGuedez Feb 7, 2020
08ab741
Fix format.
AndresGuedez Feb 11, 2020
b553ef5
Fix docs.
AndresGuedez Feb 13, 2020
fdc7c9b
Refactor SETTINGS frame received callback handling.
AndresGuedez Feb 13, 2020
cc5fe09
Fix format.
AndresGuedez Feb 13, 2020
6def08e
Fix format.
AndresGuedez Feb 13, 2020
1cf9c32
Fix spelling.
AndresGuedez Feb 13, 2020
6bfbfc6
Remove test debug logs.
AndresGuedez Feb 13, 2020
5632a90
Merge remote-tracking branch 'upstream/master' into h2-user-defined-s…
AndresGuedez Feb 18, 2020
b764d34
Regenerate API shadow.
AndresGuedez Feb 18, 2020
9455741
Refactor newly introduced references to Http2Settings.
AndresGuedez Feb 18, 2020
78d3e63
Adjust expected memory usage for cluster memory tests.
AndresGuedez Feb 18, 2020
8b7621c
Kick CI
AndresGuedez Feb 25, 2020
7f18239
Merge remote-tracking branch 'upstream/master' into h2-user-defined-s…
AndresGuedez Feb 25, 2020
07c1814
Merge conflict resolution.
AndresGuedez Feb 27, 2020
8293133
Disallow user defined and named parameter collisions in Http2Protocol…
AndresGuedez Mar 12, 2020
31c56a0
Merge remote-tracking branch 'upstream/master' into h2-user-defined-s…
AndresGuedez Mar 12, 2020
cb35501
Spelling fixes.
AndresGuedez Mar 12, 2020
43b83bf
Simplify custom settings parameters validation logic and minor cleanup.
AndresGuedez Mar 14, 2020
be39274
Minor cleanup.
AndresGuedez Mar 14, 2020
e48de85
Fix spelling.
AndresGuedez Mar 16, 2020
59e2dfe
Fix cluster memory usage expectations.
AndresGuedez Mar 16, 2020
3f513d4
Avoid using deprecated router filter name.
AndresGuedez Mar 16, 2020
53bf245
Change warning to debug log.
AndresGuedez Mar 17, 2020
cf8f7f2
Simplify H/2 custom settings parameters validation logic.
AndresGuedez Mar 18, 2020
ad297e2
Change comment wording.
AndresGuedez Mar 18, 2020
6ae55cd
Minor cleaup.
AndresGuedez Mar 18, 2020
a3dd8bf
Enforce stricter validation of HTTP/2 custom settings parameters.
AndresGuedez Mar 18, 2020
e5b5e53
Fix format.
AndresGuedez Mar 19, 2020
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
43 changes: 42 additions & 1 deletion api/envoy/api/v2/core/protocol.proto
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,21 @@ message Http1ProtocolOptions {
bool enable_trailers = 5;
}

// [#next-free-field: 13]
// [#next-free-field: 14]
message Http2ProtocolOptions {
// Defines a parameter to be sent in the SETTINGS frame.
// See `RFC7540, sec. 6.5.1 <https://tools.ietf.org/html/rfc7540#section-6.5.1>`_ for details.
message SettingsParameter {
// The 16 bit parameter identifier.
google.protobuf.UInt32Value identifier = 1 [
(validate.rules).uint32 = {lte: 65536 gte: 1},
(validate.rules).message = {required: true}
];

// The 32 bit parameter value.
google.protobuf.UInt32Value value = 2 [(validate.rules).message = {required: true}];
mattklein123 marked this conversation as resolved.
Show resolved Hide resolved
}

// `Maximum table size <https://httpwg.org/specs/rfc7541.html#rfc.section.4.2>`_
// (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
Expand Down Expand Up @@ -216,6 +229,34 @@ message Http2ProtocolOptions {
//
// See `RFC7540, sec. 8.1 <https://tools.ietf.org/html/rfc7540#section-8.1>`_ for details.
bool stream_error_on_invalid_http_messaging = 12;

// [#not-implemented-hide:]
// Specifies SETTINGS frame parameters to be sent to the peer, with two exceptions:
//
// 1. SETTINGS_ENABLE_PUSH (0x2) is not configurable as HTTP/2 server push is not supported by
// Envoy.
//
// 2. SETTINGS_ENABLE_CONNECT_PROTOCOL (0x8) is only configurable through the named field
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to suggest "must be consistent" but I think we can deal with this when we get to the deprecation PR and just land this now.

My only concern is API stability issues - I don't think we can land this and change the behavior, and if we want to deprecate allow_connect we can't leave this as-is.

If we flag this "not implemented hide" until we do the deprecation PR (and hash out what long term behavior is optimal) can we land this as-is now and work out details over next week or two as the deprecation PR goes out? If so I'd suggest making it invisible and landing now.

@mattklein123

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I agree that for deprecation we will have to check consistency as previously discussed, but I agree with @alyssawilk that if we hide this new feature for right now we can land this until we figure it out. I have a change blocked behind this one so I would love to get this in and iterate if possible, so that plan SGTM.

/wait

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I've hidden the API.

// 'allow_connect'.
//
// Note that custom parameters specified through this field can not also be set in the
// corresponding named parameters:
//
// .. code-block:: text
//
// ID Field Name
// ----------------
// 0x1 hpack_table_size
// 0x3 max_concurrent_streams
// 0x4 initial_stream_window_size
//
// Collisions will trigger config validation failure on load/update. Likewise, inconsistencies
// between custom parameters with the same identifier will trigger a failure.
//
// See `IANA HTTP/2 Settings
// <https://www.iana.org/assignments/http2-parameters/http2-parameters.xhtml#settings>`_ for
// standardized identifiers.
repeated SettingsParameter custom_settings_parameters = 13;
}

// [#not-implemented-hide:]
Expand Down
46 changes: 45 additions & 1 deletion api/envoy/config/core/v3/protocol.proto
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,27 @@ message Http1ProtocolOptions {
bool enable_trailers = 5;
}

// [#next-free-field: 13]
// [#next-free-field: 14]
message Http2ProtocolOptions {
option (udpa.annotations.versioning).previous_message_type =
"envoy.api.v2.core.Http2ProtocolOptions";

// Defines a parameter to be sent in the SETTINGS frame.
// See `RFC7540, sec. 6.5.1 <https://tools.ietf.org/html/rfc7540#section-6.5.1>`_ for details.
message SettingsParameter {
option (udpa.annotations.versioning).previous_message_type =
"envoy.api.v2.core.Http2ProtocolOptions.SettingsParameter";

// The 16 bit parameter identifier.
google.protobuf.UInt32Value identifier = 1 [
(validate.rules).uint32 = {lte: 65536 gte: 1},
(validate.rules).message = {required: true}
];

// The 32 bit parameter value.
google.protobuf.UInt32Value value = 2 [(validate.rules).message = {required: true}];
}

// `Maximum table size <https://httpwg.org/specs/rfc7541.html#rfc.section.4.2>`_
// (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
Expand Down Expand Up @@ -235,6 +251,34 @@ message Http2ProtocolOptions {
//
// See `RFC7540, sec. 8.1 <https://tools.ietf.org/html/rfc7540#section-8.1>`_ for details.
bool stream_error_on_invalid_http_messaging = 12;

// [#not-implemented-hide:]
// Specifies SETTINGS frame parameters to be sent to the peer, with two exceptions:
//
// 1. SETTINGS_ENABLE_PUSH (0x2) is not configurable as HTTP/2 server push is not supported by
// Envoy.
//
// 2. SETTINGS_ENABLE_CONNECT_PROTOCOL (0x8) is only configurable through the named field
// 'allow_connect'.
//
// Note that custom parameters specified through this field can not also be set in the
// corresponding named parameters:
//
// .. code-block:: text
//
// ID Field Name
// ----------------
// 0x1 hpack_table_size
// 0x3 max_concurrent_streams
// 0x4 initial_stream_window_size
//
// Collisions will trigger config validation failure on load/update. Likewise, inconsistencies
// between custom parameters with the same identifier will trigger a failure.
//
// See `IANA HTTP/2 Settings
// <https://www.iana.org/assignments/http2-parameters/http2-parameters.xhtml#settings>`_ for
// standardized identifiers.
repeated SettingsParameter custom_settings_parameters = 13;
}

// [#not-implemented-hide:]
Expand Down
5 changes: 5 additions & 0 deletions bazel/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,11 @@ def _com_google_absl():
actual = "@com_google_absl//absl/time:time",
)

native.bind(
name = "abseil_algorithm",
actual = "@com_google_absl//absl/algorithm:algorithm",
)

def _com_google_protobuf():
_repository_impl("rules_python")
_repository_impl(
Expand Down
43 changes: 42 additions & 1 deletion generated_api_shadow/envoy/api/v2/core/protocol.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

46 changes: 45 additions & 1 deletion generated_api_shadow/envoy/config/core/v3/protocol.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

72 changes: 0 additions & 72 deletions include/envoy/http/codec.h
Original file line number Diff line number Diff line change
Expand Up @@ -326,78 +326,6 @@ struct Http1Settings {
HeaderKeyFormat header_key_format_{HeaderKeyFormat::Default};
};

/**
* HTTP/2 codec settings
*/
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};
bool allow_connect_{DEFAULT_ALLOW_CONNECT};
bool allow_metadata_{DEFAULT_ALLOW_METADATA};
bool stream_error_on_invalid_http_messaging_{DEFAULT_STREAM_ERROR_ON_INVALID_HTTP_MESSAGING};
uint32_t max_outbound_frames_{DEFAULT_MAX_OUTBOUND_FRAMES};
uint32_t max_outbound_control_frames_{DEFAULT_MAX_OUTBOUND_CONTROL_FRAMES};
uint32_t max_consecutive_inbound_frames_with_empty_payload_{
DEFAULT_MAX_CONSECUTIVE_INBOUND_FRAMES_WITH_EMPTY_PAYLOAD};
uint32_t max_inbound_priority_frames_per_stream_{DEFAULT_MAX_INBOUND_PRIORITY_FRAMES_PER_STREAM};
uint32_t max_inbound_window_update_frames_per_data_frame_sent_{
DEFAULT_MAX_INBOUND_WINDOW_UPDATE_FRAMES_PER_DATA_FRAME_SENT};

// 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 = std::numeric_limits<uint32_t>::max();

// 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;
// By default both nghttp2 and Envoy do not allow CONNECT over H2.
static const bool DEFAULT_ALLOW_CONNECT = false;
// By default Envoy does not allow METADATA support.
static const bool DEFAULT_ALLOW_METADATA = false;
// By default Envoy does not allow invalid headers.
static const bool DEFAULT_STREAM_ERROR_ON_INVALID_HTTP_MESSAGING = false;

// Default limit on the number of outbound frames of all types.
static const uint32_t DEFAULT_MAX_OUTBOUND_FRAMES = 10000;
// Default limit on the number of outbound frames of types PING, SETTINGS and RST_STREAM.
static const uint32_t DEFAULT_MAX_OUTBOUND_CONTROL_FRAMES = 1000;
// Default limit on the number of consecutive inbound frames with an empty payload
// and no end stream flag.
static const uint32_t DEFAULT_MAX_CONSECUTIVE_INBOUND_FRAMES_WITH_EMPTY_PAYLOAD = 1;
// Default limit on the number of inbound frames of type PRIORITY (per stream).
static const uint32_t DEFAULT_MAX_INBOUND_PRIORITY_FRAMES_PER_STREAM = 100;
// Default limit on the number of inbound frames of type WINDOW_UPDATE (per DATA frame sent).
static const uint32_t DEFAULT_MAX_INBOUND_WINDOW_UPDATE_FRAMES_PER_DATA_FRAME_SENT = 10;
};

/**
* A connection (client or server) that owns multiple streams.
*/
Expand Down
7 changes: 4 additions & 3 deletions include/envoy/upstream/upstream.h
Original file line number Diff line number Diff line change
Expand Up @@ -720,10 +720,11 @@ class ClusterInfo {
virtual const Http::Http1Settings& http1Settings() const PURE;

/**
* @return const Http::Http2Settings& for HTTP/2 connections created on behalf of this cluster.
* @see Http::Http2Settings.
* @return const envoy::config::core::v3::Http2ProtocolOptions& for HTTP/2 connections
* created on behalf of this cluster.
* @see envoy::config::core::v3::Http2ProtocolOptions.
*/
virtual const Http::Http2Settings& http2Settings() const PURE;
virtual const envoy::config::core::v3::Http2ProtocolOptions& http2Options() const PURE;

/**
* @param name std::string containing the well-known name of the extension for which protocol
Expand Down
1 change: 1 addition & 0 deletions source/common/http/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ envoy_cc_library(
external_deps = [
"abseil_optional",
"http_parser",
"nghttp2",
],
deps = [
":exception_lib",
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 @@ -155,7 +155,7 @@ CodecClientProd::CodecClientProd(Type type, Network::ClientConnectionPtr&& conne
}
case Type::HTTP2: {
codec_ = std::make_unique<Http2::ClientConnectionImpl>(
*connection_, *this, host->cluster().statsScope(), host->cluster().http2Settings(),
*connection_, *this, host->cluster().statsScope(), host->cluster().http2Options(),
Http::DEFAULT_MAX_REQUEST_HEADERS_KB, host->cluster().maxResponseHeadersCount());
break;
}
Expand Down
6 changes: 3 additions & 3 deletions source/common/http/conn_manager_utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ std::string ConnectionManagerUtility::determineNextProtocol(Network::Connection&
ServerConnectionPtr ConnectionManagerUtility::autoCreateCodec(
Network::Connection& connection, const Buffer::Instance& data,
ServerConnectionCallbacks& callbacks, Stats::Scope& scope, const Http1Settings& http1_settings,
const Http2Settings& http2_settings, uint32_t max_request_headers_kb,
uint32_t max_request_headers_count) {
const envoy::config::core::v3::Http2ProtocolOptions& http2_options,
uint32_t max_request_headers_kb, uint32_t max_request_headers_count) {
if (determineNextProtocol(connection, data) == Http2::ALPN_STRING) {
return std::make_unique<Http2::ServerConnectionImpl>(connection, callbacks, scope,
http2_settings, max_request_headers_kb,
http2_options, max_request_headers_kb,
max_request_headers_count);
} else {
return std::make_unique<Http1::ServerConnectionImpl>(connection, scope, callbacks,
Expand Down
3 changes: 2 additions & 1 deletion source/common/http/conn_manager_utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ class ConnectionManagerUtility {
static ServerConnectionPtr
autoCreateCodec(Network::Connection& connection, const Buffer::Instance& data,
ServerConnectionCallbacks& callbacks, Stats::Scope& scope,
const Http1Settings& http1_settings, const Http2Settings& http2_settings,
const Http1Settings& http1_settings,
const envoy::config::core::v3::Http2ProtocolOptions& http2_options,
uint32_t max_request_headers_kb, uint32_t max_request_headers_count);

/**
Expand Down
Loading