Revert "fix(xds): Allow and normalize trailing dot (FQDN) in matchHostName (#12644) (1.80.x backport) (#12674)

We want to synchronize the behavior across all gRPC languages, and also
with envoy.

This reverts commit 0ef1b39.

Author: shivaspeaks

xds/src/main/java/io/grpc/xds/RoutingUtils.java

@@ -92,24 +92,15 @@ static VirtualHost findVirtualHostForHostName(List<VirtualHost> virtualHosts, St
    * </ol>
    */
   private static boolean matchHostName(String hostName, String pattern) {
-    checkArgument(hostName.length() != 0 && !hostName.startsWith("."),
+    checkArgument(hostName.length() != 0 && !hostName.startsWith(".") && !hostName.endsWith("."),
         "Invalid host name");
-    checkArgument(pattern.length() != 0 && !pattern.startsWith("."),
+    checkArgument(pattern.length() != 0 && !pattern.startsWith(".") && !pattern.endsWith("."),
         "Invalid pattern/domain name");
 
     hostName = hostName.toLowerCase(Locale.US);
     pattern = pattern.toLowerCase(Locale.US);
     // hostName and pattern are now in lower case -- domain names are case-insensitive.
 
-    // Strip trailing dot to normalize FQDN (e.g. "example.com.") to a relative form,
-    // as per RFC 1034 Section 3.1 the two are semantically equivalent.
-    if (hostName.endsWith(".")) {
-      hostName = hostName.substring(0, hostName.length() - 1);
-    }
-    if (pattern.endsWith(".")) {
-      pattern = pattern.substring(0, pattern.length() - 1);
-    }
-
     if (!pattern.contains("*")) {
       // Not a wildcard pattern -- hostName and pattern must match exactly.
       return hostName.equals(pattern);

xds/src/main/java/io/grpc/xds/XdsNameResolver.java

@@ -73,6 +73,7 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
@@ -340,6 +341,66 @@ private void updateResolutionResult(XdsConfig xdsConfig) {
     }
   }
 
+  /**
+   * Returns {@code true} iff {@code hostName} matches the domain name {@code pattern} with
+   * case-insensitive.
+   *
+   * <p>Wildcard pattern rules:
+   * <ol>
+   * <li>A single asterisk (*) matches any domain.</li>
+   * <li>Asterisk (*) is only permitted in the left-most or the right-most part of the pattern,
+   *     but not both.</li>
+   * </ol>
+   */
+  @VisibleForTesting
+  static boolean matchHostName(String hostName, String pattern) {
+    checkArgument(hostName.length() != 0 && !hostName.startsWith(".") && !hostName.endsWith("."),
+        "Invalid host name");
+    checkArgument(pattern.length() != 0 && !pattern.startsWith(".") && !pattern.endsWith("."),
+        "Invalid pattern/domain name");
+
+    hostName = hostName.toLowerCase(Locale.US);
+    pattern = pattern.toLowerCase(Locale.US);
+    // hostName and pattern are now in lower case -- domain names are case-insensitive.
+
+    if (!pattern.contains("*")) {
+      // Not a wildcard pattern -- hostName and pattern must match exactly.
+      return hostName.equals(pattern);
+    }
+    // Wildcard pattern
+
+    if (pattern.length() == 1) {
+      return true;
+    }
+
+    int index = pattern.indexOf('*');
+
+    // At most one asterisk (*) is allowed.
+    if (pattern.indexOf('*', index + 1) != -1) {
+      return false;
+    }
+
+    // Asterisk can only match prefix or suffix.
+    if (index != 0 && index != pattern.length() - 1) {
+      return false;
+    }
+
+    // HostName must be at least as long as the pattern because asterisk has to
+    // match one or more characters.
+    if (hostName.length() < pattern.length()) {
+      return false;
+    }
+
+    if (index == 0 && hostName.endsWith(pattern.substring(1))) {
+      // Prefix matching fails.
+      return true;
+    }
+
+    // Pattern matches hostname if suffix matching succeeds.
+    return index == pattern.length() - 1
+        && hostName.startsWith(pattern.substring(0, pattern.length() - 1));
+  }
+
   private final class ConfigSelector extends InternalConfigSelector {
     @Override
     public Result selectConfig(PickSubchannelArgs args) {

xds/src/test/java/io/grpc/xds/RoutingUtilsTest.java

@@ -17,7 +17,6 @@
 package io.grpc.xds;
 
 import static com.google.common.truth.Truth.assertThat;
-import static org.junit.Assert.assertThrows;
 import static org.mockito.Mockito.mock;
 
 import com.google.common.collect.ImmutableMap;
@@ -89,170 +88,6 @@ public void findVirtualHostForHostName_asteriskMatchAnyDomain() {
         .isEqualTo(vHost1);
   }
 
-  @Test
-  public void findVirtualHostForHostName_trailingDot() {
-    // FQDN (trailing dot) is semantically equivalent to the relative form
-    // per RFC 1034 Section 3.1.
-    List<Route> routes = Collections.emptyList();
-    VirtualHost vHost1 = VirtualHost.create("virtualhost01.googleapis.com",
-        Collections.singletonList("a.googleapis.com"), routes,
-        ImmutableMap.of());
-    VirtualHost vHost2 = VirtualHost.create("virtualhost02.googleapis.com",
-        Collections.singletonList("*.googleapis.com"), routes,
-        ImmutableMap.of());
-    VirtualHost vHost3 = VirtualHost.create("virtualhost03.googleapis.com",
-        Collections.singletonList("*"), routes,
-        ImmutableMap.of());
-    List<VirtualHost> virtualHosts = Arrays.asList(vHost1, vHost2, vHost3);
-
-    // Trailing dot in hostName, exact match.
-    assertThat(RoutingUtils.findVirtualHostForHostName(
-        virtualHosts, "a.googleapis.com.")).isEqualTo(vHost1);
-    // Trailing dot in hostName, wildcard match.
-    assertThat(RoutingUtils.findVirtualHostForHostName(
-        virtualHosts, "b.googleapis.com.")).isEqualTo(vHost2);
-
-    // Trailing dot in domain pattern, exact match.
-    VirtualHost vHost4 = VirtualHost.create("virtualhost04.googleapis.com",
-        Collections.singletonList("a.googleapis.com."), routes,
-        ImmutableMap.of());
-    List<VirtualHost> virtualHosts2 =
-        Arrays.asList(vHost4, vHost2, vHost3);
-    assertThat(RoutingUtils.findVirtualHostForHostName(
-        virtualHosts2, "a.googleapis.com")).isEqualTo(vHost4);
-
-    // Trailing dot in both hostName and domain pattern.
-    assertThat(RoutingUtils.findVirtualHostForHostName(
-        virtualHosts2, "a.googleapis.com.")).isEqualTo(vHost4);
-
-    // Trailing dot in domain pattern, wildcard match.
-    VirtualHost vHost5 = VirtualHost.create("virtualhost05.googleapis.com",
-        Collections.singletonList("*.googleapis.com."), routes,
-        ImmutableMap.of());
-    List<VirtualHost> virtualHosts3 =
-        Arrays.asList(vHost5, vHost3);
-    assertThat(RoutingUtils.findVirtualHostForHostName(
-        virtualHosts3, "b.googleapis.com")).isEqualTo(vHost5);
-    assertThat(RoutingUtils.findVirtualHostForHostName(
-        virtualHosts3, "b.googleapis.com.")).isEqualTo(vHost5);
-  }
-
-  @Test
-  public void findVirtualHostForHostName_exactMatch() {
-    List<Route> routes = Collections.emptyList();
-    VirtualHost vHostFoo = VirtualHost.create("vhost-foo",
-        Collections.singletonList("foo.googleapis.com"), routes,
-        ImmutableMap.of());
-    VirtualHost vHostBar = VirtualHost.create("vhost-bar",
-        Collections.singletonList("bar.googleapis.com"), routes,
-        ImmutableMap.of());
-    List<VirtualHost> virtualHosts =
-        Arrays.asList(vHostFoo, vHostBar);
-
-    assertThat(RoutingUtils.findVirtualHostForHostName(
-        virtualHosts, "foo.googleapis.com")).isEqualTo(vHostFoo);
-    assertThat(RoutingUtils.findVirtualHostForHostName(
-        virtualHosts, "bar.googleapis.com")).isEqualTo(vHostBar);
-    // No match returns null.
-    assertThat(RoutingUtils.findVirtualHostForHostName(
-        virtualHosts, "baz.googleapis.com")).isNull();
-    assertThat(RoutingUtils.findVirtualHostForHostName(
-        virtualHosts, "foo.googleapis")).isNull();
-  }
-
-  @Test
-  public void findVirtualHostForHostName_invalidHostName() {
-    List<Route> routes = Collections.emptyList();
-    VirtualHost vHost = VirtualHost.create("vhost",
-        Collections.singletonList("a.googleapis.com"), routes,
-        ImmutableMap.of());
-    List<VirtualHost> virtualHosts = Collections.singletonList(vHost);
-
-    // Empty hostName.
-    assertThrows(IllegalArgumentException.class,
-        () -> RoutingUtils.findVirtualHostForHostName(
-            virtualHosts, ""));
-    // HostName starting with dot.
-    assertThrows(IllegalArgumentException.class,
-        () -> RoutingUtils.findVirtualHostForHostName(
-            virtualHosts, ".a.googleapis.com"));
-  }
-
-  @Test
-  public void findVirtualHostForHostName_invalidPattern() {
-    List<Route> routes = Collections.emptyList();
-    // Empty domain pattern.
-    VirtualHost vHostEmpty = VirtualHost.create("vhost-empty",
-        Collections.singletonList(""), routes,
-        ImmutableMap.of());
-    assertThrows(IllegalArgumentException.class,
-        () -> RoutingUtils.findVirtualHostForHostName(
-            Collections.singletonList(vHostEmpty),
-            "a.googleapis.com"));
-    // Domain pattern starting with dot.
-    VirtualHost vHostDot = VirtualHost.create("vhost-dot",
-        Collections.singletonList(".a.googleapis.com"), routes,
-        ImmutableMap.of());
-    assertThrows(IllegalArgumentException.class,
-        () -> RoutingUtils.findVirtualHostForHostName(
-            Collections.singletonList(vHostDot),
-            "a.googleapis.com"));
-  }
-
-  @Test
-  public void findVirtualHostForHostName_prefixWildcard() {
-    List<Route> routes = Collections.emptyList();
-    VirtualHost vHostWild = VirtualHost.create("vhost-wild",
-        Collections.singletonList("*.foo.googleapis.com"),
-        routes, ImmutableMap.of());
-    VirtualHost vHostOther = VirtualHost.create("vhost-other",
-        Collections.singletonList("other.googleapis.com"),
-        routes, ImmutableMap.of());
-    List<VirtualHost> virtualHosts =
-        Arrays.asList(vHostWild, vHostOther);
-
-    // Prefix wildcard matches.
-    assertThat(RoutingUtils.findVirtualHostForHostName(
-        virtualHosts, "bar.foo.googleapis.com"))
-        .isEqualTo(vHostWild);
-    // Base domain without subdomain does not match *.foo.googleapis.com.
-    assertThat(RoutingUtils.findVirtualHostForHostName(
-        virtualHosts, "foo.googleapis.com")).isNull();
-
-    // Longer prefix wildcard is preferred over shorter one.
-    VirtualHost vHostLong = VirtualHost.create("vhost-long",
-        Collections.singletonList("*.bar.foo.googleapis.com"),
-        routes, ImmutableMap.of());
-    List<VirtualHost> virtualHosts2 =
-        Arrays.asList(vHostLong, vHostWild);
-    assertThat(RoutingUtils.findVirtualHostForHostName(
-        virtualHosts2, "baz.bar.foo.googleapis.com"))
-        .isEqualTo(vHostLong);
-  }
-
-  @Test
-  public void findVirtualHostForHostName_postfixWildcard() {
-    List<Route> routes = Collections.emptyList();
-    VirtualHost vHostWild = VirtualHost.create("vhost-wild",
-        Collections.singletonList("foo.*"), routes,
-        ImmutableMap.of());
-    VirtualHost vHostOther = VirtualHost.create("vhost-other",
-        Collections.singletonList("bar.googleapis.com"),
-        routes, ImmutableMap.of());
-    List<VirtualHost> virtualHosts =
-        Arrays.asList(vHostWild, vHostOther);
-
-    // Postfix wildcard matches.
-    assertThat(RoutingUtils.findVirtualHostForHostName(
-        virtualHosts, "foo.googleapis.com"))
-        .isEqualTo(vHostWild);
-    assertThat(RoutingUtils.findVirtualHostForHostName(
-        virtualHosts, "foo.com")).isEqualTo(vHostWild);
-    // Different prefix does not match foo.*.
-    assertThat(RoutingUtils.findVirtualHostForHostName(
-        virtualHosts, "bar.foo.googleapis.com")).isNull();
-  }
-
   @Test
   public void routeMatching_pathOnly() {
     Metadata headers = new Metadata();

xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java

@@ -2020,6 +2020,48 @@ public void generateServiceConfig_forPerMethodConfig() throws IOException {
         .isEqualTo(expectedServiceConfig);
   }
 
+  @Test
+  public void matchHostName_exactlyMatch() {
+    String pattern = "foo.googleapis.com";
+    assertThat(XdsNameResolver.matchHostName("bar.googleapis.com", pattern)).isFalse();
+    assertThat(XdsNameResolver.matchHostName("fo.googleapis.com", pattern)).isFalse();
+    assertThat(XdsNameResolver.matchHostName("oo.googleapis.com", pattern)).isFalse();
+    assertThat(XdsNameResolver.matchHostName("googleapis.com", pattern)).isFalse();
+    assertThat(XdsNameResolver.matchHostName("foo.googleapis", pattern)).isFalse();
+    assertThat(XdsNameResolver.matchHostName("foo.googleapis.com", pattern)).isTrue();
+  }
+
+  @Test
+  public void matchHostName_prefixWildcard() {
+    String pattern = "*.foo.googleapis.com";
+    assertThat(XdsNameResolver.matchHostName("foo.googleapis.com", pattern)).isFalse();
+    assertThat(XdsNameResolver.matchHostName("bar-baz.foo.googleapis", pattern)).isFalse();
+    assertThat(XdsNameResolver.matchHostName("bar.foo.googleapis.com", pattern)).isTrue();
+    pattern = "*-bar.foo.googleapis.com";
+    assertThat(XdsNameResolver.matchHostName("bar.foo.googleapis.com", pattern)).isFalse();
+    assertThat(XdsNameResolver.matchHostName("baz-bar.foo.googleapis", pattern)).isFalse();
+    assertThat(XdsNameResolver.matchHostName("-bar.foo.googleapis.com", pattern)).isFalse();
+    assertThat(XdsNameResolver.matchHostName("baz-bar.foo.googleapis.com", pattern))
+        .isTrue();
+  }
+
+  @Test
+  public void matchHostName_postfixWildCard() {
+    String pattern = "foo.*";
+    assertThat(XdsNameResolver.matchHostName("bar.googleapis.com", pattern)).isFalse();
+    assertThat(XdsNameResolver.matchHostName("bar.foo.googleapis.com", pattern)).isFalse();
+    assertThat(XdsNameResolver.matchHostName("foo.googleapis.com", pattern)).isTrue();
+    assertThat(XdsNameResolver.matchHostName("foo.com", pattern)).isTrue();
+    pattern = "foo-*";
+    assertThat(XdsNameResolver.matchHostName("bar-.googleapis.com", pattern)).isFalse();
+    assertThat(XdsNameResolver.matchHostName("foo.googleapis.com", pattern)).isFalse();
+    assertThat(XdsNameResolver.matchHostName("foo.googleapis.com", pattern)).isFalse();
+    assertThat(XdsNameResolver.matchHostName("foo-", pattern)).isFalse();
+    assertThat(XdsNameResolver.matchHostName("foo-bar.com", pattern)).isTrue();
+    assertThat(XdsNameResolver.matchHostName("foo-.com", pattern)).isTrue();
+    assertThat(XdsNameResolver.matchHostName("foo-bar", pattern)).isTrue();
+  }
+
   @Test
   public void resolved_faultAbortInLdsUpdate() {
     resolver.start(mockListener);