Fix URI construction.

This fixes two issues.

1) Use the URI constructor with multiple arguments to construct the placeholder
URI for direct address, so that special characters can be correctly
escaped. This resolves #1883.

It requires the second fix.

2) Stop URI from mistreating paths as authorities.

There is a bug in URI constructors that take multiple arguments.  They simply
concatenate escaped components and try to parse the resulting string.

For example, URI("dns", null, "//127.0.0.1", null), which is what we do
internally when the application passes "/127.0.0.1" as the target, is supposed
to create a [scheme="dns", path="//127.0.0.1"].  Instead, it internally composes
"dns://127.0.0.1", which is parsed into [scheme="dns", authority="127.0.0.1"].

To avoid this issue, we pass empty string instead of null as the authority to
the URI constructor. The constructor would make "dns:////127.0.0.1" internally
which can be parsed correctly.

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

@@ -33,6 +33,7 @@
 
 import static com.google.common.base.MoreObjects.firstNonNull;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.util.concurrent.MoreExecutors;
 
@@ -49,6 +50,7 @@
 
 import java.net.SocketAddress;
 import java.net.URI;
+import java.net.URISyntaxException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -98,8 +100,23 @@ protected AbstractManagedChannelImplBuilder(String target) {
     this.directServerAddress = null;
   }
 
+  /**
+   * Returns a target string for the SocketAddress. It is only used as a placeholder, because
+   * DirectAddressNameResolverFactory will not actually try to use it. However, it must be a valid
+   * URI.
+   */
+  @VisibleForTesting
+  static String makeTargetStringForDirectAddress(SocketAddress address) {
+    try {
+      return new URI(DIRECT_ADDRESS_SCHEME, "", "/" + address, null).toString();
+    } catch (URISyntaxException e) {
+      // It should not happen.
+      throw new RuntimeException(e);
+    }
+  }
+
   protected AbstractManagedChannelImplBuilder(SocketAddress directServerAddress, String authority) {
-    this.target = DIRECT_ADDRESS_SCHEME + ":///" + directServerAddress;
+    this.target = makeTargetStringForDirectAddress(directServerAddress);
     this.directServerAddress = directServerAddress;
     this.nameResolverFactory = new DirectAddressNameResolverFactory(directServerAddress, authority);
   }

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

@@ -229,7 +229,7 @@ static NameResolver getNameResolver(String target, NameResolver.Factory nameReso
       // It doesn't look like a URI target. Maybe it's an authority string. Try with the default
       // scheme from the factory.
       try {
-        targetUri = new URI(nameResolverFactory.getDefaultScheme(), null, "/" + target, null);
+        targetUri = new URI(nameResolverFactory.getDefaultScheme(), "", "/" + target, null);
       } catch (URISyntaxException e) {
         // Should not be possible.
         throw new IllegalArgumentException(e);

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

@@ -58,34 +58,38 @@ public void invalidUriTarget() {
 
   @Test
   public void validTargetWithInvalidDnsName() throws Exception {
-    testValidTarget("[valid]", new URI("defaultscheme", null, "/[valid]", null));
+    testValidTarget("[valid]", "defaultscheme:///%5Bvalid%5D",
+        new URI("defaultscheme", "", "/[valid]", null));
   }
 
   @Test
   public void validAuthorityTarget() throws Exception {
-    testValidTarget("foo.googleapis.com:8080",
-        new URI("defaultscheme", null, "/foo.googleapis.com:8080", null));
+    testValidTarget("foo.googleapis.com:8080", "defaultscheme:///foo.googleapis.com:8080",
+        new URI("defaultscheme", "", "/foo.googleapis.com:8080", null));
   }
 
   @Test
   public void validUriTarget() throws Exception {
-    testValidTarget("scheme:///foo.googleapis.com:8080",
-        new URI("scheme", null, "/foo.googleapis.com:8080", null));
+    testValidTarget("scheme:///foo.googleapis.com:8080", "scheme:///foo.googleapis.com:8080",
+        new URI("scheme", "", "/foo.googleapis.com:8080", null));
   }
 
   @Test
   public void validIpv4AuthorityTarget() throws Exception {
-    testValidTarget("127.0.0.1:1234", new URI("defaultscheme", null, "/127.0.0.1:1234", null));
+    testValidTarget("127.0.0.1:1234", "defaultscheme:///127.0.0.1:1234",
+        new URI("defaultscheme", "", "/127.0.0.1:1234", null));
   }
 
   @Test
   public void validIpv4UriTarget() throws Exception {
-    testValidTarget("dns:///127.0.0.1:1234", new URI("dns", null, "/127.0.0.1:1234", null));
+    testValidTarget("dns:///127.0.0.1:1234", "dns:///127.0.0.1:1234",
+        new URI("dns", "", "/127.0.0.1:1234", null));
   }
 
   @Test
   public void validIpv6AuthorityTarget() throws Exception {
-    testValidTarget("[::1]:1234", new URI("defaultscheme", null, "/[::1]:1234", null));
+    testValidTarget("[::1]:1234", "defaultscheme:///%5B::1%5D:1234",
+        new URI("defaultscheme", "", "/[::1]:1234", null));
   }
 
   @Test
@@ -95,7 +99,14 @@ public void invalidIpv6UriTarget() throws Exception {
 
   @Test
   public void validIpv6UriTarget() throws Exception {
-    testValidTarget("dns:///%5B::1%5D:1234", new URI("dns", null, "/[::1]:1234", null));
+    testValidTarget("dns:///%5B::1%5D:1234", "dns:///%5B::1%5D:1234",
+        new URI("dns", "", "/[::1]:1234", null));
+  }
+
+  @Test
+  public void validTargetStartingWithSlash() throws Exception {
+    testValidTarget("/target", "defaultscheme:////target",
+        new URI("defaultscheme", "", "//target", null));
   }
 
   @Test
@@ -120,12 +131,13 @@ public String getDefaultScheme() {
     }
   }
 
-  private void testValidTarget(String target, URI expectedUri) {
+  private void testValidTarget(String target, String expectedUriString, URI expectedUri) {
     Factory nameResolverFactory = new FakeNameResolverFactory(expectedUri.getScheme());
     FakeNameResolver nameResolver = (FakeNameResolver) ManagedChannelImpl.getNameResolver(
         target, nameResolverFactory, NAME_RESOLVER_PARAMS);
     assertNotNull(nameResolver);
     assertEquals(expectedUri, nameResolver.uri);
+    assertEquals(expectedUriString, nameResolver.uri.toString());
   }
 
   private void testInvalidTarget(String target) {