Make the limit on HTTP/2 metadata that can be handled configurable (e…

…nvoyproxy#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 <[email protected]>
bazmurphy · Jan 29, 2025 · 5611246 · 5611246
1 parent 67ea3d6
commit 5611246
Show file tree

Hide file tree

Showing 7 changed files with 57 additions and 10 deletions.
diff --git a/api/envoy/config/core/v3/protocol.proto b/api/envoy/config/core/v3/protocol.proto
@@ -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";
@@ -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:]

diff --git a/changelogs/current.yaml b/changelogs/current.yaml
@@ -73,5 +73,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:
diff --git a/source/common/http/http2/codec_impl.cc b/source/common/http/http2/codec_impl.cc
@@ -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_;
 }
@@ -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));
 }
@@ -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) {

diff --git a/source/common/http/http2/codec_impl.h b/source/common/http/http2/codec_impl.h
@@ -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.

diff --git a/source/common/http/http2/metadata_decoder.cc b/source/common/http/http2/metadata_decoder.cc
@@ -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);
 

diff --git a/source/common/http/http2/metadata_decoder.h b/source/common/http/http2/metadata_decoder.h
@@ -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();
 
   /**
@@ -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;
 
@@ -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;
 

diff --git a/test/common/http/http2/metadata_encoder_test.cc b/test/common/http/http2/metadata_encoder_test.cc
@@ -1,3 +1,5 @@
+#include <cstdint>
+
 #include "envoy/http/metadata_interface.h"
 
 #include "source/common/buffer/buffer_impl.h"
@@ -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 {
@@ -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;
@@ -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 = {};
@@ -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);
 }