Skip to content

Commit

Permalink
fix header matcher for null value (#8503)
Browse files Browse the repository at this point in the history
  • Loading branch information
@YifeiZhuang
YifeiZhuang committed Sep 9, 2021
1 parent a6df9de commit 7ad7876
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 37 deletions.
32 changes: 1 addition & 31 deletions xds/src/main/java/io/grpc/xds/XdsNameResolver.java
View file
Expand Up @@ -576,7 +576,7 @@ static boolean matchRoute(RouteMatch routeMatch, String fullMethodName,
return false;
}
for (HeaderMatcher headerMatcher : routeMatch.headerMatchers()) {
if (!matchHeader(headerMatcher, getHeaderValue(headers, headerMatcher.name()))) {
if (!headerMatcher.matches(getHeaderValue(headers, headerMatcher.name()))) {
return false;
}
}
Expand All @@ -597,36 +597,6 @@ private static boolean matchPath(PathMatcher pathMatcher, String fullMethodName)
return pathMatcher.regEx().matches(fullMethodName);
}

// TODO(zivy): consider reuse Matchers.HeaderMatcher.matches()
private static boolean matchHeader(HeaderMatcher headerMatcher, @Nullable String value) {
if (headerMatcher.present() != null) {
return (value == null) == headerMatcher.present().equals(headerMatcher.inverted());
}
if (value == null) {
return false;
}
boolean baseMatch;
if (headerMatcher.exactValue() != null) {
baseMatch = headerMatcher.exactValue().equals(value);
} else if (headerMatcher.safeRegEx() != null) {
baseMatch = headerMatcher.safeRegEx().matches(value);
} else if (headerMatcher.range() != null) {
long numValue;
try {
numValue = Long.parseLong(value);
baseMatch = numValue >= headerMatcher.range().start()
&& numValue <= headerMatcher.range().end();
} catch (NumberFormatException ignored) {
baseMatch = false;
}
} else if (headerMatcher.prefix() != null) {
baseMatch = value.startsWith(headerMatcher.prefix());
} else {
baseMatch = value.endsWith(headerMatcher.suffix());
}
return baseMatch != headerMatcher.inverted();
}

@Nullable
private static String getHeaderValue(Metadata headers, String headerName) {
if (headerName.endsWith(Metadata.BINARY_HEADER_SUFFIX)) {
Expand Down
9 changes: 3 additions & 6 deletions xds/src/main/java/io/grpc/xds/internal/Matchers.java
View file
Expand Up @@ -117,13 +117,8 @@ private static HeaderMatcher create(String name, @Nullable String exactValue,

/** Returns the matching result. */
public boolean matches(@Nullable String value) {
if (present() != null) {
return (value == null) == present().equals(inverted());
}
// FIXME(zivy@): invert result for null value.
// https://github.com/envoyproxy/envoy/blob/0fae6970ddaf93f024908ba304bbd2b34e997a51/source/common/http/header_utility.cc#L130
if (value == null) {
return false;
return present() != null && present() == inverted();
}
boolean baseMatch;
if (exactValue() != null) {
Expand All @@ -141,6 +136,8 @@ public boolean matches(@Nullable String value) {
}
} else if (prefix() != null) {
baseMatch = value.startsWith(prefix());
} else if (present() != null) {
baseMatch = present();
} else {
baseMatch = value.endsWith(suffix());
}
Expand Down
13 changes: 13 additions & 0 deletions xds/src/test/java/io/grpc/xds/internal/MatcherTest.java
View file
Expand Up @@ -127,45 +127,58 @@ public void headerMatcher() {
HeaderMatcher matcher = HeaderMatcher.forExactValue("version", "v1", false);
assertThat(matcher.matches("v1")).isTrue();
assertThat(matcher.matches("v2")).isFalse();
assertThat(matcher.matches(null)).isFalse();

matcher = HeaderMatcher.forExactValue("version", "v1", true);
assertThat(matcher.matches("v1")).isFalse();
assertThat(matcher.matches( "v2")).isTrue();
assertThat(matcher.matches(null)).isFalse();

matcher = HeaderMatcher.forPresent("version", true, false);
assertThat(matcher.matches("any")).isTrue();
assertThat(matcher.matches(null)).isFalse();
matcher = HeaderMatcher.forPresent("version", true, true);
assertThat(matcher.matches("version")).isFalse();
assertThat(matcher.matches(null)).isTrue();
matcher = HeaderMatcher.forPresent("version", false, true);
assertThat(matcher.matches("tag")).isTrue();
assertThat(matcher.matches(null)).isFalse();
matcher = HeaderMatcher.forPresent("version", false, false);
assertThat(matcher.matches("tag")).isFalse();
assertThat(matcher.matches(null)).isTrue();

matcher = HeaderMatcher.forPrefix("version", "v2", false);
assertThat(matcher.matches("v22")).isTrue();
assertThat(matcher.matches(null)).isFalse();
matcher = HeaderMatcher.forPrefix("version", "v2", true);
assertThat(matcher.matches("v22")).isFalse();
assertThat(matcher.matches(null)).isFalse();

matcher = HeaderMatcher.forSuffix("version", "v1", false);
assertThat(matcher.matches("xv1")).isTrue();
assertThat(matcher.matches("v1x")).isFalse();
assertThat(matcher.matches(null)).isFalse();
matcher = HeaderMatcher.forSuffix("version", "v2", true);
assertThat(matcher.matches("xv1")).isTrue();
assertThat(matcher.matches("1v2")).isFalse();
assertThat(matcher.matches(null)).isFalse();

matcher = HeaderMatcher.forSafeRegEx("version", Pattern.compile("v2.*"), false);
assertThat(matcher.matches("v2..")).isTrue();
assertThat(matcher.matches("v1")).isFalse();
assertThat(matcher.matches(null)).isFalse();
matcher = HeaderMatcher.forSafeRegEx("version", Pattern.compile("v1\\..*"), true);
assertThat(matcher.matches("v1.43")).isFalse();
assertThat(matcher.matches("v2")).isTrue();
assertThat(matcher.matches(null)).isFalse();

matcher = HeaderMatcher.forRange("version", Range.create(8080L, 8090L), false);
assertThat(matcher.matches("8080")).isTrue();
assertThat(matcher.matches("1")).isFalse();
assertThat(matcher.matches(null)).isFalse();
matcher = HeaderMatcher.forRange("version", Range.create(8080L, 8090L), true);
assertThat(matcher.matches("1")).isTrue();
assertThat(matcher.matches("8080")).isFalse();
assertThat(matcher.matches(null)).isFalse();
}
}

0 comments on commit 7ad7876

Please sign in to comment.