From 65e4d9f47abdad2a0a7a67fc5f1002bcf6f6fab1 Mon Sep 17 00:00:00 2001
From: Eric Anderson <ejona@google.com>
Date: Tue, 24 Jan 2017 11:05:21 -0800
Subject: [PATCH] all: avoid DNS with GRPC_PROXY_EXP

In some environments DNS is not available and is performed by the
CONNECT proxy. Nothing "special" should need to be done for these
environments, but the previous support took shortcuts which knowingly
would not support such environments.

This change should fix both OkHttp and Netty. Netty's
Bootstrap.connect() resolved the name immediately whereas using
ChannelPipeline.connect() waits until the address reaches the end of the
pipeline. Netty uses NetUtil.toSocketAddressString() to get the name of
the address, which uses InetSocketAddress.getHostString() when
available.

OkHttp is still using InetSocketAddress.getHostName() which may issue
reverse DNS lookups. However, if the reverse DNS lookup fails, it should
convert the IP to a textual string like getHostString(). So as long as
the reverse DNS maps to the same machine as the IP, there should only be
performance concerns, not correctness issues. Since the DnsNameResolver
is creating unresolved addresses, the reverse DNS lookups shouldn't
occur in the common case.
---
 .../java/io/grpc/internal/DnsNameResolver.java   |  9 +++++++++
 .../testing/integration/TestServiceClient.java   | 16 ++--------------
 .../java/io/grpc/netty/NettyClientTransport.java |  5 +++--
 3 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/core/src/main/java/io/grpc/internal/DnsNameResolver.java b/core/src/main/java/io/grpc/internal/DnsNameResolver.java
index 15f3685f619..c2dfa41e41c 100644
--- a/core/src/main/java/io/grpc/internal/DnsNameResolver.java
+++ b/core/src/main/java/io/grpc/internal/DnsNameResolver.java
@@ -140,6 +140,15 @@ public void run() {
           resolving = true;
         }
         try {
+          if (System.getenv("GRPC_PROXY_EXP") != null) {
+            ResolvedServerInfoGroup servers = ResolvedServerInfoGroup.builder()
+                .add(new ResolvedServerInfo(
+                    InetSocketAddress.createUnresolved(host, port), Attributes.EMPTY))
+                .build();
+            savedListener.onUpdate(Collections.singletonList(servers), Attributes.EMPTY);
+            return;
+          }
+
           try {
             inetAddrs = getAllByName(host);
           } catch (UnknownHostException e) {
diff --git a/interop-testing/src/main/java/io/grpc/testing/integration/TestServiceClient.java b/interop-testing/src/main/java/io/grpc/testing/integration/TestServiceClient.java
index e9f2e7d8f80..85f89e8c10c 100644
--- a/interop-testing/src/main/java/io/grpc/testing/integration/TestServiceClient.java
+++ b/interop-testing/src/main/java/io/grpc/testing/integration/TestServiceClient.java
@@ -43,9 +43,6 @@
 import io.netty.handler.ssl.SslContext;
 import java.io.File;
 import java.io.FileInputStream;
-import java.net.InetAddress;
-import java.net.InetSocketAddress;
-import java.net.UnknownHostException;
 import java.nio.charset.Charset;
 import javax.net.ssl.SSLSocketFactory;
 
@@ -304,16 +301,6 @@ private class Tester extends AbstractInteropTest {
     @Override
     protected ManagedChannel createChannel() {
       if (!useOkHttp) {
-        InetAddress address;
-        try {
-          address = InetAddress.getByName(serverHost);
-          if (serverHostOverride != null) {
-            // Force the hostname to match the cert the server uses.
-            address = InetAddress.getByAddress(serverHostOverride, address.getAddress());
-          }
-        } catch (UnknownHostException ex) {
-          throw new RuntimeException(ex);
-        }
         SslContext sslContext = null;
         if (useTestCa) {
           try {
@@ -323,7 +310,8 @@ protected ManagedChannel createChannel() {
             throw new RuntimeException(ex);
           }
         }
-        return NettyChannelBuilder.forAddress(new InetSocketAddress(address, serverPort))
+        return NettyChannelBuilder.forAddress(serverHost, serverPort)
+            .overrideAuthority(serverHostOverride)
             .flowControlWindow(65 * 1024)
             .negotiationType(useTls ? NegotiationType.TLS : NegotiationType.PLAINTEXT)
             .sslContext(sslContext)
diff --git a/netty/src/main/java/io/grpc/netty/NettyClientTransport.java b/netty/src/main/java/io/grpc/netty/NettyClientTransport.java
index ce063f6c946..d745409a0b7 100644
--- a/netty/src/main/java/io/grpc/netty/NettyClientTransport.java
+++ b/netty/src/main/java/io/grpc/netty/NettyClientTransport.java
@@ -194,8 +194,9 @@ public Runnable start(Listener transportListener) {
      * that it may begin buffering writes.
      */
     b.handler(negotiationHandler);
+    channel = b.register().channel();
     // Start the connection operation to the server.
-    channel = b.connect(address).addListener(new ChannelFutureListener() {
+    channel.connect(address).addListener(new ChannelFutureListener() {
       @Override
       public void operationComplete(ChannelFuture future) throws Exception {
         if (!future.isSuccess()) {
@@ -209,7 +210,7 @@ public void operationComplete(ChannelFuture future) throws Exception {
           future.channel().pipeline().fireExceptionCaught(future.cause());
         }
       }
-    }).channel();
+    });
     // Start the write queue as soon as the channel is constructed
     handler.startWriteQueue(channel);
     // This write will have no effect, yet it will only complete once the negotiationHandler