Chiebot-Mirror
/
grpc
mirror of https://github.com/grpc/grpc.git
8
0
Fork 0
Code Issues Projects Releases Wiki Activity

Change StringMatcher to not support case-insensitive regex matching. (#26567)

Browse Source 
* Change StringMatcher to not support case-insensitive regex matching.

* clang-format

* clang-tidy
pull/26586/head
Mark D. Roth 3 years ago committed by GitHub
parent fb9d0e0ae6
commit 34bf26357b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 11 additions and 100 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 28
      src/core/lib/matchers/matchers.cc
  2. 3
      src/core/lib/matchers/matchers.h
  3. 14
      test/core/security/matchers_test.cc
  4. 66
      test/cpp/end2end/xds_end2end_test.cc

28
src/core/lib/matchers/matchers.cc
Unescape View File

@ -32,14 +32,12 @@ absl::StatusOr<StringMatcher> 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<RE2>(std::string(matcher), options);
auto regex_matcher = absl::make_unique<RE2>(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<RE2> regex_matcher,
bool case_sensitive)
: type_(Type::kSafeRegex),
regex_matcher_(std::move(regex_matcher)),
case_sensitive_(case_sensitive) {}
StringMatcher::StringMatcher(std::unique_ptr<RE2> 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<RE2>(other.regex_matcher_->pattern(), options);
regex_matcher_ = absl::make_unique<RE2>(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<RE2>(other.regex_matcher_->pattern(), options);
regex_matcher_ = absl::make_unique<RE2>(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 "";
}

3
src/core/lib/matchers/matchers.h
Unescape View File

@ -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<StringMatcher> 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<RE2> regex_matcher, bool case_sensitive);
explicit StringMatcher(std::unique_ptr<RE2> regex_matcher);
Type type_ = Type::kExact;
std::string string_matcher_;

14
test/core/security/matchers_test.cc
Unescape View File

@ -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",

66
test/cpp/end2end/xds_end2end_test.cc
Unescape View File

@ -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";

Write Preview
Loading…
Cancel
Save