Skip to content

Commit 98d153b

Browse files
authored
Make the limit on HTTP/2 metadata that can be handled configurable (envoyproxy#37372)
Make the limit on HTTP/2 metadata configurable. Defaults to the current hardcoded value of 1 MiB. The value can be configured through a new field on `Http2ProtocolOptions` Risk Level: Low Testing: Adds a unit test that covers the new logic in MetadataDecoder Docs Changes: Added to new proto field but kept hidden to be consistent with other H2 metadata configuration --------- Signed-off-by: Jay Dunkelberger <jaydunk@google.com>
1 parent a2a3cda commit 98d153b

File tree

7 files changed

+57
-10
lines changed

7 files changed

+57
-10
lines changed

api/envoy/config/core/v3/protocol.proto

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ message KeepaliveSettings {
449449
[(validate.rules).duration = {gte {nanos: 1000000}}];
450450
}
451451

452-
// [#next-free-field: 17]
452+
// [#next-free-field: 18]
453453
message Http2ProtocolOptions {
454454
option (udpa.annotations.versioning).previous_message_type =
455455
"envoy.api.v2.core.Http2ProtocolOptions";
@@ -633,6 +633,9 @@ message Http2ProtocolOptions {
633633
// If unset, HTTP/2 codec is selected based on envoy.reloadable_features.http2_use_oghttp2.
634634
google.protobuf.BoolValue use_oghttp2_codec = 16
635635
[(xds.annotations.v3.field_status).work_in_progress = true];
636+
637+
// Configure the maximum amount of metadata than can be handled per stream. Defaults to 1 MB.
638+
google.protobuf.UInt64Value max_metadata_size = 17;
636639
}
637640

638641
// [#not-implemented-hide:]

changelogs/current.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,5 +73,9 @@ new_features:
7373
change: |
7474
Adding support for a new body mode: FULL_DUPLEX_STREAMED in the ext_proc filter
7575
:ref:`processing_mode <envoy_v3_api_field_extensions.filters.http.ext_proc.v3.ExternalProcessor.processing_mode>`.
76+
- area: http
77+
change: |
78+
Added :ref:`max_metadata_size <envoy_v3_api_field_config.core.v3.Http2ProtocolOptions.max_metadata_size>` to make
79+
HTTP/2 metadata limits configurable.
7680
7781
deprecated:

source/common/http/http2/codec_impl.cc

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -816,7 +816,7 @@ MetadataDecoder& ConnectionImpl::StreamImpl::getMetadataDecoder() {
816816
auto cb = [this](MetadataMapPtr&& metadata_map_ptr) {
817817
this->onMetadataDecoded(std::move(metadata_map_ptr));
818818
};
819-
metadata_decoder_ = std::make_unique<MetadataDecoder>(cb);
819+
metadata_decoder_ = std::make_unique<MetadataDecoder>(cb, parent_.max_metadata_size_);
820820
}
821821
return *metadata_decoder_;
822822
}
@@ -2139,6 +2139,8 @@ ClientConnectionImpl::ClientConnectionImpl(
21392139
}
21402140
http2_session_factory.init(base(), http2_options);
21412141
allow_metadata_ = http2_options.allow_metadata();
2142+
max_metadata_size_ =
2143+
PROTOBUF_GET_WRAPPED_OR_DEFAULT(http2_options, max_metadata_size, 1024 * 1024);
21422144
idle_session_requires_ping_interval_ = std::chrono::milliseconds(PROTOBUF_GET_MS_OR_DEFAULT(
21432145
http2_options.connection_keepalive(), connection_idle_interval, 0));
21442146
}
@@ -2223,6 +2225,8 @@ ServerConnectionImpl::ServerConnectionImpl(
22232225
#endif
22242226
sendSettings(http2_options, false);
22252227
allow_metadata_ = http2_options.allow_metadata();
2228+
max_metadata_size_ =
2229+
PROTOBUF_GET_WRAPPED_OR_DEFAULT(http2_options, max_metadata_size, 1024 * 1024);
22262230
}
22272231

