router: converting internal_only_headers from list to vector (envoypr…

…oxy#36898) Commit Message: router: converting internal_only_headers from list to vector Additional Description: Converting the use of std::list to std::vector for internal_only_headers. Typically std::vector is better than std::list in many aspects, so changing the non-mutable list to vector makes more sense. Risk Level: low - refactor only Testing: N/A Docs Changes: N/A Release Notes: N/A Platform Specific Features: N/A Signed-off-by: Adi Suissa-Peleg <[email protected]>
fredyw · Oct 30, 2024 · cfa5803 · cfa5803
1 parent eb87014
commit cfa5803
Show file tree

Hide file tree

Showing 8 changed files with 14 additions and 18 deletions.
diff --git a/envoy/router/router.h b/envoy/router/router.h
@@ -3,7 +3,6 @@
 #include <chrono>
 #include <cstdint>
 #include <functional>
-#include <list>
 #include <map>
 #include <memory>
 #include <string>
@@ -1333,7 +1332,7 @@ class CommonConfig {
    * Return a list of headers that will be cleaned from any requests that are not from an internal
    * (RFC1918) source.
    */
-  virtual const std::list<Http::LowerCaseString>& internalOnlyHeaders() const PURE;
+  virtual const std::vector<Http::LowerCaseString>& internalOnlyHeaders() const PURE;
 
   /**
    * @return const std::string the RouteConfiguration name.

diff --git a/source/common/http/conn_manager_utility.cc b/source/common/http/conn_manager_utility.cc
@@ -322,7 +322,7 @@ void ConnectionManagerUtility::sanitizeTEHeader(RequestHeaderMap& request_header
 
 void ConnectionManagerUtility::cleanInternalHeaders(
     RequestHeaderMap& request_headers, bool edge_request,
-    const std::list<Http::LowerCaseString>& internal_only_headers) {
+    const std::vector<Http::LowerCaseString>& internal_only_headers) {
   if (edge_request) {
     // Headers to be stripped from edge requests, i.e. to sanitize so
     // clients can't inject values.

diff --git a/source/common/http/conn_manager_utility.h b/source/common/http/conn_manager_utility.h
@@ -145,7 +145,7 @@ class ConnectionManagerUtility {
                                       ConnectionManagerConfig& config);
   static void sanitizeTEHeader(RequestHeaderMap& request_headers);
   static void cleanInternalHeaders(RequestHeaderMap& request_headers, bool edge_request,
-                                   const std::list<Http::LowerCaseString>& internal_only_headers);
+                                   const std::vector<Http::LowerCaseString>& internal_only_headers);
 };
 
 } // namespace Http

diff --git a/source/common/http/null_route_impl.cc b/source/common/http/null_route_impl.cc
@@ -18,6 +18,6 @@ const NullCommonConfig NullVirtualHost::route_configuration_;
 const std::multimap<std::string, std::string> RouteEntryImpl::opaque_config_;
 const NullPathMatchCriterion RouteEntryImpl::path_match_criterion_;
 const RouteEntryImpl::ConnectConfigOptRef RouteEntryImpl::connect_config_nullopt_;
-const std::list<LowerCaseString> NullCommonConfig::internal_only_headers_;
+const std::vector<LowerCaseString> NullCommonConfig::internal_only_headers_;
 } // namespace Http
 } // namespace Envoy
diff --git a/source/common/http/null_route_impl.h b/source/common/http/null_route_impl.h
@@ -35,7 +35,7 @@ struct NullRateLimitPolicy : public Router::RateLimitPolicy {
 };
 
 struct NullCommonConfig : public Router::CommonConfig {
-  const std::list<LowerCaseString>& internalOnlyHeaders() const override {
+  const std::vector<LowerCaseString>& internalOnlyHeaders() const override {
     return internal_only_headers_;
   }
 
@@ -50,7 +50,7 @@ struct NullCommonConfig : public Router::CommonConfig {
     return Router::DefaultRouteMetadataPack::get().typed_metadata_;
   }
 
-  static const std::list<LowerCaseString> internal_only_headers_;
+  static const std::vector<LowerCaseString> internal_only_headers_;
 };
 
 struct NullVirtualHost : public Router::VirtualHost {

diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h
@@ -3,7 +3,6 @@
 #include <chrono>
 #include <cstdint>
 #include <iterator>
-#include <list>
 #include <map>
 #include <memory>
 #include <regex>
@@ -1611,7 +1610,7 @@ class CommonConfigImpl : public CommonConfig {
   }
 
   // Router::CommonConfig
-  const std::list<Http::LowerCaseString>& internalOnlyHeaders() const override {
+  const std::vector<Http::LowerCaseString>& internalOnlyHeaders() const override {
     return internal_only_headers_;
   }
   const std::string& name() const override { return name_; }
@@ -1635,7 +1634,7 @@ class CommonConfigImpl : public CommonConfig {
   CommonConfigImpl(const envoy::config::route::v3::RouteConfiguration& config,
                    Server::Configuration::ServerFactoryContext& factory_context,
                    ProtobufMessage::ValidationVisitor& validator, absl::Status& creation_status);
-  std::list<Http::LowerCaseString> internal_only_headers_;
+  std::vector<Http::LowerCaseString> internal_only_headers_;
   HeaderParserPtr request_headers_parser_;
   HeaderParserPtr response_headers_parser_;
   const std::string name_;
@@ -1675,7 +1674,7 @@ class ConfigImpl : public Config {
   RouteConstSharedPtr route(const RouteCallback& cb, const Http::RequestHeaderMap& headers,
                             const StreamInfo::StreamInfo& stream_info,
                             uint64_t random_value) const override;
-  const std::list<Http::LowerCaseString>& internalOnlyHeaders() const override {
+  const std::vector<Http::LowerCaseString>& internalOnlyHeaders() const override {
     return shared_config_->internalOnlyHeaders();
   }
   const std::string& name() const override { return shared_config_->name(); }
@@ -1726,7 +1725,7 @@ class NullConfigImpl : public Config {
     return nullptr;
   }
 
-  const std::list<Http::LowerCaseString>& internalOnlyHeaders() const override {
+  const std::vector<Http::LowerCaseString>& internalOnlyHeaders() const override {
     return internal_only_headers_;
   }
 
@@ -1738,7 +1737,7 @@ class NullConfigImpl : public Config {
   const Envoy::Config::TypedMetadata& typedMetadata() const override;
 
 private:
-  std::list<Http::LowerCaseString> internal_only_headers_;
+  std::vector<Http::LowerCaseString> internal_only_headers_;
   const std::string name_;
 };
 

diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc
@@ -1,7 +1,6 @@
 #include <chrono>
 #include <cstdint>
 #include <fstream>
-#include <list>
 #include <map>
 #include <memory>
 #include <string>
@@ -2022,7 +2021,7 @@ TEST_F(RouteMatcherTest, TestAddRemoveResponseHeaders) {
     }
   }
 
-  EXPECT_THAT(std::list<Http::LowerCaseString>{Http::LowerCaseString("x-lyft-user-id")},
+  EXPECT_THAT(std::vector<Http::LowerCaseString>{Http::LowerCaseString("x-lyft-user-id")},
               ContainerEq(config.internalOnlyHeaders()));
 }
 

diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h
@@ -2,7 +2,6 @@
 
 #include <chrono>
 #include <cstdint>
-#include <list>
 #include <map>
 #include <memory>
 #include <string>
@@ -547,7 +546,7 @@ class MockConfig : public Config {
                const Envoy::StreamInfo::StreamInfo&, uint64_t random_value),
               (const));
 
-  MOCK_METHOD(const std::list<Http::LowerCaseString>&, internalOnlyHeaders, (), (const));
+  MOCK_METHOD(const std::vector<Http::LowerCaseString>&, internalOnlyHeaders, (), (const));
   MOCK_METHOD(const std::string&, name, (), (const));
   MOCK_METHOD(bool, usesVhds, (), (const));
   MOCK_METHOD(bool, mostSpecificHeaderMutationsWins, (), (const));
@@ -556,7 +555,7 @@ class MockConfig : public Config {
   MOCK_METHOD(const Envoy::Config::TypedMetadata&, typedMetadata, (), (const));
 
   std::shared_ptr<MockRoute> route_;
-  std::list<Http::LowerCaseString> internal_only_headers_;
+  std::vector<Http::LowerCaseString> internal_only_headers_;
   std::string name_{"fake_config"};
   envoy::config::core::v3::Metadata metadata_;
   MockRouteMetadata typed_metadata_;