Add Missing test case: present match

Interop reported an error for a present match and it is discovered this
case is missing from unit test.  We have test for present (false) and we
have special case test but we don't have the normal present (true) test.

We also had a bug in a our code:
if (!value.has_value()) {
    if (header_matcher.type ==
        XdsApi::Route::Matchers::HeaderMatcher::HeaderMatcherType::PRESENT) {
      return !header_matcher.present_match;
    } else {
      // For all other header matcher types, we need the header value to
      // exist to consider matches.
      return false;
    }
  }
  switch (header_matcher.type) {
    case XdsApi::Route::Matchers::HeaderMatcher::HeaderMatcherType::EXACT:
      return value.value() == header_matcher.string_matcher;
    case XdsApi::Route::Matchers::HeaderMatcher::HeaderMatcherType::REGEX:
      return RE2::FullMatch(value.value().data(), *header_matcher.regex_match);
    case XdsApi::Route::Matchers::HeaderMatcher::HeaderMatcherType::RANGE:
      int64_t int_value;
      if (!absl::SimpleAtoi(value.value(), &int_value)) {
        return false;
      }
      return int_value >= header_matcher.range_start &&
             int_value < header_matcher.range_end;
    case XdsApi::Route::Matchers::HeaderMatcher::HeaderMatcherType::PREFIX:
      return absl::StartsWith(value.value(), header_matcher.string_matcher);
    case XdsApi::Route::Matchers::HeaderMatcher::HeaderMatcherType::SUFFIX:
      return absl::EndsWith(value.value(), header_matcher.string_matcher);
    default:
      return false;
  }

Note if header has value, we will go into the swtich but there is no case
XdsApi::Route::Matchers::HeaderMatcher::HeaderMatcherType::PRESENT,
which means we would have gone into default and decleared not a match.

The same day this bug was discovered by interop test, https://github.com/grpc/grpc/pull/25122
was submitted and the refactoring fixed the bug.

Nevertheless, I added the test case for the future.
pull/25178/head
Donna Dionne 4 years ago
parent d17ef6be04
commit 83d6d96c86
  1. 19
      test/cpp/end2end/xds_end2end_test.cc

@ -4905,20 +4905,27 @@ TEST_P(LdsRdsTest, XdsRoutingHeadersMatching) {
header_matcher4->set_present_match(false);
auto* header_matcher5 = route1->mutable_match()->add_headers();
header_matcher5->set_name("header5");
header_matcher5->set_prefix_match("/grpc");
header_matcher5->set_present_match(true);
auto* header_matcher6 = route1->mutable_match()->add_headers();
header_matcher6->set_name("header6");
header_matcher6->set_suffix_match(".cc");
header_matcher6->set_invert_match(true);
header_matcher6->set_prefix_match("/grpc");
auto* header_matcher7 = route1->mutable_match()->add_headers();
header_matcher7->set_name("header7");
header_matcher7->set_suffix_match(".cc");
header_matcher7->set_invert_match(true);
route1->mutable_route()->set_cluster(kNewClusterName);
auto* default_route = route_config.mutable_virtual_hosts(0)->add_routes();
default_route->mutable_match()->set_prefix("");
default_route->mutable_route()->set_cluster(kDefaultClusterName);
SetRouteConfiguration(0, route_config);
std::vector<std::pair<std::string, std::string>> metadata = {
{"header1", "POST"}, {"header2", "blah"},
{"header3", "1"}, {"header5", "/grpc.testing.EchoTest1Service/"},
{"header1", "PUT"}, {"header6", "grpc.java"},
{"header1", "POST"},
{"header2", "blah"},
{"header3", "1"},
{"header5", "anything"},
{"header6", "/grpc.testing.EchoTest1Service/"},
{"header1", "PUT"},
{"header7", "grpc.java"},
{"header1", "GET"},
};
const auto header_match_rpc_options = RpcOptions()

Loading…
Cancel
Save