api: Add RFC 3986 support to DnsNameResolverProvider (#12602)

Accept both absolute (e.g. `dns:///hostname`) and rootless (e.g.
`dns:hostname`) paths as specified by
https://github.com/grpc/grpc/blob/master/doc/naming.md and matching the
behavior of grpc core and grpc-go.

Author: jdcormie

api/src/main/java/io/grpc/Uri.java

@@ -481,6 +481,15 @@ public String getPath() {
    * <p>Prefer this method over {@link #getPath()} because it preserves the distinction between
    * segment separators and literal '/'s within a path segment.
    *
+   * <p>A trailing '/' delimiter in the path results in the empty string as the last element in the
+   * returned list. For example, <code>file://localhost/foo/bar/</code> has path segments <code>
+   * ["foo", "bar", ""]</code>
+   *
+   * <p>A leading '/' delimiter cannot be detected using this method. For example, both <code>
+   * dns:example.com</code> and <code>dns:///example.com</code> have the same list of path segments:
+   * <code>["example.com"]</code>. Use {@link #isPathAbsolute()} or {@link #isPathRootless()} to
+   * distinguish these cases.
+   *
    * <p>The returned list is immutable.
    */
   public List<String> getPathSegments() {
@@ -490,6 +499,44 @@ public List<String> getPathSegments() {
     return segmentsBuilder.build();
   }
 
+  /**
+   * Returns true iff this URI's path component starts with a path segment (rather than the '/'
+   * segment delimiter).
+   *
+   * <p>The path of an RFC 3986 URI is either empty, absolute (starts with the '/' segment
+   * delimiter) or rootless (starts with a path segment). For example, <code>tel:+1-206-555-1212
+   * </code>, <code>mailto:[email protected]</code> and <code>urn:isbn:978-1492082798</code> all have
+   * rootless paths. <code>mailto:%2Fdev%[email protected]</code> is also rootless because its
+   * percent-encoded slashes are not segment delimiters but rather part of the first and only path
+   * segment.
+   *
+   * <p>Contrast rootless paths with absolute ones (see {@link #isPathAbsolute()}.
+   */
+  public boolean isPathRootless() {
+    return !path.isEmpty() && !path.startsWith("/");
+  }
+
+  /**
+   * Returns true iff this URI's path component starts with the '/' segment delimiter (rather than a
+   * path segment).
+   *
+   * <p>The path of an RFC 3986 URI is either empty, absolute (starts with the '/' segment
+   * delimiter) or rootless (starts with a path segment). For example, <code>file:///resume.txt
+   * </code>, <code>file:/resume.txt</code> and <code>file://localhost/</code> all have absolute
+   * paths while <code>tel:+1-206-555-1212</code>'s path is not absolute. <code>
+   * mailto:%2Fdev%[email protected]</code> is also not absolute because its percent-encoded
+   * slashes are not segment delimiters but rather part of the first and only path segment.
+   *
+   * <p>Contrast absolute paths with rootless ones (see {@link #isPathRootless()}.
+   *
+   * <p>NB: The term "absolute" has two different meanings in RFC 3986 which are easily confused.
+   * This method tests for a property of this URI's path component. Contrast with {@link
+   * #isAbsolute()} which tests the URI itself for a different property.
+   */
+  public boolean isPathAbsolute() {
+    return path.startsWith("/");
+  }
+
   /**
    * Returns the path component of this URI in its originally parsed, possibly percent-encoded form.
    */

api/src/test/java/io/grpc/UriTest.java

@@ -46,6 +46,8 @@ public void parse_allComponents() throws URISyntaxException {
     assertThat(uri.getFragment()).isEqualTo("fragment");
     assertThat(uri.toString()).isEqualTo("scheme://user@host:0443/path?query#fragment");
     assertThat(uri.isAbsolute()).isFalse(); // Has a fragment.
+    assertThat(uri.isPathAbsolute()).isTrue();
+    assertThat(uri.isPathRootless()).isFalse();
   }
 
   @Test
@@ -127,6 +129,8 @@ public void parse_emptyPathWithAuthority() throws URISyntaxException {
     assertThat(uri.getFragment()).isNull();
     assertThat(uri.toString()).isEqualTo("scheme://authority");
     assertThat(uri.isAbsolute()).isTrue();
+    assertThat(uri.isPathAbsolute()).isFalse();
+    assertThat(uri.isPathRootless()).isFalse();
   }
 
   @Test
@@ -139,6 +143,8 @@ public void parse_rootless() throws URISyntaxException {
     assertThat(uri.getFragment()).isNull();
     assertThat(uri.toString()).isEqualTo("mailto:[email protected]?subject=raise");
     assertThat(uri.isAbsolute()).isTrue();
+    assertThat(uri.isPathAbsolute()).isFalse();
+    assertThat(uri.isPathRootless()).isTrue();
   }
 
   @Test
@@ -151,6 +157,8 @@ public void parse_emptyPath() throws URISyntaxException {
     assertThat(uri.getFragment()).isNull();
     assertThat(uri.toString()).isEqualTo("scheme:");
     assertThat(uri.isAbsolute()).isTrue();
+    assertThat(uri.isPathAbsolute()).isFalse();
+    assertThat(uri.isPathRootless()).isFalse();
   }
 
   @Test
@@ -348,12 +356,34 @@ public void parse_onePathSegment_trailingSlash() throws URISyntaxException {
     assertThat(uri.getPathSegments()).containsExactly("foo", "");
   }
 
+  @Test
+  public void parse_onePathSegment_rootless() throws URISyntaxException {
+    Uri uri = Uri.create("dns:www.example.com");
+    assertThat(uri.getPathSegments()).containsExactly("www.example.com");
+    assertThat(uri.isPathAbsolute()).isFalse();
+    assertThat(uri.isPathRootless()).isTrue();
+  }
+
   @Test
   public void parse_twoPathSegments() throws URISyntaxException {
     Uri uri = Uri.create("file:/foo/bar");
     assertThat(uri.getPathSegments()).containsExactly("foo", "bar");
   }
 
+  @Test
+  public void parse_twoPathSegments_rootless() throws URISyntaxException {
+    Uri uri = Uri.create("file:foo/bar");
+    assertThat(uri.getPathSegments()).containsExactly("foo", "bar");
+  }
+
+  @Test
+  public void parse_percentEncodedPathSegment_rootless() throws URISyntaxException {
+    Uri uri = Uri.create("mailto:%2Fdev%[email protected]");
+    assertThat(uri.getPathSegments()).containsExactly("/dev/[email protected]");
+    assertThat(uri.isPathAbsolute()).isFalse();
+    assertThat(uri.isPathRootless()).isTrue();
+  }
+
   @Test
   public void toString_percentEncoding() throws URISyntaxException {
     Uri uri =

core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java

@@ -21,25 +21,30 @@
 import io.grpc.InternalServiceProviders;
 import io.grpc.NameResolver;
 import io.grpc.NameResolverProvider;
+import io.grpc.Uri;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.net.URI;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.List;
 
 /**
  * A provider for {@link DnsNameResolver}.
  *
  * <p>It resolves a target URI whose scheme is {@code "dns"}. The (optional) authority of the target
- * URI is reserved for the address of alternative DNS server (not implemented yet). The path of the
- * target URI, excluding the leading slash {@code '/'}, is treated as the host name and the optional
- * port to be resolved by DNS. Example target URIs:
+ * URI is reserved for the address of alternative DNS server (not implemented yet). The first path
+ * segment of the hierarchical target URI is interpreted as an RFC 2396 "server-based" authority and
+ * used as the "service authority" of the resulting {@link NameResolver}. The "host" part of this
+ * authority is the name to be resolved by DNS. The "port" part of this authority (if present) will
+ * become the port number for all {@link InetSocketAddress} produced by this resolver. For example:
  *
  * <ul>
  *   <li>{@code "dns:///foo.googleapis.com:8080"} (using default DNS)</li>
  *   <li>{@code "dns://8.8.8.8/foo.googleapis.com:8080"} (using alternative DNS (not implemented
  *   yet))</li>
- *   <li>{@code "dns:///foo.googleapis.com"} (without port)</li>
+ *   <li>{@code "dns:///foo.googleapis.com"} (output addresses will have port {@link
+ *       NameResolver.Args#getDefaultPort()})</li>
  * </ul>
  */
 public final class DnsNameResolverProvider extends NameResolverProvider {
@@ -51,6 +56,7 @@ public final class DnsNameResolverProvider extends NameResolverProvider {
 
   @Override
   public NameResolver newNameResolver(URI targetUri, NameResolver.Args args) {
+    // TODO(jdcormie): Remove once RFC 3986 migration is complete.
     if (SCHEME.equals(targetUri.getScheme())) {
       String targetPath = Preconditions.checkNotNull(targetUri.getPath(), "targetPath");
       Preconditions.checkArgument(targetPath.startsWith("/"),
@@ -68,6 +74,25 @@ public NameResolver newNameResolver(URI targetUri, NameResolver.Args args) {
     }
   }
 
+  @Override
+  public NameResolver newNameResolver(Uri targetUri, final NameResolver.Args args) {
+    if (SCHEME.equals(targetUri.getScheme())) {
+      List<String> pathSegments = targetUri.getPathSegments();
+      Preconditions.checkArgument(!pathSegments.isEmpty(),
+          "expected 1 path segment in target %s but found %s", targetUri, pathSegments);
+      String domainNameToResolve = pathSegments.get(0);
+      return new DnsNameResolver(
+          targetUri.getAuthority(),
+          domainNameToResolve,
+          args,
+          GrpcUtil.SHARED_CHANNEL_EXECUTOR,
+          Stopwatch.createUnstarted(),
+          IS_ANDROID);
+    } else {
+      return null;
+    }
+  }
+
   @Override
   public String getDefaultScheme() {
     return SCHEME;

core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java

@@ -16,25 +16,36 @@
 
 package io.grpc.internal;
 
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertSame;
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.TruthJUnit.assume;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.mock;
 
 import io.grpc.ChannelLogger;
 import io.grpc.NameResolver;
 import io.grpc.NameResolver.ServiceConfigParser;
 import io.grpc.SynchronizationContext;
+import io.grpc.Uri;
 import java.net.URI;
+import java.util.Arrays;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
 
 /** Unit tests for {@link DnsNameResolverProvider}. */
-@RunWith(JUnit4.class)
+@RunWith(Parameterized.class)
 public class DnsNameResolverProviderTest {
   private final FakeClock fakeClock = new FakeClock();
 
+  @Parameters(name = "enableRfc3986UrisParam={0}")
+  public static Iterable<Object[]> data() {
+    return Arrays.asList(new Object[][] {{true}, {false}});
+  }
+
+  @Parameter public boolean enableRfc3986UrisParam;
+
   private final SynchronizationContext syncContext = new SynchronizationContext(
       new Thread.UncaughtExceptionHandler() {
         @Override
@@ -59,10 +70,47 @@ public void isAvailable() {
   }
 
   @Test
-  public void newNameResolver() {
-    assertSame(DnsNameResolver.class,
-        provider.newNameResolver(URI.create("dns:///localhost:443"), args).getClass());
-    assertNull(
-        provider.newNameResolver(URI.create("notdns:///localhost:443"), args));
+  public void newNameResolver_acceptsHostAndPort() {
+    NameResolver nameResolver = newNameResolver("dns:///localhost:443", args);
+    assertThat(nameResolver).isNotNull();
+    assertThat(nameResolver.getClass()).isSameInstanceAs(DnsNameResolver.class);
+    assertThat(nameResolver.getServiceAuthority()).isEqualTo("localhost:443");
+  }
+
+  @Test
+  public void newNameResolver_acceptsRootless() {
+    assume().that(enableRfc3986UrisParam).isTrue();
+    NameResolver nameResolver = newNameResolver("dns:localhost:443", args);
+    assertThat(nameResolver).isNotNull();
+    assertThat(nameResolver.getClass()).isSameInstanceAs(DnsNameResolver.class);
+    assertThat(nameResolver.getServiceAuthority()).isEqualTo("localhost:443");
+  }
+
+  @Test
+  public void newNameResolver_rejectsNonDnsScheme() {
+    NameResolver nameResolver = newNameResolver("notdns:///localhost:443", args);
+    assertThat(nameResolver).isNull();
+  }
+
+  @Test
+  public void newNameResolver_toleratesTrailingPathSegments() {
+    NameResolver nameResolver = newNameResolver("dns:///foo.googleapis.com/ig/nor/ed", args);
+    assertThat(nameResolver).isNotNull();
+    assertThat(nameResolver.getClass()).isSameInstanceAs(DnsNameResolver.class);
+    assertThat(nameResolver.getServiceAuthority()).isEqualTo("foo.googleapis.com");
+  }
+
+  @Test
+  public void newNameResolver_toleratesAuthority() {
+    NameResolver nameResolver = newNameResolver("dns://8.8.8.8/foo.googleapis.com", args);
+    assertThat(nameResolver).isNotNull();
+    assertThat(nameResolver.getClass()).isSameInstanceAs(DnsNameResolver.class);
+    assertThat(nameResolver.getServiceAuthority()).isEqualTo("foo.googleapis.com");
+  }
+
+  private NameResolver newNameResolver(String uriString, NameResolver.Args args) {
+    return enableRfc3986UrisParam
+        ? provider.newNameResolver(Uri.create(uriString), args)
+        : provider.newNameResolver(URI.create(uriString), args);
   }
 }