22282232
Status ServerConnectionImpl::onBeginHeaders(int32_t stream_id) {

source/common/http/http2/codec_impl.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -720,6 +720,7 @@ class ConnectionImpl : public virtual Connection,
720720
const uint32_t max_headers_count_;
721721
uint32_t per_stream_buffer_limit_;
722722
bool allow_metadata_;
723+
uint64_t max_metadata_size_;
723724
const bool stream_error_on_invalid_http_messaging_;
724725

725726
// Status for any errors encountered by the nghttp2 callbacks.

source/common/http/http2/metadata_decoder.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ struct MetadataDecoder::HpackDecoderContext {
4343
http2::HpackDecoder decoder;
4444
};
4545

46-
MetadataDecoder::MetadataDecoder(MetadataCallback cb) {
46+
MetadataDecoder::MetadataDecoder(MetadataCallback cb, uint64_t max_payload_size_bound)
47+
: max_payload_size_bound_(max_payload_size_bound) {
4748
ASSERT(cb != nullptr);
4849
callback_ = std::move(cb);
4950

source/common/http/http2/metadata_decoder.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ class MetadataDecoder : Logger::Loggable<Logger::Id::http2> {
2222
* @param cb is the decoder's callback function. The callback function is called when the decoder
2323
* finishes decoding metadata.
2424
*/
25-
MetadataDecoder(MetadataCallback cb);
25+
MetadataDecoder(MetadataCallback cb, uint64_t max_payload_size_bound);
2626
~MetadataDecoder();
2727

2828
/**
@@ -53,6 +53,7 @@ class MetadataDecoder : Logger::Loggable<Logger::Id::http2> {
5353
friend class MetadataEncoderDecoderTest_VerifyEncoderDecoderMultipleMetadataReachSizeLimit_Test;
5454
friend class MetadataEncoderTest_VerifyEncoderDecoderOnMultipleMetadataMaps_Test;
5555
friend class MetadataEncoderTest_VerifyEncoderDecoderMultipleMetadataReachSizeLimit_Test;
56+
friend class MetadataEncoderTest_VerifyAdjustingMetadataSizeLimit_Test;
5657

5758
struct HpackDecoderContext;
5859

@@ -75,7 +76,7 @@ class MetadataDecoder : Logger::Loggable<Logger::Id::http2> {
7576
Buffer::OwnedImpl payload_;
7677

7778
// Payload size limit. If the total payload received exceeds the limit, fails the connection.
78-
const uint64_t max_payload_size_bound_ = 1024 * 1024;
79+
const uint64_t max_payload_size_bound_;
7980

8081
uint64_t total_payload_size_ = 0;
8182

test/common/http/http2/metadata_encoder_test.cc

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
#include <cstdint>
2+
13
#include "envoy/http/metadata_interface.h"
24

35
#include "source/common/buffer/buffer_impl.h"
@@ -32,6 +34,7 @@ absl::string_view toStringView(uint8_t* data, size_t length) {
3234
}
3335

3436
static const uint64_t STREAM_ID = 1;
37+
static constexpr uint64_t kDefaultMaxPayloadSizeBound = 1024 * 1024;
3538

3639
// The buffer stores data sent by encoder and received by decoder.
3740
struct TestBuffer {
@@ -79,8 +82,9 @@ class MetadataUnpackingVisitor : public http2::adapter::test::MockHttp2Visitor {
7982

8083
class MetadataEncoderTest : public testing::Test {
8184
public:
82-
void initialize(MetadataCallback cb) {
83-
decoder_ = std::make_unique<MetadataDecoder>(cb);
85+
void initialize(MetadataCallback cb,
86+
uint64_t max_payload_size_bound = kDefaultMaxPayloadSizeBound) {
87+
decoder_ = std::make_unique<MetadataDecoder>(cb, max_payload_size_bound);
8488

8589
// Enables extension frame.
8690
nghttp2_option* option;
@@ -197,6 +201,33 @@ TEST_F(MetadataEncoderTest, VerifyEncoderDecoderMultipleMetadataReachSizeLimit)
197201
EXPECT_LE(decoder_->max_payload_size_bound_, decoder_->total_payload_size_);
198202
}
199203

204+
TEST_F(MetadataEncoderTest, VerifyAdjustingMetadataSizeLimit) {
205+
MetadataMap metadata_map_empty = {};
206+
MetadataCallback cb = [](std::unique_ptr<MetadataMap>) -> void {};
207+
initialize(cb, 10 * kDefaultMaxPayloadSizeBound);
208+
209+
for (int i = 0; i < 1000; i++) {
210+
// Cleans up the output buffer.
211+
memset(output_buffer_.buf, 0, output_buffer_.length);
212+
output_buffer_.length = 0;
213+
214+
MetadataMap metadata_map = {
215+
{"header_key", std::string(10000, 'a')},
216+
};
217+
MetadataMapPtr metadata_map_ptr = std::make_unique<MetadataMap>(metadata_map);
218+
MetadataMapVector metadata_map_vector;
219+
metadata_map_vector.push_back(std::move(metadata_map_ptr));
220+
221+
// Encode and decode the second MetadataMap.
222+
decoder_->callback_ = [this, &metadata_map_vector](MetadataMapPtr&& metadata_map_ptr) -> void {
223+
this->verifyMetadataMapVector(metadata_map_vector, std::move(metadata_map_ptr));
224+
};
225+
submitMetadata(metadata_map_vector);
226+
ASSERT_GT(session_->ProcessBytes(toStringView(output_buffer_.buf, output_buffer_.length)), 0);
227+
}
228+
EXPECT_GT(decoder_->max_payload_size_bound_, decoder_->total_payload_size_);
229+
}
230+
200231
// Tests encoding an empty map.
201232
TEST_F(MetadataEncoderTest, EncodeMetadataMapEmpty) {
202233
MetadataMap empty = {};
@@ -309,9 +340,11 @@ TEST_F(MetadataEncoderTest, EncodeDecodeFrameTest) {
309340
metadata_map_vector.push_back(std::move(metadataMapPtr));
310341
Http2Frame http2FrameFromUltility = Http2Frame::makeMetadataFrameFromMetadataMap(
311342
1, metadataMap, Http2Frame::MetadataFlags::EndMetadata);
312-
MetadataDecoder decoder([this, &metadata_map_vector](MetadataMapPtr&& metadata_map_ptr) -> void {
313-
this->verifyMetadataMapVector(metadata_map_vector, std::move(metadata_map_ptr));
314-
});
343+
MetadataDecoder decoder(
344+
[this, &metadata_map_vector](MetadataMapPtr&& metadata_map_ptr) -> void {
345+
this->verifyMetadataMapVector(metadata_map_vector, std::move(metadata_map_ptr));
346+
},
347+
kDefaultMaxPayloadSizeBound);
315348
decoder.receiveMetadata(http2FrameFromUltility.data() + 9, http2FrameFromUltility.size() - 9);
316349
decoder.onMetadataFrameComplete(true);
317350
}

0 commit comments

Comments
 (0)