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

Make the limit on HTTP/2 metadata that can be handled configurable #37372

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion api/envoy/config/core/v3/protocol.proto
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ message KeepaliveSettings {
[(validate.rules).duration = {gte {nanos: 1000000}}];
}

// [#next-free-field: 17]
// [#next-free-field: 18]
message Http2ProtocolOptions {
option (udpa.annotations.versioning).previous_message_type =
"envoy.api.v2.core.Http2ProtocolOptions";
Expand Down Expand Up @@ -633,6 +633,9 @@ message Http2ProtocolOptions {
// If unset, HTTP/2 codec is selected based on envoy.reloadable_features.http2_use_oghttp2.
google.protobuf.BoolValue use_oghttp2_codec = 16
[(xds.annotations.v3.field_status).work_in_progress = true];

// Configure the maximum amount of metadata than can be handled per stream. Defaults to 1 MB.
google.protobuf.UInt64Value max_metadata_size = 17;
}

// [#not-implemented-hide:]
Expand Down
4 changes: 4 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,9 @@ new_features:
change: |
Adding support for a new body mode: FULL_DUPLEX_STREAMED in the ext_proc filter
:ref:`processing_mode <envoy_v3_api_field_extensions.filters.http.ext_proc.v3.ExternalProcessor.processing_mode>`.
- area: http
change: |
Added :ref:`max_metadata_size <envoy_v3_api_field_config.core.v3.Http2ProtocolOptions.max_metadata_size>` to make
HTTP/2 metadata limits configurable.

deprecated:
6 changes: 5 additions & 1 deletion source/common/http/http2/codec_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,7 @@ MetadataDecoder& ConnectionImpl::StreamImpl::getMetadataDecoder() {
auto cb = [this](MetadataMapPtr&& metadata_map_ptr) {
this->onMetadataDecoded(std::move(metadata_map_ptr));
};
metadata_decoder_ = std::make_unique<MetadataDecoder>(cb);
metadata_decoder_ = std::make_unique<MetadataDecoder>(cb, parent_.max_metadata_size_);
}
return *metadata_decoder_;
}
Expand Down Expand Up @@ -2139,6 +2139,8 @@ ClientConnectionImpl::ClientConnectionImpl(
}
http2_session_factory.init(base(), http2_options);
allow_metadata_ = http2_options.allow_metadata();
max_metadata_size_ =
PROTOBUF_GET_WRAPPED_OR_DEFAULT(http2_options, max_metadata_size, 1024 * 1024);
idle_session_requires_ping_interval_ = std::chrono::milliseconds(PROTOBUF_GET_MS_OR_DEFAULT(
http2_options.connection_keepalive(), connection_idle_interval, 0));
}
Expand Down Expand Up @@ -2223,6 +2225,8 @@ ServerConnectionImpl::ServerConnectionImpl(
#endif
sendSettings(http2_options, false);
allow_metadata_ = http2_options.allow_metadata();
max_metadata_size_ =
PROTOBUF_GET_WRAPPED_OR_DEFAULT(http2_options, max_metadata_size, 1024 * 1024);
}

