http2: updates CodecImpl to use a more abstract data type than `nghtt…

…p2_settings` internally (#31875)

This is refactoring in preparation for removing the nghttp2_callback API layer.

Commit Message: http2: updates CodecImpl to use a more abstract data type than nghttp2_settings internally
Additional Description:
Risk Level: low; refactoring, no functional change intended
Testing: ran unit and integration tests locally
Docs Changes:
Release Notes:
Platform Specific Features:

Signed-off-by: Biren Roy <birenroy@google.com>

source/common/http/http2/codec_impl.cc

@@ -86,10 +86,11 @@ int reasonToReset(StreamResetReason reason) {
 
 using Http2ResponseCodeDetails = ConstSingleton<Http2ResponseCodeDetailValues>;
 
-ReceivedSettingsImpl::ReceivedSettingsImpl(const nghttp2_settings& settings) {
-  for (uint32_t i = 0; i < settings.niv; ++i) {
-    if (settings.iv[i].settings_id == NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS) {
-      concurrent_stream_limit_ = settings.iv[i].value;
+ReceivedSettingsImpl::ReceivedSettingsImpl(
+    absl::Span<const http2::adapter::Http2Setting> settings) {
+  for (const auto& [id, value] : settings) {
+    if (id == NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS) {
+      concurrent_stream_limit_ = value;
       break;
     }
   }
@@ -1134,7 +1135,13 @@ Status ConnectionImpl::onFrameReceived(const nghttp2_frame* frame) {
   }
 
   if (frame->hd.type == NGHTTP2_SETTINGS && frame->hd.flags == NGHTTP2_FLAG_NONE) {
-    onSettings(frame->settings);
+    std::vector<http2::adapter::Http2Setting> settings;
+    for (const nghttp2_settings_entry& entry :
+         absl::MakeSpan(frame->settings.iv, frame->settings.niv)) {
+      settings.push_back(
+          {static_cast<http2::adapter::Http2SettingsId>(entry.settings_id), entry.value});
+    }
+    onSettings(settings);
   }
 
   StreamImpl* stream = getStreamUnchecked(frame->hd.stream_id);

source/common/http/http2/codec_impl.h

@@ -33,8 +33,10 @@
 #include "source/common/http/utility.h"
 
 #include "absl/types/optional.h"
+#include "absl/types/span.h"
 #include "nghttp2/nghttp2.h"
 #include "quiche/http2/adapter/http2_adapter.h"
+#include "quiche/http2/adapter/http2_protocol.h"
 #include "quiche/http2/adapter/oghttp2_adapter.h"
 
 namespace Envoy {
@@ -50,7 +52,7 @@ constexpr uint64_t H2_FRAME_HEADER_SIZE = 9;
 
 class ReceivedSettingsImpl : public ReceivedSettings {
 public:
-  explicit ReceivedSettingsImpl(const nghttp2_settings& settings);
+  explicit ReceivedSettingsImpl(absl::Span<const http2::adapter::Http2Setting> settings);
 
   // ReceivedSettings
   const absl::optional<uint32_t>& maxConcurrentStreams() const override {
@@ -596,7 +598,7 @@ class ConnectionImpl : public virtual Connection,
   void sendSettingsHelperOld(const envoy::config::core::v3::Http2ProtocolOptions& http2_options,
                              bool disable_push);
   // Callback triggered when the peer's SETTINGS frame is received.
-  virtual void onSettings(const nghttp2_settings& settings) {
+  virtual void onSettings(absl::Span<const http2::adapter::Http2Setting> settings) {
     ReceivedSettingsImpl received_settings(settings);
     callbacks().onSettings(received_settings);
   }

test/common/http/http2/codec_impl_test_util.h

@@ -40,20 +40,17 @@ class TestCodecSettingsProvider {
 protected:
   // Stores SETTINGS parameters contained in |settings_frame| to make them available via
   // getRemoteSettingsParameterValue().
-  void onSettingsFrame(const nghttp2_settings& settings_frame) {
-    for (uint32_t i = 0; i < settings_frame.niv; ++i) {
-      auto result = settings_.insert(
-          std::make_pair(settings_frame.iv[i].settings_id, settings_frame.iv[i].value));
+  void onSettingsFrame(absl::Span<const http2::adapter::Http2Setting> settings) {
+    for (const auto& [id, value] : settings) {
+      auto result = settings_.insert(std::make_pair(id, value));
       // It is possible to have duplicate settings parameters, each new parameter replaces any
       // existing value.
       // https://tools.ietf.org/html/rfc7540#section-6.5
       if (!result.second) {
-        ENVOY_LOG_MISC(debug, "Duplicated settings parameter {} with value {}",
-                       settings_frame.iv[i].settings_id, settings_frame.iv[i].value);
+        ENVOY_LOG_MISC(debug, "Duplicated settings parameter {} with value {}", id, value);
         settings_.erase(result.first);
         // Guaranteed success here.
-        settings_.insert(
-            std::make_pair(settings_frame.iv[i].settings_id, settings_frame.iv[i].value));
+        settings_.insert(std::make_pair(id, value));
       }
     }
   }
@@ -96,7 +93,9 @@ class TestServerConnectionImpl : public TestCodecStatsProvider,
 
 protected:
   // Overrides ServerConnectionImpl::onSettings().
-  void onSettings(const nghttp2_settings& settings) override { onSettingsFrame(settings); }
+  void onSettings(absl::Span<const http2::adapter::Http2Setting> settings) override {
+    onSettingsFrame(settings);
+  }
 
   testing::NiceMock<Random::MockRandomGenerator> random_;
 };
@@ -132,7 +131,9 @@ class TestClientConnectionImpl : public TestCodecStatsProvider,
 
 protected:
   // Overrides ClientConnectionImpl::onSettings().
-  void onSettings(const nghttp2_settings& settings) override { onSettingsFrame(settings); }
+  void onSettings(absl::Span<const http2::adapter::Http2Setting> settings) override {
+    onSettingsFrame(settings);
+  }
 };
 
 } // namespace Http2