-
Notifications
You must be signed in to change notification settings - Fork 5.5k
http2: support custom SETTINGS parameters #9964
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
Changes from 33 commits
9691ae7
28cdf4d
bad389c
4b78a16
0f50401
16b8ba7
08ab741
b553ef5
fdc7c9b
cc5fe09
6def08e
1cf9c32
6bfbfc6
5632a90
b764d34
9455741
78d3e63
8b7621c
7f18239
07c1814
8293133
31c56a0
cb35501
43b83bf
be39274
e48de85
59e2dfe
3f513d4
53bf245
cf8f7f2
ad297e2
6ae55cd
a3dd8bf
e5b5e53
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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}]; | ||
| } | ||
|
|
||
| // `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 | ||
|
|
@@ -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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:] | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.