From c289263625812af8fa87bcd9b15e8293d0c92436 Mon Sep 17 00:00:00 2001 From: Adi Suissa-Peleg Date: Fri, 22 Nov 2024 21:55:46 +0000 Subject: [PATCH 1/9] string-matcher: introduce OO string-matcher prior to refactor Signed-off-by: Adi Suissa-Peleg --- envoy/common/matchers.h | 6 + source/common/common/matchers.cc | 33 +++++ source/common/common/matchers.h | 189 +++++++++++++++++++++++++ source/common/common/regex.h | 3 + test/common/common/matchers_test.cc | 142 ++++++++++++++++++- test/common/common/regex_test.cc | 11 ++ test/mocks/matcher/mocks.h | 1 + tools/spelling/spelling_dictionary.txt | 1 + 8 files changed, 380 insertions(+), 6 deletions(-) diff --git a/envoy/common/matchers.h b/envoy/common/matchers.h index 4a79d00b97d62..00c70fe6bb45c 100644 --- a/envoy/common/matchers.h +++ b/envoy/common/matchers.h @@ -20,6 +20,12 @@ class StringMatcher { * Return whether a passed string value matches. */ virtual bool match(const absl::string_view value) const PURE; + + /** + * Returns a string representation of the matcher (the contents to be + * matched). + */ + virtual const std::string& stringRepresentation() const PURE; }; using StringMatcherPtr = std::unique_ptr; diff --git a/source/common/common/matchers.cc b/source/common/common/matchers.cc index 45f256e74e1f6..c19673ef2b50d 100644 --- a/source/common/common/matchers.cc +++ b/source/common/common/matchers.cc @@ -224,5 +224,38 @@ StringMatcherPtr getExtensionStringMatcher(const ::xds::core::v3::TypedExtension return factory->createStringMatcher(*message, context); } +template +std::unique_ptr +createStringMatcher(const StringMatcherType& config, + Server::Configuration::CommonFactoryContext& context) { + switch (config.match_pattern_case()) { + case StringMatcherType::MatchPatternCase::kExact: + return std::make_unique(config.exact(), config.ignore_case()); + case StringMatcherType::MatchPatternCase::kPrefix: + return std::make_unique(config.prefix(), config.ignore_case()); + case StringMatcherType::MatchPatternCase::kSuffix: + return std::make_unique(config.suffix(), config.ignore_case()); + case StringMatcherType::MatchPatternCase::kSafeRegex: + if (config.ignore_case()) { + ExceptionUtil::throwEnvoyException("ignore_case has no effect for safe_regex."); + } + return std::make_unique(config.safe_regex(), context); + case StringMatcherType::MatchPatternCase::kContains: + return std::make_unique(config.contains(), config.ignore_case()); + case StringMatcherType::MatchPatternCase::kCustom: + return std::make_unique(config.custom(), context); + default: + PANIC("unexpected"); + } +} + +// Explicit instantiation of the `createStringMatcher` function to allow the +// implementation to be in the cc file instead of the header file. +template std::unique_ptr +createStringMatcher(const envoy::type::matcher::v3::StringMatcher& config, + Server::Configuration::CommonFactoryContext& context); +template std::unique_ptr +createStringMatcher(const xds::type::matcher::v3::StringMatcher& config, + Server::Configuration::CommonFactoryContext& context); } // namespace Matchers } // namespace Envoy diff --git a/source/common/common/matchers.h b/source/common/common/matchers.h index 8c2ce5a76c0e1..b3412369b1cc6 100644 --- a/source/common/common/matchers.h +++ b/source/common/common/matchers.h @@ -82,14 +82,22 @@ class DoubleMatcher : public ValueMatcher { const envoy::type::matcher::v3::DoubleMatcher matcher_; }; +// A StringMatcher that matches all given strings (similar to the regex ".*"). class UniversalStringMatcher : public StringMatcher { public: bool match(absl::string_view) const override { return true; } + + const std::string& stringRepresentation() const override { return EMPTY_STRING; } }; StringMatcherPtr getExtensionStringMatcher(const ::xds::core::v3::TypedExtensionConfig& config, Server::Configuration::CommonFactoryContext& context); +// This class will be replaced by StringMatcherImplBase (see below) and will +// follow cleaner polymorphism and encapsulation principles. The class will be +// removed once all its usages are removed. +// TODO(adisuissa): remove this class once there all uses are replaced by +// StringMatcherImplBase/StringMatcherPtr. template class StringMatcherImpl : public ValueMatcher, public StringMatcher { public: @@ -173,6 +181,10 @@ class StringMatcherImpl : public ValueMatcher, public StringMatcher { return false; } + // TODO(adiuissa): this will be removed once this entire class is replaced by + // StringMatcherImplBase. + const std::string& stringRepresentation() const override { return EMPTY_STRING; } + private: StringMatcherImpl(absl::string_view exact_match) : matcher_([&]() -> StringMatcherType { @@ -187,6 +199,177 @@ class StringMatcherImpl : public ValueMatcher, public StringMatcher { StringMatcherPtr custom_; }; +// An abstract common implementation for all types of StringMatchers. +class StringMatcherImplBase : public ValueMatcher, public StringMatcher { +public: + StringMatcherImplBase() = default; + virtual ~StringMatcherImplBase() = default; + + // ValueMatcher + bool match(const ProtobufWkt::Value& value) const override { + if (value.kind_case() != ProtobufWkt::Value::kStringValue) { + return false; + } + + return match(value.string_value()); + } + + // StringMatcher + virtual bool match(const absl::string_view value) const override PURE; + + /** + * Helps applications optimize the case where a matcher is a case-sensitive + * prefix-match. + * + * @param prefix the returned prefix string + * @return true if the matcher is a case-sensitive prefix-match. + */ + virtual bool getCaseSensitivePrefixMatch(std::string& prefix) const { + UNREFERENCED_PARAMETER(prefix); + return false; + } +}; + +// A matcher for the `exact` StringMatcher. +class ExactStringMatcher : public StringMatcherImplBase { +public: + ExactStringMatcher(absl::string_view exact, bool ignore_case) + : exact_(exact), ignore_case_(ignore_case) {} + + // StringMatcher + bool match(const absl::string_view value) const override { + return ignore_case_ ? absl::EqualsIgnoreCase(value, exact_) : value == exact_; + } + + const std::string& stringRepresentation() const override { return exact_; } + +private: + const std::string exact_; + const bool ignore_case_; +}; + +// A matcher for the `prefix` StringMatcher. +class PrefixStringMatcher : public StringMatcherImplBase { +public: + PrefixStringMatcher(absl::string_view prefix, bool ignore_case) + : prefix_(prefix), ignore_case_(ignore_case) {} + + // StringMatcher + bool match(const absl::string_view value) const override { + return ignore_case_ ? absl::StartsWithIgnoreCase(value, prefix_) + : absl::StartsWith(value, prefix_); + } + + // StringMatcherImplBase + bool getCaseSensitivePrefixMatch(std::string& prefix) const override { + if (!ignore_case_) { + prefix = prefix_; + return true; + } + return false; + } + + const std::string& stringRepresentation() const override { return prefix_; } + +private: + const std::string prefix_; + const bool ignore_case_; +}; + +// A matcher for the `suffix` StringMatcher. +class SuffixStringMatcher : public StringMatcherImplBase { +public: + SuffixStringMatcher(absl::string_view suffix, bool ignore_case) + : suffix_(suffix), ignore_case_(ignore_case) {} + + // StringMatcher + bool match(const absl::string_view value) const override { + return ignore_case_ ? absl::EndsWithIgnoreCase(value, suffix_) : absl::EndsWith(value, suffix_); + } + + const std::string& stringRepresentation() const override { return suffix_; } + +private: + const std::string suffix_; + const bool ignore_case_; +}; + +// A matcher for the `safe_regex` StringMatcher. +class RegexStringMatcher : public StringMatcherImplBase { +public: + // The RegexMatcher can either be from the ::envoy or ::xds type, + // and the templated c'tor handles both cases. + template + RegexStringMatcher(const RegexMatcherType& safe_regex, + Server::Configuration::CommonFactoryContext& context) + : regex_(THROW_OR_RETURN_VALUE(Regex::Utility::parseRegex(safe_regex, context.regexEngine()), + Regex::CompiledMatcherPtr)) {} + + // StringMatcher + bool match(const absl::string_view value) const override { return regex_->match(value); } + + const std::string& stringRepresentation() const override { + return regex_->stringRepresentation(); + } + +private: + Regex::CompiledMatcherPtr regex_; +}; + +// A matcher for the `contains` StringMatcher. +class ContainsStringMatcher : public StringMatcherImplBase { +public: + ContainsStringMatcher(absl::string_view contents, bool ignore_case) + : contents_(ignore_case ? absl::AsciiStrToLower(contents) : contents), + ignore_case_(ignore_case) {} + + // StringMatcher + bool match(const absl::string_view value) const override { + return ignore_case_ ? absl::StrContains(absl::AsciiStrToLower(value), contents_) + : absl::StrContains(value, contents_); + } + + const std::string& stringRepresentation() const override { + // Note that in case where the matcher is configured to be case-insensitive + // this will return the lower-case configured string (as opposed to the + // other matchers). This is incompatible with the other matchers, but achieves + // the intent of this method. + return contents_; + } + +private: + // If ignore_case is set the contents_ will contain lower-case letters. + const std::string contents_; + const bool ignore_case_; +}; + +// A matcher for the `custom` StringMatcher. +class CustomStringMatcher : public StringMatcherImplBase { +public: + CustomStringMatcher(const xds::core::v3::TypedExtensionConfig& custom, + Server::Configuration::CommonFactoryContext& context) + : custom_(getExtensionStringMatcher(custom, context)) {} + + // StringMatcher + bool match(const absl::string_view value) const override { return custom_->match(value); } + + const std::string& stringRepresentation() const override { + return custom_->stringRepresentation(); + } + +private: + const StringMatcherPtr custom_; +}; + +// Creates a StringMatcher given a config. +// Note that Envoy supports both matchers that are created using the +// `envoy::type::matcher::v3::StringMatcher` type and the +// `xds::type::matcher::v3::StringMatcher` type and therefore this function is templated. +template +std::unique_ptr +createStringMatcher(const StringMatcherType& config, + Server::Configuration::CommonFactoryContext& context); + class StringMatcherExtensionFactory : public Config::TypedFactory { public: virtual StringMatcherPtr @@ -280,6 +463,12 @@ class PathMatcher : public StringMatcher { const StringMatcherImpl& matcher() const { return matcher_; } + // TODO(adisuissa): This method will replace the `matcher()` call above. + const std::string& stringRepresentation() const override { + // TODO(adisuissa): replace with: return matcher_->stringRepresentation(); + // after the type of `matcher_` is replaced. + return EMPTY_STRING; + } private: const StringMatcherImpl matcher_; diff --git a/source/common/common/regex.h b/source/common/common/regex.h index 63b4314d46818..5acfae796831d 100644 --- a/source/common/common/regex.h +++ b/source/common/common/regex.h @@ -38,6 +38,9 @@ class CompiledGoogleReMatcher : public CompiledMatcher { return result; } + // CompiledMatcher + const std::string& stringRepresentation() const override { return regex_.pattern(); } + protected: explicit CompiledGoogleReMatcher(const std::string& regex) : regex_(regex, re2::RE2::Quiet) { ENVOY_BUG(regex_.ok(), "Invalid regex"); diff --git a/test/common/common/matchers_test.cc b/test/common/common/matchers_test.cc index b61689b6b1b23..5b7b04c7a11eb 100644 --- a/test/common/common/matchers_test.cc +++ b/test/common/common/matchers_test.cc @@ -330,7 +330,8 @@ TEST_F(MetadataTest, InvertMatch) { class StringMatcher : public BaseTest {}; -TEST_F(StringMatcher, ExactMatchIgnoreCase) { +// TODO(adisuissa): remove once StringMatcherImpl is deprecated. +TEST_F(StringMatcher, ExactMatchIgnoreCaseOld) { envoy::type::matcher::v3::StringMatcher matcher; matcher.set_exact("exact"); EXPECT_TRUE(Matchers::StringMatcherImpl(matcher, context_).match("exact")); @@ -345,7 +346,8 @@ TEST_F(StringMatcher, ExactMatchIgnoreCase) { EXPECT_FALSE(Matchers::StringMatcherImpl(matcher, context_).match("other")); } -TEST_F(StringMatcher, PrefixMatchIgnoreCase) { +// TODO(adisuissa): remove once StringMatcherImpl is deprecated. +TEST_F(StringMatcher, PrefixMatchIgnoreCaseOld) { envoy::type::matcher::v3::StringMatcher matcher; matcher.set_prefix("prefix"); EXPECT_TRUE(Matchers::StringMatcherImpl(matcher, context_).match("prefix-abc")); @@ -360,7 +362,8 @@ TEST_F(StringMatcher, PrefixMatchIgnoreCase) { EXPECT_FALSE(Matchers::StringMatcherImpl(matcher, context_).match("other")); } -TEST_F(StringMatcher, SuffixMatchIgnoreCase) { +// TODO(adisuissa): remove once StringMatcherImpl is deprecated. +TEST_F(StringMatcher, SuffixMatchIgnoreCaseOld) { envoy::type::matcher::v3::StringMatcher matcher; matcher.set_suffix("suffix"); EXPECT_TRUE(Matchers::StringMatcherImpl(matcher, context_).match("abc-suffix")); @@ -375,7 +378,8 @@ TEST_F(StringMatcher, SuffixMatchIgnoreCase) { EXPECT_FALSE(Matchers::StringMatcherImpl(matcher, context_).match("other")); } -TEST_F(StringMatcher, ContainsMatchIgnoreCase) { +// TODO(adisuissa): remove once StringMatcherImpl is deprecated. +TEST_F(StringMatcher, ContainsMatchIgnoreCaseOld) { envoy::type::matcher::v3::StringMatcher matcher; matcher.set_contains("contained-str"); EXPECT_TRUE(Matchers::StringMatcherImpl(matcher, context_).match("abc-contained-str-def")); @@ -391,7 +395,8 @@ TEST_F(StringMatcher, ContainsMatchIgnoreCase) { EXPECT_FALSE(Matchers::StringMatcherImpl(matcher, context_).match("other")); } -TEST_F(StringMatcher, SafeRegexValue) { +// TODO(adisuissa): remove once StringMatcherImpl is deprecated. +TEST_F(StringMatcher, SafeRegexValueOld) { envoy::type::matcher::v3::StringMatcher matcher; matcher.mutable_safe_regex()->mutable_google_re2(); matcher.mutable_safe_regex()->set_regex("foo.*"); @@ -400,7 +405,8 @@ TEST_F(StringMatcher, SafeRegexValue) { EXPECT_FALSE(Matchers::StringMatcherImpl(matcher, context_).match("bar")); } -TEST_F(StringMatcher, SafeRegexValueIgnoreCase) { +// TODO(adisuissa): remove once StringMatcherImpl is deprecated. +TEST_F(StringMatcher, SafeRegexValueIgnoreCaseOld) { envoy::type::matcher::v3::StringMatcher matcher; matcher.set_ignore_case(true); matcher.mutable_safe_regex()->mutable_google_re2(); @@ -409,6 +415,130 @@ TEST_F(StringMatcher, SafeRegexValueIgnoreCase) { EnvoyException, "ignore_case has no effect for safe_regex."); } +TEST_F(StringMatcher, ExactMatchIgnoreCase) { + envoy::type::matcher::v3::StringMatcher matcher; + matcher.set_exact("exact"); + EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("exact")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("EXACT")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("exacz")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("other")); + + matcher.set_ignore_case(true); + EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("exact")); + EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("EXACT")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("exacz")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("other")); +} + +TEST_F(StringMatcher, PrefixMatchIgnoreCase) { + envoy::type::matcher::v3::StringMatcher matcher; + matcher.set_prefix("prefix"); + EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("prefix-abc")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("PREFIX-ABC")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("prefiz-abc")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("other")); + + matcher.set_ignore_case(true); + EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("prefix-abc")); + EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("PREFIX-ABC")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("prefiz-abc")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("other")); +} + +TEST_F(StringMatcher, SuffixMatchIgnoreCase) { + envoy::type::matcher::v3::StringMatcher matcher; + matcher.set_suffix("suffix"); + EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("abc-suffix")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("ABC-SUFFIX")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("abc-suffiz")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("other")); + + matcher.set_ignore_case(true); + EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("abc-suffix")); + EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("ABC-SUFFIX")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("abc-suffiz")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("other")); +} + +TEST_F(StringMatcher, ContainsMatchIgnoreCase) { + envoy::type::matcher::v3::StringMatcher matcher; + matcher.set_contains("contained-str"); + EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("abc-contained-str-def")); + EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("contained-str")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("ABC-Contained-Str-DEF")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("abc-container-int-def")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("other")); + + matcher.set_ignore_case(true); + EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("abc-contained-str-def")); + EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("abc-cOnTaInEd-str-def")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("abc-ContAineR-str-def")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("other")); +} + +TEST_F(StringMatcher, SafeRegexValue) { + envoy::type::matcher::v3::StringMatcher matcher; + matcher.mutable_safe_regex()->mutable_google_re2(); + matcher.mutable_safe_regex()->set_regex("foo.*"); + EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("foo")); + EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("foobar")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("bar")); +} + +TEST_F(StringMatcher, SafeRegexValueIgnoreCase) { + envoy::type::matcher::v3::StringMatcher matcher; + matcher.set_ignore_case(true); + matcher.mutable_safe_regex()->mutable_google_re2(); + matcher.mutable_safe_regex()->set_regex("foo"); + EXPECT_THROW_WITH_MESSAGE(Matchers::createStringMatcher(matcher, context_)->match("foo"), + EnvoyException, "ignore_case has no effect for safe_regex."); +} + +TEST_F(StringMatcher, ExactMatchIgnoreCaseStringRepresentation) { + envoy::type::matcher::v3::StringMatcher matcher; + matcher.set_exact("eXaCt"); + EXPECT_EQ(Matchers::createStringMatcher(matcher, context_)->stringRepresentation(), "eXaCt"); + + matcher.set_ignore_case(true); + EXPECT_EQ(Matchers::createStringMatcher(matcher, context_)->stringRepresentation(), "eXaCt"); +} + +TEST_F(StringMatcher, PrefixMatchIgnoreCaseStringRepresentation) { + envoy::type::matcher::v3::StringMatcher matcher; + matcher.set_prefix("pReFix"); + EXPECT_EQ(Matchers::createStringMatcher(matcher, context_)->stringRepresentation(), "pReFix"); + + matcher.set_ignore_case(true); + EXPECT_EQ(Matchers::createStringMatcher(matcher, context_)->stringRepresentation(), "pReFix"); +} + +TEST_F(StringMatcher, SuffixMatchIgnoreCaseStringRepresentation) { + envoy::type::matcher::v3::StringMatcher matcher; + matcher.set_suffix("sUfFix"); + EXPECT_EQ(Matchers::createStringMatcher(matcher, context_)->stringRepresentation(), "sUfFix"); + + matcher.set_ignore_case(true); + EXPECT_EQ(Matchers::createStringMatcher(matcher, context_)->stringRepresentation(), "sUfFix"); +} + +TEST_F(StringMatcher, ContainsMatchIgnoreCaseStringRepresentation) { + envoy::type::matcher::v3::StringMatcher matcher; + matcher.set_contains("ConTained-STR"); + EXPECT_EQ(Matchers::createStringMatcher(matcher, context_)->stringRepresentation(), + "ConTained-STR"); + + matcher.set_ignore_case(true); + EXPECT_EQ(Matchers::createStringMatcher(matcher, context_)->stringRepresentation(), + "contained-str"); +} + +TEST_F(StringMatcher, SafeRegexStringRepresentation) { + envoy::type::matcher::v3::StringMatcher matcher; + matcher.mutable_safe_regex()->mutable_google_re2(); + matcher.mutable_safe_regex()->set_regex("fOo.*BaR"); + EXPECT_EQ(Matchers::createStringMatcher(matcher, context_)->stringRepresentation(), "fOo.*BaR"); +} + class PathMatcher : public BaseTest {}; TEST_F(PathMatcher, MatchExactPath) { diff --git a/test/common/common/regex_test.cc b/test/common/common/regex_test.cc index ae117450f245a..8901c02ce0c66 100644 --- a/test/common/common/regex_test.cc +++ b/test/common/common/regex_test.cc @@ -140,6 +140,17 @@ TEST(Utility, ParseRegex) { } } +// Validates that the returned string representation matches the pattern. +TEST(Utility, CompiledMatcherStringRepresentation) { + Regex::GoogleReEngine engine; + + envoy::type::matcher::v3::RegexMatcher matcher; + matcher.set_regex("/aS?df/.*"); + matcher.mutable_google_re2(); + const CompiledMatcherPtr re_matcher = Utility::parseRegex(matcher, engine); + EXPECT_EQ(re_matcher->stringRepresentation(), "/aS?df/.*"); +} + } // namespace } // namespace Regex } // namespace Envoy diff --git a/test/mocks/matcher/mocks.h b/test/mocks/matcher/mocks.h index 46483563ed62c..7c4d9fa8dccd0 100644 --- a/test/mocks/matcher/mocks.h +++ b/test/mocks/matcher/mocks.h @@ -24,6 +24,7 @@ namespace Matchers { class MockStringMatcher : public StringMatcher { public: MOCK_METHOD(bool, match, (absl::string_view), (const, override)); + MOCK_METHOD(const std::string&, stringRepresentation, (), (const, override)); }; } // namespace Matchers } // namespace Envoy diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt index b4269825ba5f7..702c1d91aaade 100644 --- a/tools/spelling/spelling_dictionary.txt +++ b/tools/spelling/spelling_dictionary.txt @@ -1098,6 +1098,7 @@ plaintext pluggable pointee poller +polymorphism popen pos posix From 26de986c22d88a248dce80725f57af789ebdb504 Mon Sep 17 00:00:00 2001 From: Adi Suissa-Peleg Date: Mon, 25 Nov 2024 15:45:02 +0000 Subject: [PATCH 2/9] fix hyperscan Signed-off-by: Adi Suissa-Peleg --- contrib/hyperscan/matching/input_matchers/source/matcher.h | 4 ++++ .../hyperscan/matching/input_matchers/test/matcher_test.cc | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/contrib/hyperscan/matching/input_matchers/source/matcher.h b/contrib/hyperscan/matching/input_matchers/source/matcher.h index d49743ae0c5da..05a31080bf61a 100644 --- a/contrib/hyperscan/matching/input_matchers/source/matcher.h +++ b/contrib/hyperscan/matching/input_matchers/source/matcher.h @@ -44,6 +44,10 @@ class Matcher : public Envoy::Regex::CompiledMatcher, public Envoy::Matcher::Inp // Envoy::Matcher::InputMatcher bool match(const ::Envoy::Matcher::MatchingDataType& input) override; + const std::string& stringRepresentation() const override { + CONSTRUCT_ON_FIRST_USE(std::string, "Optimized for HyperScan"); + } + private: hs_database_t* database_{}; hs_database_t* start_of_match_database_{}; diff --git a/contrib/hyperscan/matching/input_matchers/test/matcher_test.cc b/contrib/hyperscan/matching/input_matchers/test/matcher_test.cc index dcd2e9667753c..b36a57a464986 100644 --- a/contrib/hyperscan/matching/input_matchers/test/matcher_test.cc +++ b/contrib/hyperscan/matching/input_matchers/test/matcher_test.cc @@ -190,6 +190,12 @@ TEST_F(MatcherTest, ReplaceAll) { EXPECT_EQ(matcher_->replaceAll("yabba dabba doo", "d"), "yada dada doo"); } +TEST_F(MatcherTest, StringRepresentation) { + setup("b+", 0, true); + + EXPECT_EQ(matcher_->stringRepresentation(), "Optimized for HyperScan"); +} + } // namespace Hyperscan } // namespace InputMatchers } // namespace Matching From 664d3f45a79a091319b2af8ee6fb9254cb1bc730 Mon Sep 17 00:00:00 2001 From: Adi Suissa-Peleg Date: Mon, 25 Nov 2024 20:27:54 +0000 Subject: [PATCH 3/9] lua Signed-off-by: Adi Suissa-Peleg --- source/extensions/string_matcher/lua/match.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/source/extensions/string_matcher/lua/match.h b/source/extensions/string_matcher/lua/match.h index 436ea36c270ba..830395e3bcc42 100644 --- a/source/extensions/string_matcher/lua/match.h +++ b/source/extensions/string_matcher/lua/match.h @@ -22,6 +22,10 @@ class LuaStringMatcher : public Matchers::StringMatcher, public ThreadLocal::Thr // Matchers::StringMatcher bool match(const absl::string_view value) const override; + const std::string& stringRepresentation() const override { + CONSTRUCT_ON_FIRST_USE(std::string, "Optimized for LuaStringMatcher"); + } + private: CSmartPtr state_; int matcher_func_ref_{LUA_NOREF}; From 195538839a3cb7039c21a57cc3332c8314f1daba Mon Sep 17 00:00:00 2001 From: Adi Suissa-Peleg Date: Mon, 25 Nov 2024 22:24:28 +0000 Subject: [PATCH 4/9] lua2 Signed-off-by: Adi Suissa-Peleg --- source/extensions/string_matcher/lua/match.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/source/extensions/string_matcher/lua/match.cc b/source/extensions/string_matcher/lua/match.cc index 58a26430f8f2e..a70cbc8e2998a 100644 --- a/source/extensions/string_matcher/lua/match.cc +++ b/source/extensions/string_matcher/lua/match.cc @@ -89,6 +89,9 @@ class LuaStringMatcherThreadWrapper : public Matchers::StringMatcher { } bool match(const absl::string_view value) const override { return (*tls_slot_)->match(value); } + const std::string& stringRepresentation() const override { + return (*tls_slot_)->stringRepresentation(); + } private: ThreadLocal::TypedSlotPtr tls_slot_; From 4709928531dba581bfdc75e33765c5451aa85837 Mon Sep 17 00:00:00 2001 From: Adi Suissa-Peleg Date: Tue, 26 Nov 2024 14:39:35 +0000 Subject: [PATCH 5/9] fix regex test Signed-off-by: Adi Suissa-Peleg --- test/common/common/regex_test.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/common/common/regex_test.cc b/test/common/common/regex_test.cc index 8901c02ce0c66..71479467611bf 100644 --- a/test/common/common/regex_test.cc +++ b/test/common/common/regex_test.cc @@ -147,8 +147,9 @@ TEST(Utility, CompiledMatcherStringRepresentation) { envoy::type::matcher::v3::RegexMatcher matcher; matcher.set_regex("/aS?df/.*"); matcher.mutable_google_re2(); - const CompiledMatcherPtr re_matcher = Utility::parseRegex(matcher, engine); - EXPECT_EQ(re_matcher->stringRepresentation(), "/aS?df/.*"); + const absl::StatusOr re_matcher = Utility::parseRegex(matcher, engine); + EXPECT_TRUE(re_matcher.status().ok()); + EXPECT_EQ(re_matcher.value()->stringRepresentation(), "/aS?df/.*"); } } // namespace From b5d75b9202fa1a6aec96c7566f31bb2d9a373b9b Mon Sep 17 00:00:00 2001 From: Adi Suissa-Peleg Date: Tue, 26 Nov 2024 21:48:37 +0000 Subject: [PATCH 6/9] moving stringRepresentation out of the interface Signed-off-by: Adi Suissa-Peleg --- .../matching/input_matchers/source/matcher.h | 4 ---- .../input_matchers/test/matcher_test.cc | 6 ------ envoy/common/matchers.h | 6 ------ envoy/common/regex.h | 6 ++++++ source/common/common/matchers.h | 20 +++++++++---------- source/extensions/string_matcher/lua/match.cc | 3 --- source/extensions/string_matcher/lua/match.h | 4 ---- test/mocks/matcher/mocks.h | 1 - 8 files changed, 15 insertions(+), 35 deletions(-) diff --git a/contrib/hyperscan/matching/input_matchers/source/matcher.h b/contrib/hyperscan/matching/input_matchers/source/matcher.h index 05a31080bf61a..d49743ae0c5da 100644 --- a/contrib/hyperscan/matching/input_matchers/source/matcher.h +++ b/contrib/hyperscan/matching/input_matchers/source/matcher.h @@ -44,10 +44,6 @@ class Matcher : public Envoy::Regex::CompiledMatcher, public Envoy::Matcher::Inp // Envoy::Matcher::InputMatcher bool match(const ::Envoy::Matcher::MatchingDataType& input) override; - const std::string& stringRepresentation() const override { - CONSTRUCT_ON_FIRST_USE(std::string, "Optimized for HyperScan"); - } - private: hs_database_t* database_{}; hs_database_t* start_of_match_database_{}; diff --git a/contrib/hyperscan/matching/input_matchers/test/matcher_test.cc b/contrib/hyperscan/matching/input_matchers/test/matcher_test.cc index b36a57a464986..dcd2e9667753c 100644 --- a/contrib/hyperscan/matching/input_matchers/test/matcher_test.cc +++ b/contrib/hyperscan/matching/input_matchers/test/matcher_test.cc @@ -190,12 +190,6 @@ TEST_F(MatcherTest, ReplaceAll) { EXPECT_EQ(matcher_->replaceAll("yabba dabba doo", "d"), "yada dada doo"); } -TEST_F(MatcherTest, StringRepresentation) { - setup("b+", 0, true); - - EXPECT_EQ(matcher_->stringRepresentation(), "Optimized for HyperScan"); -} - } // namespace Hyperscan } // namespace InputMatchers } // namespace Matching diff --git a/envoy/common/matchers.h b/envoy/common/matchers.h index 00c70fe6bb45c..4a79d00b97d62 100644 --- a/envoy/common/matchers.h +++ b/envoy/common/matchers.h @@ -20,12 +20,6 @@ class StringMatcher { * Return whether a passed string value matches. */ virtual bool match(const absl::string_view value) const PURE; - - /** - * Returns a string representation of the matcher (the contents to be - * matched). - */ - virtual const std::string& stringRepresentation() const PURE; }; using StringMatcherPtr = std::unique_ptr; diff --git a/envoy/common/regex.h b/envoy/common/regex.h index b5261476708d7..84d3c98cbc035 100644 --- a/envoy/common/regex.h +++ b/envoy/common/regex.h @@ -22,6 +22,12 @@ class CompiledMatcher : public Matchers::StringMatcher { */ virtual std::string replaceAll(absl::string_view value, absl::string_view substitution) const PURE; + + /** + * Returns a string representation of the Regex matcher (the pattern to be + * matched). + */ + virtual const std::string& stringRepresentation() const PURE; }; using CompiledMatcherPtr = std::unique_ptr; diff --git a/source/common/common/matchers.h b/source/common/common/matchers.h index b3412369b1cc6..c0258be572c15 100644 --- a/source/common/common/matchers.h +++ b/source/common/common/matchers.h @@ -86,8 +86,6 @@ class DoubleMatcher : public ValueMatcher { class UniversalStringMatcher : public StringMatcher { public: bool match(absl::string_view) const override { return true; } - - const std::string& stringRepresentation() const override { return EMPTY_STRING; } }; StringMatcherPtr getExtensionStringMatcher(const ::xds::core::v3::TypedExtensionConfig& config, @@ -181,10 +179,6 @@ class StringMatcherImpl : public ValueMatcher, public StringMatcher { return false; } - // TODO(adiuissa): this will be removed once this entire class is replaced by - // StringMatcherImplBase. - const std::string& stringRepresentation() const override { return EMPTY_STRING; } - private: StringMatcherImpl(absl::string_view exact_match) : matcher_([&]() -> StringMatcherType { @@ -228,6 +222,12 @@ class StringMatcherImplBase : public ValueMatcher, public StringMatcher { UNREFERENCED_PARAMETER(prefix); return false; } + + /** + * Returns a string representation of the matcher (the contents to be + * matched). + */ + virtual const std::string& stringRepresentation() const PURE; }; // A matcher for the `exact` StringMatcher. @@ -353,9 +353,7 @@ class CustomStringMatcher : public StringMatcherImplBase { // StringMatcher bool match(const absl::string_view value) const override { return custom_->match(value); } - const std::string& stringRepresentation() const override { - return custom_->stringRepresentation(); - } + const std::string& stringRepresentation() const override { return EMPTY_STRING; } private: const StringMatcherPtr custom_; @@ -464,9 +462,9 @@ class PathMatcher : public StringMatcher { return matcher_; } // TODO(adisuissa): This method will replace the `matcher()` call above. - const std::string& stringRepresentation() const override { + const std::string& stringRepresentation() const { // TODO(adisuissa): replace with: return matcher_->stringRepresentation(); - // after the type of `matcher_` is replaced. + // after the type of `matcher_` is replaced with type StringMatcherImplBase. return EMPTY_STRING; } diff --git a/source/extensions/string_matcher/lua/match.cc b/source/extensions/string_matcher/lua/match.cc index a70cbc8e2998a..58a26430f8f2e 100644 --- a/source/extensions/string_matcher/lua/match.cc +++ b/source/extensions/string_matcher/lua/match.cc @@ -89,9 +89,6 @@ class LuaStringMatcherThreadWrapper : public Matchers::StringMatcher { } bool match(const absl::string_view value) const override { return (*tls_slot_)->match(value); } - const std::string& stringRepresentation() const override { - return (*tls_slot_)->stringRepresentation(); - } private: ThreadLocal::TypedSlotPtr tls_slot_; diff --git a/source/extensions/string_matcher/lua/match.h b/source/extensions/string_matcher/lua/match.h index 830395e3bcc42..436ea36c270ba 100644 --- a/source/extensions/string_matcher/lua/match.h +++ b/source/extensions/string_matcher/lua/match.h @@ -22,10 +22,6 @@ class LuaStringMatcher : public Matchers::StringMatcher, public ThreadLocal::Thr // Matchers::StringMatcher bool match(const absl::string_view value) const override; - const std::string& stringRepresentation() const override { - CONSTRUCT_ON_FIRST_USE(std::string, "Optimized for LuaStringMatcher"); - } - private: CSmartPtr state_; int matcher_func_ref_{LUA_NOREF}; diff --git a/test/mocks/matcher/mocks.h b/test/mocks/matcher/mocks.h index 7c4d9fa8dccd0..46483563ed62c 100644 --- a/test/mocks/matcher/mocks.h +++ b/test/mocks/matcher/mocks.h @@ -24,7 +24,6 @@ namespace Matchers { class MockStringMatcher : public StringMatcher { public: MOCK_METHOD(bool, match, (absl::string_view), (const, override)); - MOCK_METHOD(const std::string&, stringRepresentation, (), (const, override)); }; } // namespace Matchers } // namespace Envoy From 5f936723049580c5e9425f61dab99a35a9f5f796 Mon Sep 17 00:00:00 2001 From: Adi Suissa-Peleg Date: Wed, 27 Nov 2024 13:52:54 +0000 Subject: [PATCH 7/9] hyperscan Signed-off-by: Adi Suissa-Peleg --- contrib/hyperscan/matching/input_matchers/source/matcher.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contrib/hyperscan/matching/input_matchers/source/matcher.h b/contrib/hyperscan/matching/input_matchers/source/matcher.h index d49743ae0c5da..70bcae4e6baa8 100644 --- a/contrib/hyperscan/matching/input_matchers/source/matcher.h +++ b/contrib/hyperscan/matching/input_matchers/source/matcher.h @@ -40,6 +40,9 @@ class Matcher : public Envoy::Regex::CompiledMatcher, public Envoy::Matcher::Inp // Envoy::Regex::CompiledMatcher bool match(absl::string_view value) const override; std::string replaceAll(absl::string_view value, absl::string_view substitution) const override; + const std::string& stringRepresentation() const override { + CONSTRUCT_ON_FIRST_USE(std::string, "Optimized for HyperScan"); + } // Envoy::Matcher::InputMatcher bool match(const ::Envoy::Matcher::MatchingDataType& input) override; From 6a85808f9607b8694e8c7895777a791d04aa060b Mon Sep 17 00:00:00 2001 From: Adi Suissa-Peleg Date: Mon, 2 Dec 2024 17:22:00 +0000 Subject: [PATCH 8/9] Adding more tests to increase coverage Signed-off-by: Adi Suissa-Peleg --- source/common/common/matchers.cc | 10 -- source/common/common/matchers.h | 5 +- test/common/common/BUILD | 7 + .../common/custom_test_string_matcher.proto | 7 + test/common/common/matchers_test.cc | 159 ++++++++++++++++++ 5 files changed, 174 insertions(+), 14 deletions(-) create mode 100644 test/common/common/custom_test_string_matcher.proto diff --git a/source/common/common/matchers.cc b/source/common/common/matchers.cc index c19673ef2b50d..a6fbf49c95e0d 100644 --- a/source/common/common/matchers.cc +++ b/source/common/common/matchers.cc @@ -189,16 +189,6 @@ PathMatcher::createPrefix(const std::string& prefix, bool ignore_case, return createPrefixPathMatcher(prefix, ignore_case, context); } -PathMatcherConstSharedPtr -PathMatcher::createPattern(const std::string& pattern, bool ignore_case, - Server::Configuration::CommonFactoryContext& context) { - // TODO(silverstar194): implement pattern specific matcher - envoy::type::matcher::v3::StringMatcher matcher; - matcher.set_prefix(pattern); - matcher.set_ignore_case(ignore_case); - return std::make_shared(matcher, context); -} - PathMatcherConstSharedPtr PathMatcher::createSafeRegex(const envoy::type::matcher::v3::RegexMatcher& regex_matcher, Server::Configuration::CommonFactoryContext& context) { diff --git a/source/common/common/matchers.h b/source/common/common/matchers.h index c0258be572c15..819a58154e637 100644 --- a/source/common/common/matchers.h +++ b/source/common/common/matchers.h @@ -353,7 +353,7 @@ class CustomStringMatcher : public StringMatcherImplBase { // StringMatcher bool match(const absl::string_view value) const override { return custom_->match(value); } - const std::string& stringRepresentation() const override { return EMPTY_STRING; } + const std::string& stringRepresentation() const override { PANIC("not implemented"); } private: const StringMatcherPtr custom_; @@ -451,9 +451,6 @@ class PathMatcher : public StringMatcher { createPrefix(const std::string& prefix, bool ignore_case, Server::Configuration::CommonFactoryContext& context); static PathMatcherConstSharedPtr - createPattern(const std::string& pattern, bool ignore_case, - Server::Configuration::CommonFactoryContext& context); - static PathMatcherConstSharedPtr createSafeRegex(const envoy::type::matcher::v3::RegexMatcher& regex_matcher, Server::Configuration::CommonFactoryContext& context); diff --git a/test/common/common/BUILD b/test/common/common/BUILD index acf62a947fbd5..f504326f3651a 100644 --- a/test/common/common/BUILD +++ b/test/common/common/BUILD @@ -5,6 +5,7 @@ load( "envoy_cc_fuzz_test", "envoy_cc_test", "envoy_package", + "envoy_proto_library", "envoy_select_boringssl", ) @@ -260,11 +261,17 @@ envoy_cc_fuzz_test( deps = ["//source/common/common:minimal_logger_lib"], ) +envoy_proto_library( + name = "custom_test_string_matcher_proto", + srcs = ["custom_test_string_matcher.proto"], +) + envoy_cc_test( name = "matchers_test", srcs = ["matchers_test.cc"], rbe_pool = "6gig", deps = [ + ":custom_test_string_matcher_proto_cc_proto", "//source/common/common:matchers_lib", "//source/common/config:metadata_lib", "//source/common/protobuf:utility_lib", diff --git a/test/common/common/custom_test_string_matcher.proto b/test/common/common/custom_test_string_matcher.proto new file mode 100644 index 0000000000000..28ad46df2b944 --- /dev/null +++ b/test/common/common/custom_test_string_matcher.proto @@ -0,0 +1,7 @@ +syntax = "proto3"; + +package test.string_matcher; + +message CustomTestStringMatcher { + string exact = 1; +} diff --git a/test/common/common/matchers_test.cc b/test/common/common/matchers_test.cc index 5b7b04c7a11eb..67542863f763d 100644 --- a/test/common/common/matchers_test.cc +++ b/test/common/common/matchers_test.cc @@ -9,6 +9,8 @@ #include "source/common/protobuf/protobuf.h" #include "source/common/stream_info/filter_state_impl.h" +#include "test/common/common/custom_test_string_matcher.pb.h" +#include "test/common/common/custom_test_string_matcher.pb.validate.h" #include "test/mocks/server/server_factory_context.h" #include "test/test_common/utility.h" @@ -74,6 +76,20 @@ TEST_F(MetadataTest, MatchDoubleValue) { r->set_start(9); r->set_end(9.1); EXPECT_TRUE(Envoy::Matchers::MetadataMatcher(matcher, context_).match(metadata)); + + // Matching a non-number should return false. + { + envoy::config::core::v3::Metadata metadataStringOnly; + Envoy::Config::Metadata::mutableMetadataValue(metadataStringOnly, "envoy.filter.a", "label") + .set_string_value("test"); + + envoy::type::matcher::v3::MetadataMatcher matcherDoubleOnly; + matcherDoubleOnly.set_filter("envoy.filter.a"); + matcherDoubleOnly.add_path()->set_key("label"); + matcherDoubleOnly.mutable_value()->mutable_double_match()->set_exact(1); + EXPECT_FALSE( + Envoy::Matchers::MetadataMatcher(matcherDoubleOnly, context_).match(metadataStringOnly)); + } } TEST_F(MetadataTest, MatchStringExactValue) { @@ -247,6 +263,20 @@ TEST_F(MetadataTest, MatchStringListValue) { values->clear_values(); metadataValue.Clear(); + + // Matching a non-list entry should return false. + { + envoy::config::core::v3::Metadata metadataStringOnly; + Envoy::Config::Metadata::mutableMetadataValue(metadataStringOnly, "envoy.filter.a", "groups") + .set_string_value("test"); + + envoy::type::matcher::v3::MetadataMatcher matcherListOnly; + matcherListOnly.set_filter("envoy.filter.a"); + matcherListOnly.add_path()->set_key("groups"); + listMatchEntry(&matcherListOnly)->mutable_string_match()->set_exact("some_string"); + EXPECT_FALSE( + Envoy::Matchers::MetadataMatcher(matcherListOnly, context_).match(metadataStringOnly)); + } } TEST_F(MetadataTest, MatchBoolListValue) { @@ -328,6 +358,40 @@ TEST_F(MetadataTest, InvertMatch) { EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher, context_).match(metadata)); } +// A custom string matcher that invokes the exact string matcher code. +class CustomTestStringMatcher : public Matchers::StringMatcher { +public: + CustomTestStringMatcher(const std::string& exact) : internal_matcher_(exact, false) {} + + // Matchers::StringMatcher + bool match(const absl::string_view value) const override { + return internal_matcher_.match(value); + } + +private: + const Matchers::ExactStringMatcher internal_matcher_; +}; + +class CustomTestStringMatcherFactory : public Matchers::StringMatcherExtensionFactory { +public: + Matchers::StringMatcherPtr + createStringMatcher(const Protobuf::Message& untyped_config, + Server::Configuration::CommonFactoryContext& context) override { + const auto& config = + MessageUtil::downcastAndValidate( + untyped_config, context.messageValidationContext().staticValidationVisitor()); + return std::make_unique(config.exact()); + } + + std::string name() const override { return "test.string_matcher.custom_test"; } + + ProtobufTypes::MessagePtr createEmptyConfigProto() override { + return std::make_unique(); + } +}; + +REGISTER_FACTORY(CustomTestStringMatcherFactory, Matchers::StringMatcherExtensionFactory); + class StringMatcher : public BaseTest {}; // TODO(adisuissa): remove once StringMatcherImpl is deprecated. @@ -423,11 +487,37 @@ TEST_F(StringMatcher, ExactMatchIgnoreCase) { EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("exacz")); EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("other")); + { + std::string prefix_opt; + EXPECT_FALSE( + Matchers::createStringMatcher(matcher, context_)->getCaseSensitivePrefixMatch(prefix_opt)); + } + matcher.set_ignore_case(true); EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("exact")); EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("EXACT")); EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("exacz")); EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("other")); + + { + std::string prefix_opt; + EXPECT_FALSE( + Matchers::createStringMatcher(matcher, context_)->getCaseSensitivePrefixMatch(prefix_opt)); + } +} + +TEST_F(StringMatcher, ExactMatchValue) { + envoy::type::matcher::v3::StringMatcher matcher; + matcher.set_exact("exact"); + ProtobufWkt::Value value; + value.set_string_value("exact"); + EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match(value)); + value.set_string_value("EXACT"); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match(value)); + + ProtobufWkt::Value double_value; + double_value.set_number_value(9); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match(value)); } TEST_F(StringMatcher, PrefixMatchIgnoreCase) { @@ -445,6 +535,25 @@ TEST_F(StringMatcher, PrefixMatchIgnoreCase) { EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("other")); } +TEST_F(StringMatcher, PrefixMatchCaseSensitivePrefixOptimization) { + envoy::type::matcher::v3::StringMatcher matcher; + matcher.set_prefix("prefix"); + + { + std::string prefix_opt; + EXPECT_TRUE( + Matchers::createStringMatcher(matcher, context_)->getCaseSensitivePrefixMatch(prefix_opt)); + EXPECT_EQ(prefix_opt, "prefix"); + } + + matcher.set_ignore_case(true); + { + std::string prefix_opt; + EXPECT_FALSE( + Matchers::createStringMatcher(matcher, context_)->getCaseSensitivePrefixMatch(prefix_opt)); + } +} + TEST_F(StringMatcher, SuffixMatchIgnoreCase) { envoy::type::matcher::v3::StringMatcher matcher; matcher.set_suffix("suffix"); @@ -453,11 +562,23 @@ TEST_F(StringMatcher, SuffixMatchIgnoreCase) { EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("abc-suffiz")); EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("other")); + { + std::string prefix_opt; + EXPECT_FALSE( + Matchers::createStringMatcher(matcher, context_)->getCaseSensitivePrefixMatch(prefix_opt)); + } + matcher.set_ignore_case(true); EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("abc-suffix")); EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("ABC-SUFFIX")); EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("abc-suffiz")); EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("other")); + + { + std::string prefix_opt; + EXPECT_FALSE( + Matchers::createStringMatcher(matcher, context_)->getCaseSensitivePrefixMatch(prefix_opt)); + } } TEST_F(StringMatcher, ContainsMatchIgnoreCase) { @@ -469,11 +590,23 @@ TEST_F(StringMatcher, ContainsMatchIgnoreCase) { EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("abc-container-int-def")); EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("other")); + { + std::string prefix_opt; + EXPECT_FALSE( + Matchers::createStringMatcher(matcher, context_)->getCaseSensitivePrefixMatch(prefix_opt)); + } + matcher.set_ignore_case(true); EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("abc-contained-str-def")); EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("abc-cOnTaInEd-str-def")); EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("abc-ContAineR-str-def")); EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("other")); + + { + std::string prefix_opt; + EXPECT_FALSE( + Matchers::createStringMatcher(matcher, context_)->getCaseSensitivePrefixMatch(prefix_opt)); + } } TEST_F(StringMatcher, SafeRegexValue) { @@ -483,6 +616,12 @@ TEST_F(StringMatcher, SafeRegexValue) { EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("foo")); EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("foobar")); EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("bar")); + + { + std::string prefix_opt; + EXPECT_FALSE( + Matchers::createStringMatcher(matcher, context_)->getCaseSensitivePrefixMatch(prefix_opt)); + } } TEST_F(StringMatcher, SafeRegexValueIgnoreCase) { @@ -494,6 +633,26 @@ TEST_F(StringMatcher, SafeRegexValueIgnoreCase) { EnvoyException, "ignore_case has no effect for safe_regex."); } +TEST_F(StringMatcher, CustomMatchIgnoreCase) { + test::string_matcher::CustomTestStringMatcher custom_matcher; + custom_matcher.set_exact("exact"); + envoy::type::matcher::v3::StringMatcher matcher; + auto* string_match_extension = matcher.mutable_custom(); + string_match_extension->set_name("unused test.string_matcher.custom"); + string_match_extension->mutable_typed_config()->PackFrom(custom_matcher); + + EXPECT_TRUE(Matchers::createStringMatcher(matcher, context_)->match("exact")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("EXACT")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("exacz")); + EXPECT_FALSE(Matchers::createStringMatcher(matcher, context_)->match("other")); + + { + std::string prefix_opt; + EXPECT_FALSE( + Matchers::createStringMatcher(matcher, context_)->getCaseSensitivePrefixMatch(prefix_opt)); + } +} + TEST_F(StringMatcher, ExactMatchIgnoreCaseStringRepresentation) { envoy::type::matcher::v3::StringMatcher matcher; matcher.set_exact("eXaCt"); From f186d000e943f6e93301a1103695e3a7406b4492 Mon Sep 17 00:00:00 2001 From: Adi Suissa-Peleg Date: Mon, 2 Dec 2024 17:29:29 +0000 Subject: [PATCH 9/9] more coverage tests Signed-off-by: Adi Suissa-Peleg --- test/common/common/matchers_test.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/common/common/matchers_test.cc b/test/common/common/matchers_test.cc index 67542863f763d..a9720545d8581 100644 --- a/test/common/common/matchers_test.cc +++ b/test/common/common/matchers_test.cc @@ -798,6 +798,15 @@ TEST_F(PathMatcher, MatchSuffixPath) { EXPECT_FALSE(Matchers::PathMatcher(matcher, context_).match("/suffiz#suffix")); } +TEST_F(PathMatcher, SuffixPathStringRepresentation) { + envoy::type::matcher::v3::PathMatcher matcher; + matcher.mutable_path()->set_suffix("suffix"); + + // TODO(adisuissa): once the internal matcher_ type is replaced, + // this test will need to be updated. + EXPECT_EQ(Matchers::PathMatcher(matcher, context_).stringRepresentation(), ""); +} + TEST_F(PathMatcher, MatchContainsPath) { envoy::type::matcher::v3::PathMatcher matcher; matcher.mutable_path()->set_contains("contains");