diff --git a/CMakeLists.txt b/CMakeLists.txt index 28ebb186e8e..d6b552a4ec4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -15109,7 +15109,7 @@ endif() if(gRPC_BUILD_TESTS) add_executable(matchers_test - test/core/security/matchers_test.cc + test/core/matchers/matchers_test.cc test/core/util/cmdline.cc test/core/util/fuzzer_util.cc test/core/util/grpc_profiler.cc diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 6d2e8139e98..6f6fa14c5ed 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -9530,7 +9530,7 @@ targets: - test/core/util/subprocess.h - test/core/util/tracer_util.h src: - - test/core/security/matchers_test.cc + - test/core/matchers/matchers_test.cc - test/core/util/cmdline.cc - test/core/util/fuzzer_util.cc - test/core/util/grpc_profiler.cc diff --git a/doc/grpc_xds_features.md b/doc/grpc_xds_features.md index 389bd16b132..b997cb01a5e 100644 --- a/doc/grpc_xds_features.md +++ b/doc/grpc_xds_features.md @@ -71,3 +71,4 @@ Support for [xDS v2 APIs](https://www.envoyproxy.io/docs/envoy/latest/api/api_su [Custom Load Balancer Configuration](https://github.com/envoyproxy/envoy/blob/57be3189ffa3372b34e9480d1f02b2d165e49077/api/envoy/config/cluster/v3/cluster.proto#L1208) | [A52](https://github.com/grpc/proposal/blob/master/A52-xds-custom-lb-policies.md) | v1.55.0 | v1.47.0 | | | [xDS Federation](https://github.com/cncf/xds/blob/main/proposals/TP1-xds-transport-next.md) | [A47](https://github.com/grpc/proposal/blob/master/A47-xds-federation.md) | v1.55.0 | | | | [Client-Side Weighted Round Robin LB Policy](https://github.com/envoyproxy/envoy/blob/a6d46b6ac4750720eec9a49abe701f0df9bf8e0a/api/envoy/extensions/load_balancing_policies/client_side_weighted_round_robin/v3/client_side_weighted_round_robin.proto#L36) | [A58](https://github.com/grpc/proposal/blob/master/A58-client-side-weighted-round-robin-lb-policy.md) | v1.55.0 | | | | +[StringMatcher for Header Matching](https://github.com/envoyproxy/envoy/blob/3fe4b8d335fa339ef6f17325c8d31f87ade7bb1a/api/envoy/config/route/v3/route_components.proto#L2280) | [A63](https://github.com/grpc/proposal/blob/master/A63-xds-string-matcher-in-header-matching.md) | v1.56.0 | | | | diff --git a/src/core/ext/xds/xds_route_config.cc b/src/core/ext/xds/xds_route_config.cc index aef1037abf2..4fafe69646d 100644 --- a/src/core/ext/xds/xds_route_config.cc +++ b/src/core/ext/xds/xds_route_config.cc @@ -45,6 +45,7 @@ #include "envoy/config/route/v3/route.upbdefs.h" #include "envoy/config/route/v3/route_components.upb.h" #include "envoy/type/matcher/v3/regex.upb.h" +#include "envoy/type/matcher/v3/string.upb.h" #include "envoy/type/v3/percent.upb.h" #include "envoy/type/v3/range.upb.h" #include "google/protobuf/any.upb.h" @@ -486,6 +487,7 @@ void RouteHeaderMatchersParse(const envoy_config_route_v3_RouteMatch* match, int64_t range_start = 0; int64_t range_end = 0; bool present_match = false; + bool case_sensitive = true; if (envoy_config_route_v3_HeaderMatcher_has_exact_match(header)) { type = HeaderMatcher::Type::kExact; match_string = UpbStringToStdString( @@ -514,11 +516,46 @@ void RouteHeaderMatchersParse(const envoy_config_route_v3_RouteMatch* match, type = HeaderMatcher::Type::kRange; const envoy_type_v3_Int64Range* range_matcher = envoy_config_route_v3_HeaderMatcher_range_match(header); + GPR_ASSERT(range_matcher != nullptr); range_start = envoy_type_v3_Int64Range_start(range_matcher); range_end = envoy_type_v3_Int64Range_end(range_matcher); } else if (envoy_config_route_v3_HeaderMatcher_has_present_match(header)) { type = HeaderMatcher::Type::kPresent; present_match = envoy_config_route_v3_HeaderMatcher_present_match(header); + } else if (envoy_config_route_v3_HeaderMatcher_has_string_match(header)) { + ValidationErrors::ScopedField field(errors, ".string_match"); + const auto* matcher = + envoy_config_route_v3_HeaderMatcher_string_match(header); + GPR_ASSERT(matcher != nullptr); + if (envoy_type_matcher_v3_StringMatcher_has_exact(matcher)) { + type = HeaderMatcher::Type::kExact; + match_string = UpbStringToStdString( + envoy_type_matcher_v3_StringMatcher_exact(matcher)); + } else if (envoy_type_matcher_v3_StringMatcher_has_prefix(matcher)) { + type = HeaderMatcher::Type::kPrefix; + match_string = UpbStringToStdString( + envoy_type_matcher_v3_StringMatcher_prefix(matcher)); + } else if (envoy_type_matcher_v3_StringMatcher_has_suffix(matcher)) { + type = HeaderMatcher::Type::kSuffix; + match_string = UpbStringToStdString( + envoy_type_matcher_v3_StringMatcher_suffix(matcher)); + } else if (envoy_type_matcher_v3_StringMatcher_has_contains(matcher)) { + type = HeaderMatcher::Type::kContains; + match_string = UpbStringToStdString( + envoy_type_matcher_v3_StringMatcher_contains(matcher)); + } else if (envoy_type_matcher_v3_StringMatcher_has_safe_regex(matcher)) { + type = HeaderMatcher::Type::kSafeRegex; + const auto* regex_matcher = + envoy_type_matcher_v3_StringMatcher_safe_regex(matcher); + GPR_ASSERT(regex_matcher != nullptr); + match_string = UpbStringToStdString( + envoy_type_matcher_v3_RegexMatcher_regex(regex_matcher)); + } else { + errors->AddError("invalid string matcher"); + continue; + } + case_sensitive = + !envoy_type_matcher_v3_StringMatcher_ignore_case(matcher); } else { errors->AddError("invalid header matcher"); continue; @@ -527,7 +564,7 @@ void RouteHeaderMatchersParse(const envoy_config_route_v3_RouteMatch* match, envoy_config_route_v3_HeaderMatcher_invert_match(header); absl::StatusOr header_matcher = HeaderMatcher::Create(name, type, match_string, range_start, range_end, - present_match, invert_match); + present_match, invert_match, case_sensitive); if (!header_matcher.ok()) { errors->AddError(absl::StrCat("cannot create header matcher: ", header_matcher.status().message())); diff --git a/src/core/lib/matchers/matchers.cc b/src/core/lib/matchers/matchers.cc index 5d57f4464e9..0b165fa82f2 100644 --- a/src/core/lib/matchers/matchers.cc +++ b/src/core/lib/matchers/matchers.cc @@ -159,12 +159,11 @@ std::string StringMatcher::ToString() const { absl::StatusOr HeaderMatcher::Create( absl::string_view name, Type type, absl::string_view matcher, int64_t range_start, int64_t range_end, bool present_match, - bool invert_match) { + bool invert_match, bool case_sensitive) { if (static_cast(type) < 5) { // Only for EXACT, PREFIX, SUFFIX, SAFE_REGEX and CONTAINS. - absl::StatusOr string_matcher = - StringMatcher::Create(static_cast(type), matcher, - /*case_sensitive=*/true); + absl::StatusOr string_matcher = StringMatcher::Create( + static_cast(type), matcher, case_sensitive); if (!string_matcher.ok()) { return string_matcher.status(); } diff --git a/src/core/lib/matchers/matchers.h b/src/core/lib/matchers/matchers.h index a969eb82d32..dff5f3ab8c6 100644 --- a/src/core/lib/matchers/matchers.h +++ b/src/core/lib/matchers/matchers.h @@ -113,7 +113,8 @@ class HeaderMatcher { int64_t range_start = 0, int64_t range_end = 0, bool present_match = false, - bool invert_match = false); + bool invert_match = false, + bool case_sensitive = true); HeaderMatcher() = default; HeaderMatcher(const HeaderMatcher& other); diff --git a/src/proto/grpc/testing/xds/v3/BUILD b/src/proto/grpc/testing/xds/v3/BUILD index a1a2f646510..4d506b3ff28 100644 --- a/src/proto/grpc/testing/xds/v3/BUILD +++ b/src/proto/grpc/testing/xds/v3/BUILD @@ -237,6 +237,7 @@ grpc_proto_library( "percent_proto", "range_proto", "regex_proto", + "string_proto", ], ) diff --git a/src/proto/grpc/testing/xds/v3/route.proto b/src/proto/grpc/testing/xds/v3/route.proto index 6c064b4edc4..97eb9d8a3a8 100644 --- a/src/proto/grpc/testing/xds/v3/route.proto +++ b/src/proto/grpc/testing/xds/v3/route.proto @@ -21,6 +21,7 @@ package envoy.config.route.v3; import "src/proto/grpc/testing/xds/v3/base.proto"; import "src/proto/grpc/testing/xds/v3/extension.proto"; import "src/proto/grpc/testing/xds/v3/regex.proto"; +import "src/proto/grpc/testing/xds/v3/string.proto"; import "src/proto/grpc/testing/xds/v3/percent.proto"; import "src/proto/grpc/testing/xds/v3/range.proto"; @@ -430,6 +431,8 @@ message HeaderMatcher { string suffix_match = 10; string contains_match = 12; + + type.matcher.v3.StringMatcher string_match = 13; } // If specified, the match result will be inverted before checking. Defaults to false. diff --git a/test/core/matchers/BUILD b/test/core/matchers/BUILD new file mode 100644 index 00000000000..38f4c78c057 --- /dev/null +++ b/test/core/matchers/BUILD @@ -0,0 +1,32 @@ +# Copyright 2017 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +load("//bazel:grpc_build_system.bzl", "grpc_cc_test", "grpc_package") + +licenses(["notice"]) + +grpc_package(name = "test/core/matchers") + +grpc_cc_test( + name = "matchers_test", + srcs = ["matchers_test.cc"], + external_deps = ["gtest"], + language = "C++", + deps = [ + "//:gpr", + "//:grpc", + "//test/core/util:grpc_test_util", + "//test/core/util:grpc_test_util_base", + ], +) diff --git a/test/core/security/matchers_test.cc b/test/core/matchers/matchers_test.cc similarity index 94% rename from test/core/security/matchers_test.cc rename to test/core/matchers/matchers_test.cc index c53aced1e36..75c6b498886 100644 --- a/test/core/security/matchers_test.cc +++ b/test/core/matchers/matchers_test.cc @@ -14,7 +14,8 @@ #include "src/core/lib/matchers/matchers.h" -#include +#include "absl/status/status.h" +#include "gtest/gtest.h" namespace grpc_core { @@ -141,6 +142,19 @@ TEST(HeaderMatcherTest, StringMatcher) { EXPECT_FALSE(header_matcher->Match(absl::nullopt)); } +TEST(HeaderMatcherTest, StringMatcherCaseInsensitive) { + auto header_matcher = + HeaderMatcher::Create(/*name=*/"key", HeaderMatcher::Type::kExact, + /*matcher=*/"exact", /*range_start=*/0, + /*range_end=*/0, /*present_match=*/false, + /*invert_match=*/false, /*case_sensitive=*/false); + ASSERT_TRUE(header_matcher.ok()); + EXPECT_TRUE(header_matcher->Match("exact")); + EXPECT_TRUE(header_matcher->Match("Exact")); + EXPECT_FALSE(header_matcher->Match("exacz")); + EXPECT_FALSE(header_matcher->Match(absl::nullopt)); +} + TEST(HeaderMatcherTest, StringMatcherWithInvertMatch) { auto header_matcher = HeaderMatcher::Create(/*name=*/"key", HeaderMatcher::Type::kExact, diff --git a/test/core/security/BUILD b/test/core/security/BUILD index 552340a1405..fc8f424946f 100644 --- a/test/core/security/BUILD +++ b/test/core/security/BUILD @@ -459,19 +459,6 @@ grpc_cc_test( ], ) -grpc_cc_test( - name = "matchers_test", - srcs = ["matchers_test.cc"], - external_deps = ["gtest"], - language = "C++", - deps = [ - "//:gpr", - "//:grpc", - "//test/core/util:grpc_test_util", - "//test/core/util:grpc_test_util_base", - ], -) - grpc_cc_test( name = "rbac_translator_test", srcs = ["rbac_translator_test.cc"], diff --git a/test/core/xds/xds_route_config_resource_type_test.cc b/test/core/xds/xds_route_config_resource_type_test.cc index b07e7815bc6..4bbc3730ace 100644 --- a/test/core/xds/xds_route_config_resource_type_test.cc +++ b/test/core/xds/xds_route_config_resource_type_test.cc @@ -65,6 +65,7 @@ #include "src/proto/grpc/testing/xds/v3/range.pb.h" #include "src/proto/grpc/testing/xds/v3/regex.pb.h" #include "src/proto/grpc/testing/xds/v3/route.pb.h" +#include "src/proto/grpc/testing/xds/v3/string.pb.h" #include "src/proto/grpc/testing/xds/v3/typed_struct.pb.h" #include "test/core/util/scoped_env_var.h" #include "test/core/util/test_config.h" @@ -1094,6 +1095,32 @@ TEST_F(RouteMatchTest, HeaderMatchers) { header_proto = route_proto->mutable_match()->add_headers(); header_proto->set_name("header6"); header_proto->set_present_match(true); + // header7: exact via StringMatch, case-insensitive + header_proto = route_proto->mutable_match()->add_headers(); + header_proto->set_name("header7"); + auto* string_match_proto = header_proto->mutable_string_match(); + string_match_proto->set_exact("exact2"); + string_match_proto->set_ignore_case(true); + // header8: prefix via StringMatch + header_proto = route_proto->mutable_match()->add_headers(); + header_proto->set_name("header8"); + string_match_proto = header_proto->mutable_string_match(); + string_match_proto->set_prefix("prefix2"); + // header9: suffix via StringMatch + header_proto = route_proto->mutable_match()->add_headers(); + header_proto->set_name("header9"); + string_match_proto = header_proto->mutable_string_match(); + string_match_proto->set_suffix("suffix2"); + // header10: contains via StringMatch + header_proto = route_proto->mutable_match()->add_headers(); + header_proto->set_name("header10"); + string_match_proto = header_proto->mutable_string_match(); + string_match_proto->set_contains("contains2"); + // header11: regex via StringMatch + header_proto = route_proto->mutable_match()->add_headers(); + header_proto->set_name("header11"); + string_match_proto = header_proto->mutable_string_match(); + string_match_proto->mutable_safe_regex()->set_regex("regex2"); std::string serialized_resource; ASSERT_TRUE(route_config.SerializeToString(&serialized_resource)); auto* resource_type = XdsRouteConfigResourceType::Get(); @@ -1108,7 +1135,7 @@ TEST_F(RouteMatchTest, HeaderMatchers) { auto& virtual_host = resource.virtual_hosts.front(); ASSERT_EQ(virtual_host.routes.size(), 1UL); auto& header_matchers = virtual_host.routes[0].matchers.header_matchers; - ASSERT_EQ(header_matchers.size(), 7UL); + ASSERT_EQ(header_matchers.size(), 12UL); // header0: exact match with invert EXPECT_EQ(header_matchers[0].ToString(), "HeaderMatcher{header0 not StringMatcher{exact=exact1}}"); @@ -1130,6 +1157,22 @@ TEST_F(RouteMatchTest, HeaderMatchers) { // header6: present match EXPECT_EQ(header_matchers[6].ToString(), "HeaderMatcher{header6 present=true}"); + // header7: exact via StringMatch, case-insensitive + EXPECT_EQ(header_matchers[7].ToString(), + "HeaderMatcher{header7 StringMatcher{exact=exact2, " + "case_sensitive=false}}"); + // header8: prefix via StringMatch + EXPECT_EQ(header_matchers[8].ToString(), + "HeaderMatcher{header8 StringMatcher{prefix=prefix2}}"); + // header9: suffix via StringMatch + EXPECT_EQ(header_matchers[9].ToString(), + "HeaderMatcher{header9 StringMatcher{suffix=suffix2}}"); + // header10: contains via StringMatch + EXPECT_EQ(header_matchers[10].ToString(), + "HeaderMatcher{header10 StringMatcher{contains=contains2}}"); + // header11: regex via StringMatch + EXPECT_EQ(header_matchers[11].ToString(), + "HeaderMatcher{header11 StringMatcher{safe_regex=regex2}}"); } TEST_F(RouteMatchTest, HeaderMatchersInvalid) { @@ -1148,6 +1191,10 @@ TEST_F(RouteMatchTest, HeaderMatchersInvalid) { header_proto->set_name("header1"); header_proto->mutable_range_match()->set_start(2); header_proto->mutable_range_match()->set_end(1); + // header2: StringMatcher empty + header_proto = route_proto->mutable_match()->add_headers(); + header_proto->set_name("header2"); + header_proto->mutable_string_match(); std::string serialized_resource; ASSERT_TRUE(route_config.SerializeToString(&serialized_resource)); auto* resource_type = XdsRouteConfigResourceType::Get(); @@ -1162,7 +1209,9 @@ TEST_F(RouteMatchTest, HeaderMatchersInvalid) { "field:virtual_hosts[0].routes[0].match.headers[1] " "error:cannot create header matcher: " "Invalid range specifier specified: " - "end cannot be smaller than start.]") + "end cannot be smaller than start.; " + "field:virtual_hosts[0].routes[0].match.headers[2].string_match " + "error:invalid string matcher]") << decode_result.resource.status(); }