diff --git a/src/core/lib/matchers/matchers.cc b/src/core/lib/matchers/matchers.cc index 489a4808004..b266176fbec 100644 --- a/src/core/lib/matchers/matchers.cc +++ b/src/core/lib/matchers/matchers.cc @@ -32,14 +32,12 @@ absl::StatusOr StringMatcher::Create(Type type, absl::string_view matcher, bool case_sensitive) { if (type == Type::kSafeRegex) { - RE2::Options options; - options.set_case_sensitive(case_sensitive); - auto regex_matcher = absl::make_unique(std::string(matcher), options); + auto regex_matcher = absl::make_unique(std::string(matcher)); if (!regex_matcher->ok()) { return absl::InvalidArgumentError( "Invalid regex string specified in matcher."); } - return StringMatcher(std::move(regex_matcher), case_sensitive); + return StringMatcher(std::move(regex_matcher)); } else { return StringMatcher(type, matcher, case_sensitive); } @@ -49,19 +47,13 @@ StringMatcher::StringMatcher(Type type, absl::string_view matcher, bool case_sensitive) : type_(type), string_matcher_(matcher), case_sensitive_(case_sensitive) {} -StringMatcher::StringMatcher(std::unique_ptr regex_matcher, - bool case_sensitive) - : type_(Type::kSafeRegex), - regex_matcher_(std::move(regex_matcher)), - case_sensitive_(case_sensitive) {} +StringMatcher::StringMatcher(std::unique_ptr regex_matcher) + : type_(Type::kSafeRegex), regex_matcher_(std::move(regex_matcher)) {} StringMatcher::StringMatcher(const StringMatcher& other) : type_(other.type_), case_sensitive_(other.case_sensitive_) { if (type_ == Type::kSafeRegex) { - RE2::Options options; - options.set_case_sensitive(other.case_sensitive_); - regex_matcher_ = - absl::make_unique(other.regex_matcher_->pattern(), options); + regex_matcher_ = absl::make_unique(other.regex_matcher_->pattern()); } else { string_matcher_ = other.string_matcher_; } @@ -70,10 +62,7 @@ StringMatcher::StringMatcher(const StringMatcher& other) StringMatcher& StringMatcher::operator=(const StringMatcher& other) { type_ = other.type_; if (type_ == Type::kSafeRegex) { - RE2::Options options; - options.set_case_sensitive(other.case_sensitive_); - regex_matcher_ = - absl::make_unique(other.regex_matcher_->pattern(), options); + regex_matcher_ = absl::make_unique(other.regex_matcher_->pattern()); } else { string_matcher_ = other.string_matcher_; } @@ -151,9 +140,8 @@ std::string StringMatcher::ToString() const { return absl::StrFormat("StringMatcher{contains=%s%s}", string_matcher_, case_sensitive_ ? "" : ", case_sensitive=false"); case Type::kSafeRegex: - return absl::StrFormat("StringMatcher{safe_regex=%s%s}", - regex_matcher_->pattern(), - case_sensitive_ ? "" : ", case_sensitive=false"); + return absl::StrFormat("StringMatcher{safe_regex=%s}", + regex_matcher_->pattern()); default: return ""; } diff --git a/src/core/lib/matchers/matchers.h b/src/core/lib/matchers/matchers.h index af2ce59bf18..fa17b416101 100644 --- a/src/core/lib/matchers/matchers.h +++ b/src/core/lib/matchers/matchers.h @@ -39,6 +39,7 @@ class StringMatcher { }; // Creates StringMatcher instance. Returns error status on failure. + // Note: case_sensitive is ignored for type kSafeRegex. static absl::StatusOr Create(Type type, absl::string_view matcher, bool case_sensitive = true); @@ -66,7 +67,7 @@ class StringMatcher { private: StringMatcher(Type type, absl::string_view matcher, bool case_sensitive); - StringMatcher(std::unique_ptr regex_matcher, bool case_sensitive); + explicit StringMatcher(std::unique_ptr regex_matcher); Type type_ = Type::kExact; std::string string_matcher_; diff --git a/test/core/security/matchers_test.cc b/test/core/security/matchers_test.cc index 0bacd167e97..d569b09f449 100644 --- a/test/core/security/matchers_test.cc +++ b/test/core/security/matchers_test.cc @@ -91,8 +91,7 @@ TEST(StringMatcherTest, InvalidRegex) { TEST(StringMatcherTest, SafeRegexMatchCaseSensitive) { auto string_matcher = StringMatcher::Create(StringMatcher::Type::kSafeRegex, - /*matcher=*/"regex.*", - /*case_sensitive=*/true); + /*matcher=*/"regex.*"); ASSERT_TRUE(string_matcher.ok()); EXPECT_TRUE(string_matcher->Match("regex-test")); EXPECT_FALSE(string_matcher->Match("xx-regex-test")); @@ -100,17 +99,6 @@ TEST(StringMatcherTest, SafeRegexMatchCaseSensitive) { EXPECT_FALSE(string_matcher->Match("test-regex")); } -TEST(StringMatcherTest, SafeRegexMatchCaseInSensitive) { - auto string_matcher = StringMatcher::Create(StringMatcher::Type::kSafeRegex, - /*matcher=*/"regex.*", - /*case_sensitive=*/false); - ASSERT_TRUE(string_matcher.ok()); - EXPECT_TRUE(string_matcher->Match("regex-test")); - EXPECT_TRUE(string_matcher->Match("Regex-test")); - EXPECT_FALSE(string_matcher->Match("xx-Regex-test")); - EXPECT_FALSE(string_matcher->Match("test-regex")); -} - TEST(StringMatcherTest, ContainsMatchCaseSensitive) { auto string_matcher = StringMatcher::Create(StringMatcher::Type::kContains, /*matcher=*/"contains", diff --git a/test/cpp/end2end/xds_end2end_test.cc b/test/cpp/end2end/xds_end2end_test.cc index 61f413fafdd..46fcb7c3161 100644 --- a/test/cpp/end2end/xds_end2end_test.cc +++ b/test/cpp/end2end/xds_end2end_test.cc @@ -4679,72 +4679,6 @@ TEST_P(LdsRdsTest, XdsRoutingPathRegexMatching) { EXPECT_EQ(kNumEcho2Rpcs, backends_[3]->backend_service2()->request_count()); } -TEST_P(LdsRdsTest, XdsRoutingPathRegexMatchingCaseInsensitive) { - const char* kNewCluster1Name = "new_cluster_1"; - const char* kNewEdsService1Name = "new_eds_service_name_1"; - const char* kNewCluster2Name = "new_cluster_2"; - const char* kNewEdsService2Name = "new_eds_service_name_2"; - const size_t kNumEcho1Rpcs = 10; - const size_t kNumEchoRpcs = 30; - SetNextResolution({}); - SetNextResolutionForLbChannelAllBalancers(); - // Populate new EDS resources. - AdsServiceImpl::EdsResourceArgs args({ - {"locality0", CreateEndpointsForBackends(0, 1)}, - }); - AdsServiceImpl::EdsResourceArgs args1({ - {"locality0", CreateEndpointsForBackends(1, 2)}, - }); - AdsServiceImpl::EdsResourceArgs args2({ - {"locality0", CreateEndpointsForBackends(2, 3)}, - }); - balancers_[0]->ads_service()->SetEdsResource(BuildEdsResource(args)); - balancers_[0]->ads_service()->SetEdsResource( - BuildEdsResource(args1, kNewEdsService1Name)); - balancers_[0]->ads_service()->SetEdsResource( - BuildEdsResource(args2, kNewEdsService2Name)); - // Populate new CDS resources. - Cluster new_cluster1 = default_cluster_; - new_cluster1.set_name(kNewCluster1Name); - new_cluster1.mutable_eds_cluster_config()->set_service_name( - kNewEdsService1Name); - balancers_[0]->ads_service()->SetCdsResource(new_cluster1); - Cluster new_cluster2 = default_cluster_; - new_cluster2.set_name(kNewCluster2Name); - new_cluster2.mutable_eds_cluster_config()->set_service_name( - kNewEdsService2Name); - balancers_[0]->ads_service()->SetCdsResource(new_cluster2); - // Populating Route Configurations for LDS. - RouteConfiguration new_route_config = default_route_config_; - // First route will not match, since it's case-sensitive. - // Second route will match with same path. - auto* route1 = new_route_config.mutable_virtual_hosts(0)->mutable_routes(0); - route1->mutable_match()->mutable_safe_regex()->set_regex( - ".*EcHoTeSt1SErViCe.*"); - route1->mutable_route()->set_cluster(kNewCluster1Name); - auto* route2 = new_route_config.mutable_virtual_hosts(0)->add_routes(); - route2->mutable_match()->mutable_safe_regex()->set_regex( - ".*EcHoTeSt1SErViCe.*"); - route2->mutable_match()->mutable_case_sensitive()->set_value(false); - route2->mutable_route()->set_cluster(kNewCluster2Name); - auto* default_route = new_route_config.mutable_virtual_hosts(0)->add_routes(); - default_route->mutable_match()->set_prefix(""); - default_route->mutable_route()->set_cluster(kDefaultClusterName); - SetRouteConfiguration(0, new_route_config); - CheckRpcSendOk(kNumEchoRpcs, RpcOptions().set_wait_for_ready(true)); - CheckRpcSendOk(kNumEcho1Rpcs, RpcOptions() - .set_rpc_service(SERVICE_ECHO1) - .set_rpc_method(METHOD_ECHO1) - .set_wait_for_ready(true)); - // Make sure RPCs all go to the correct backend. - EXPECT_EQ(kNumEchoRpcs, backends_[0]->backend_service()->request_count()); - EXPECT_EQ(0, backends_[0]->backend_service1()->request_count()); - EXPECT_EQ(0, backends_[1]->backend_service()->request_count()); - EXPECT_EQ(0, backends_[1]->backend_service1()->request_count()); - EXPECT_EQ(0, backends_[2]->backend_service()->request_count()); - EXPECT_EQ(kNumEcho1Rpcs, backends_[2]->backend_service1()->request_count()); -} - TEST_P(LdsRdsTest, XdsRoutingWeightedCluster) { const char* kNewCluster1Name = "new_cluster_1"; const char* kNewEdsService1Name = "new_eds_service_name_1";