Status ServerConnectionImpl::onBeginHeaders(int32_t stream_id) {
Expand Down
1 change: 1 addition & 0 deletions source/common/http/http2/codec_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,7 @@ class ConnectionImpl : public virtual Connection,
const uint32_t max_headers_count_;
uint32_t per_stream_buffer_limit_;
bool allow_metadata_;
uint64_t max_metadata_size_;
const bool stream_error_on_invalid_http_messaging_;

// Status for any errors encountered by the nghttp2 callbacks.
Expand Down
3 changes: 2 additions & 1 deletion source/common/http/http2/metadata_decoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ struct MetadataDecoder::HpackDecoderContext {
http2::HpackDecoder decoder;
};

MetadataDecoder::MetadataDecoder(MetadataCallback cb) {
MetadataDecoder::MetadataDecoder(MetadataCallback cb, uint64_t max_payload_size_bound)
: max_payload_size_bound_(max_payload_size_bound) {
ASSERT(cb != nullptr);
callback_ = std::move(cb);

Expand Down
5 changes: 3 additions & 2 deletions source/common/http/http2/metadata_decoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class MetadataDecoder : Logger::Loggable<Logger::Id::http2> {
* @param cb is the decoder's callback function. The callback function is called when the decoder
* finishes decoding metadata.
*/
MetadataDecoder(MetadataCallback cb);
MetadataDecoder(MetadataCallback cb, uint64_t max_payload_size_bound);
~MetadataDecoder();

/**
Expand Down Expand Up @@ -53,6 +53,7 @@ class MetadataDecoder : Logger::Loggable<Logger::Id::http2> {
friend class MetadataEncoderDecoderTest_VerifyEncoderDecoderMultipleMetadataReachSizeLimit_Test;
friend class MetadataEncoderTest_VerifyEncoderDecoderOnMultipleMetadataMaps_Test;
friend class MetadataEncoderTest_VerifyEncoderDecoderMultipleMetadataReachSizeLimit_Test;
friend class MetadataEncoderTest_VerifyAdjustingMetadataSizeLimit_Test;

struct HpackDecoderContext;

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

// Payload size limit. If the total payload received exceeds the limit, fails the connection.
const uint64_t max_payload_size_bound_ = 1024 * 1024;
const uint64_t max_payload_size_bound_;

uint64_t total_payload_size_ = 0;

Expand Down
43 changes: 38 additions & 5 deletions test/common/http/http2/metadata_encoder_test.cc
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#include <cstdint>

#include "envoy/http/metadata_interface.h"

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

static const uint64_t STREAM_ID = 1;
static constexpr uint64_t kDefaultMaxPayloadSizeBound = 1024 * 1024;

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

class MetadataEncoderTest : public testing::Test {
public:
void initialize(MetadataCallback cb) {
decoder_ = std::make_unique<MetadataDecoder>(cb);
void initialize(MetadataCallback cb,
uint64_t max_payload_size_bound = kDefaultMaxPayloadSizeBound) {
decoder_ = std::make_unique<MetadataDecoder>(cb, max_payload_size_bound);

// Enables extension frame.
nghttp2_option* option;
Expand Down Expand Up @@ -197,6 +201,33 @@ TEST_F(MetadataEncoderTest, VerifyEncoderDecoderMultipleMetadataReachSizeLimit)
EXPECT_LE(decoder_->max_payload_size_bound_, decoder_->total_payload_size_);
}

TEST_F(MetadataEncoderTest, VerifyAdjustingMetadataSizeLimit) {
MetadataMap metadata_map_empty = {};
MetadataCallback cb = [](std::unique_ptr<MetadataMap>) -> void {};
initialize(cb, 10 * kDefaultMaxPayloadSizeBound);

for (int i = 0; i < 1000; i++) {
// Cleans up the output buffer.
memset(output_buffer_.buf, 0, output_buffer_.length);
output_buffer_.length = 0;

MetadataMap metadata_map = {
{"header_key", std::string(10000, 'a')},
};
MetadataMapPtr metadata_map_ptr = std::make_unique<MetadataMap>(metadata_map);
MetadataMapVector metadata_map_vector;
metadata_map_vector.push_back(std::move(metadata_map_ptr));

// Encode and decode the second MetadataMap.
decoder_->callback_ = [this, &metadata_map_vector](MetadataMapPtr&& metadata_map_ptr) -> void {
this->verifyMetadataMapVector(metadata_map_vector, std::move(metadata_map_ptr));
};
submitMetadata(metadata_map_vector);
ASSERT_GT(session_->ProcessBytes(toStringView(output_buffer_.buf, output_buffer_.length)), 0);
}
EXPECT_GT(decoder_->max_payload_size_bound_, decoder_->total_payload_size_);
}

// Tests encoding an empty map.
TEST_F(MetadataEncoderTest, EncodeMetadataMapEmpty) {
MetadataMap empty = {};
Expand Down Expand Up @@ -309,9 +340,11 @@ TEST_F(MetadataEncoderTest, EncodeDecodeFrameTest) {
metadata_map_vector.push_back(std::move(metadataMapPtr));
Http2Frame http2FrameFromUltility = Http2Frame::makeMetadataFrameFromMetadataMap(
1, metadataMap, Http2Frame::MetadataFlags::EndMetadata);
MetadataDecoder decoder([this, &metadata_map_vector](MetadataMapPtr&& metadata_map_ptr) -> void {
this->verifyMetadataMapVector(metadata_map_vector, std::move(metadata_map_ptr));
});
MetadataDecoder decoder(
[this, &metadata_map_vector](MetadataMapPtr&& metadata_map_ptr) -> void {
this->verifyMetadataMapVector(metadata_map_vector, std::move(metadata_map_ptr));
},
kDefaultMaxPayloadSizeBound);
decoder.receiveMetadata(http2FrameFromUltility.data() + 9, http2FrameFromUltility.size() - 9);
decoder.onMetadataFrameComplete(true);
}
Expand Down