alpha matching: ensure that extracted header values are not reused between streams

envoy/matcher/matcher.h

@@ -183,7 +183,7 @@ struct DataInputGetResult {
   // which attempts to look a key up in the map: if we don't have access to the map yet, we return
   // absl::nullopt with NotAvailable. If we have the entire map, but the key doesn't exist in the
   // map, we return absl::nullopt with AllDataAvailable.
-  absl::optional<absl::string_view> data_;
+  absl::optional<std::string> data_;
 
   // For pretty printing.
   friend std::ostream& operator<<(std::ostream& out, const DataInputGetResult& result) {
@@ -209,7 +209,7 @@ template <class DataType> class DataInput {
 public:
   virtual ~DataInput() = default;
 
-  virtual DataInputGetResult get(const DataType& data) PURE;
+  virtual DataInputGetResult get(const DataType& data) const PURE;
 };
 
 template <class DataType> using DataInputPtr = std::unique_ptr<DataInput<DataType>>;
@@ -245,7 +245,7 @@ template <class DataType> class DataInputFactory : public Config::TypedFactory {
 class CommonProtocolInput {
 public:
   virtual ~CommonProtocolInput() = default;
-  virtual absl::optional<absl::string_view> get() PURE;
+  virtual absl::optional<std::string> get() PURE;
 };
 using CommonProtocolInputPtr = std::unique_ptr<CommonProtocolInput>;
 

source/common/http/matching/inputs.h

@@ -22,32 +22,25 @@ class HttpHeadersDataInputBase : public Matcher::DataInput<HttpMatchingData> {
   virtual absl::optional<std::reference_wrapper<const HeaderType>>
   headerMap(const HttpMatchingData& data) const PURE;
 
-  Matcher::DataInputGetResult get(const HttpMatchingData& data) override {
+  Matcher::DataInputGetResult get(const HttpMatchingData& data) const override {
     const auto maybe_headers = headerMap(data);
 
     if (!maybe_headers) {
       return {Matcher::DataInputGetResult::DataAvailability::NotAvailable, absl::nullopt};
     }
 
-    auto header = maybe_headers->get().get(name_);
-    if (header.empty()) {
-      return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable, absl::nullopt};
-    }
+    auto header_string = HeaderUtility::getAllOfHeaderAsString(maybe_headers->get(), name_, ",");
 
-    if (header_as_string_result_) {
+    if (header_string.result()) {
       return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable,
-              header_as_string_result_->result()};
+              std::string(header_string.result().value())};
     }
 
-    header_as_string_result_ = HeaderUtility::getAllOfHeaderAsString(header, ",");
-
-    return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable,
-            header_as_string_result_->result()};
+    return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable, absl::nullopt};
   }
 
 private:
   const LowerCaseString name_;
-  absl::optional<HeaderUtility::GetAllOfHeaderAsStringResult> header_as_string_result_;
 };
 
 /**

source/common/matcher/matcher.h

@@ -170,7 +170,7 @@ template <class DataType> class MatchTreeFactory {
     explicit CommonProtocolInputWrapper(CommonProtocolInputPtr&& common_protocol_input)
         : common_protocol_input_(std::move(common_protocol_input)) {}
 
-    DataInputGetResult get(const DataType&) override {
+    DataInputGetResult get(const DataType&) const override {
       return DataInputGetResult{DataInputGetResult::DataAvailability::AllDataAvailable,
                                 common_protocol_input_->get()};
     }

source/extensions/matching/common_inputs/environment_variable/input.h

@@ -12,7 +12,7 @@ class Input : public Matcher::CommonProtocolInput {
 public:
   explicit Input(absl::optional<std::string>&& value) : storage_(std::move(value)) {}
 
-  absl::optional<absl::string_view> get() override { return storage_; }
+  absl::optional<std::string> get() override { return storage_; }
 
 private:
   const absl::optional<std::string> storage_;

test/common/http/matching/inputs_test.cc

@@ -9,16 +9,29 @@ namespace Envoy {
 namespace Http {
 namespace Matching {
 
-// Provides test coverage for retrieving a header value multiple times from the same input class.
-TEST(HttpHeadersDataInputBase, Idempotence) {
+TEST(HttpHeadersDataInputBase, ReturnValueNotPersistedBetweenCalls) {
   HttpRequestHeadersDataInput input("header");
-
   HttpMatchingDataImpl data;
-  TestRequestHeaderMapImpl request_headers({{"header", "bar"}});
-  data.onRequestHeaders(request_headers);
 
-  EXPECT_EQ(input.get(data).data_, "bar");
-  EXPECT_EQ(input.get(data).data_, "bar");
+  {
+    TestRequestHeaderMapImpl request_headers({{"header", "bar"}});
+    data.onRequestHeaders(request_headers);
+
+    EXPECT_EQ(input.get(data).data_, "bar");
+  }
+
+  {
+    TestRequestHeaderMapImpl request_headers({{"header", "baz"}});
+    data.onRequestHeaders(request_headers);
+    EXPECT_EQ(input.get(data).data_, "baz");
+  }
+
+  TestRequestHeaderMapImpl request_headers({{"not-header", "baz"}});
+  data.onRequestHeaders(request_headers);
+  auto result = input.get(data);
+  EXPECT_EQ(result.data_availability_,
+            Matcher::DataInputGetResult::DataAvailability::AllDataAvailable);
+  EXPECT_EQ(result.data_, absl::nullopt);
 }
 } // namespace Matching
 } // namespace Http

test/common/matcher/test_utility.h

@@ -19,7 +19,7 @@ struct TestData {
 // A CommonProtocolInput that returns the configured value every time.
 struct CommonProtocolTestInput : public CommonProtocolInput {
   explicit CommonProtocolTestInput(const std::string& data) : data_(data) {}
-  absl::optional<absl::string_view> get() override { return data_; }
+  absl::optional<std::string> get() override { return data_; }
 
   const std::string data_;
 };
@@ -48,7 +48,7 @@ class TestCommonProtocolInputFactory : public CommonProtocolInputFactory {
 // A DataInput that returns the configured value every time.
 struct TestInput : public DataInput<TestData> {
   explicit TestInput(DataInputGetResult result) : result_(result) {}
-  DataInputGetResult get(const TestData&) override { return result_; }
+  DataInputGetResult get(const TestData&) const override { return result_; }
 
   DataInputGetResult result_;
 };
@@ -158,7 +158,8 @@ createSingleMatcher(absl::optional<absl::string_view> input,
                     DataInputGetResult::DataAvailability availability =
                         DataInputGetResult::DataAvailability::AllDataAvailable) {
   return std::make_unique<SingleFieldMatcher<TestData>>(
-      std::make_unique<TestInput>(DataInputGetResult{availability, input}),
+      std::make_unique<TestInput>(DataInputGetResult{
+          availability, input ? absl::make_optional(std::string(*input)) : absl::nullopt}),
       std::make_unique<TestMatcher>(predicate));
 }