The urgent argument is a platform-specific flag that leaked into the (ideally) platform-independent HTTP/2 transport layer. In an effort to clean up the cross-platform API surface, it would be helpful if we can remove this argument from the TCP Read api without losing the performance optimization that was introduced along with it (see #18240).
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.