core,okhttp,netty,alts,testing: Plumb proxy resolved addr to transports (#4137)

ProxyDetector is now responsible for resolving the proxy's
`InetSocketAddress`, and `ProxyParameters` asserts that the address is
resolved. The results are plumbed through using a `PairSocketAddress`,
which is a special `SocketAddress`.

If a proxy should be used but the proxy can not be resolved, we the
`DnsNameResolver` will re-attempt the resolution later.

Remove the unit test testing for unresolved proxy addresses, since
it's no longer applicable.

3 files changed, 26 insertions, 68 deletions

diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java
index e13e917f7..d74defcc8 100644
--- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java
+++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java
@@ -483,9 +483,7 @@ public class OkHttpChannelBuilder extends
           hostnameVerifier,
           Utils.convertSpec(connectionSpec),
           maxMessageSize,
-          proxy == null ? null : proxy.proxyAddress,
-          proxy == null ? null : proxy.username,
-          proxy == null ? null : proxy.password,
+          proxy,
           tooManyPingsRunnable,
           transportTracerFactory.create());
       if (enableKeepAlive) {
diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java
index ac3187948..5a0a70d10 100644
--- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java
+++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java
@@ -45,6 +45,7 @@ import io.grpc.internal.Http2Ping;
 import io.grpc.internal.KeepAliveManager;
 import io.grpc.internal.KeepAliveManager.ClientKeepAlivePinger;
 import io.grpc.internal.LogId;
+import io.grpc.internal.ProxyParameters;
 import io.grpc.internal.SerializingExecutor;
 import io.grpc.internal.SharedResourceHolder;
 import io.grpc.internal.StatsTraceContext;
@@ -177,25 +178,23 @@ class OkHttpClientTransport implements ConnectionClientTransport {
   private long keepAliveTimeNanos;
   private long keepAliveTimeoutNanos;
   private boolean keepAliveWithoutCalls;
-  @Nullable
-  private final InetSocketAddress proxyAddress;
-  @Nullable
-  private final String proxyUsername;
-  @Nullable
-  private final String proxyPassword;
   private final Runnable tooManyPingsRunnable;
   @GuardedBy("lock")
   private final TransportTracer transportTracer;
 
+  @VisibleForTesting
+  @Nullable
+  final ProxyParameters proxy;
+
   // The following fields should only be used for test.
   Runnable connectingCallback;
   SettableFuture<Void> connectedFuture;
 
+
   OkHttpClientTransport(InetSocketAddress address, String authority, @Nullable String userAgent,
       Executor executor, @Nullable SSLSocketFactory sslSocketFactory,
       @Nullable HostnameVerifier hostnameVerifier, ConnectionSpec connectionSpec,
-      int maxMessageSize, @Nullable InetSocketAddress proxyAddress, @Nullable String proxyUsername,
-      @Nullable String proxyPassword, Runnable tooManyPingsRunnable,
+      int maxMessageSize, @Nullable ProxyParameters proxy, Runnable tooManyPingsRunnable,
       TransportTracer transportTracer) {
     this.address = Preconditions.checkNotNull(address, "address");
     this.defaultAuthority = authority;
@@ -210,9 +209,7 @@ class OkHttpClientTransport implements ConnectionClientTransport {
     this.connectionSpec = Preconditions.checkNotNull(connectionSpec, "connectionSpec");
     this.stopwatchFactory = GrpcUtil.STOPWATCH_SUPPLIER;
     this.userAgent = GrpcUtil.getGrpcUserAgent("okhttp", userAgent);
-    this.proxyAddress = proxyAddress;
-    this.proxyUsername = proxyUsername;
-    this.proxyPassword = proxyPassword;
+    this.proxy = proxy;
     this.tooManyPingsRunnable =
         Preconditions.checkNotNull(tooManyPingsRunnable, "tooManyPingsRunnable");
     this.transportTracer = Preconditions.checkNotNull(transportTracer);
@@ -250,9 +247,7 @@ class OkHttpClientTransport implements ConnectionClientTransport {
     this.connectionSpec = null;
     this.connectingCallback = connectingCallback;
     this.connectedFuture = Preconditions.checkNotNull(connectedFuture, "connectedFuture");
-    this.proxyAddress = null;
-    this.proxyUsername = null;
-    this.proxyPassword = null;
+    this.proxy = null;
     this.tooManyPingsRunnable =
         Preconditions.checkNotNull(tooManyPingsRunnable, "tooManyPingsRunnable");
     this.transportTracer = Preconditions.checkNotNull(transportTracer, "transportTracer");
@@ -456,10 +451,11 @@ class OkHttpClientTransport implements ConnectionClientTransport {
         BufferedSink sink;
         Socket sock;
         try {
-          if (proxyAddress == null) {
+          if (proxy == null) {
             sock = new Socket(address.getAddress(), address.getPort());
           } else {
-            sock = createHttpProxySocket(address, proxyAddress, proxyUsername, proxyPassword);
+            sock = createHttpProxySocket(
+                address, proxy.proxyAddress, proxy.username, proxy.password);
           }
 
           if (sslSocketFactory != null) {
diff --git a/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java b/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java
index 1a323af40..b9d706177 100644
--- a/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java
+++ b/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java
@@ -68,6 +68,7 @@ import io.grpc.internal.ClientTransport;
 import io.grpc.internal.GrpcUtil;
 import io.grpc.internal.Instrumented;
 import io.grpc.internal.ManagedClientTransport;
+import io.grpc.internal.ProxyParameters;
 import io.grpc.internal.TransportTracer;
 import io.grpc.okhttp.OkHttpClientTransport.ClientFrameHandler;
 import io.grpc.okhttp.internal.ConnectionSpec;
@@ -124,6 +125,9 @@ public class OkHttpClientTransportTest {
   // The gRPC header length, which includes 1 byte compression flag and 4 bytes message length.
   private static final int HEADER_LENGTH = 5;
   private static final Status SHUTDOWN_REASON = Status.UNAVAILABLE.withDescription("for test");
+  private static final ProxyParameters NO_PROXY = null;
+  private static final String NO_USER = null;
+  private static final String NO_PW = null;
 
   @Rule public final Timeout globalTimeout = Timeout.seconds(10);
 
@@ -137,9 +141,6 @@ public class OkHttpClientTransportTest {
 
   private final SSLSocketFactory sslSocketFactory = null;
   private final HostnameVerifier hostnameVerifier = null;
-  private final InetSocketAddress proxyAddr = null;
-  private final String proxyUser = null;
-  private final String proxyPassword = null;
   private final TransportTracer transportTracer = new TransportTracer();
   private OkHttpClientTransport clientTransport;
   private MockFrameReader frameReader;
@@ -225,9 +226,7 @@ public class OkHttpClientTransportTest {
         hostnameVerifier,
         Utils.convertSpec(OkHttpChannelBuilder.DEFAULT_CONNECTION_SPEC),
         DEFAULT_MAX_MESSAGE_SIZE,
-        proxyAddr,
-        proxyUser,
-        proxyPassword,
+        NO_PROXY,
         tooManyPingsRunnable,
         transportTracer);
     String s = clientTransport.toString();
@@ -1450,9 +1449,7 @@ public class OkHttpClientTransportTest {
         hostnameVerifier,
         ConnectionSpec.CLEARTEXT,
         DEFAULT_MAX_MESSAGE_SIZE,
-        proxyAddr,
-        proxyUser,
-        proxyPassword,
+        NO_PROXY,
         tooManyPingsRunnable,
         transportTracer);
 
@@ -1474,9 +1471,7 @@ public class OkHttpClientTransportTest {
         hostnameVerifier,
         ConnectionSpec.CLEARTEXT,
         DEFAULT_MAX_MESSAGE_SIZE,
-        proxyAddr,
-        proxyUser,
-        proxyPassword,
+        NO_PROXY,
         tooManyPingsRunnable,
         new TransportTracer());
 
@@ -1506,9 +1501,8 @@ public class OkHttpClientTransportTest {
         hostnameVerifier,
         ConnectionSpec.CLEARTEXT,
         DEFAULT_MAX_MESSAGE_SIZE,
-        (InetSocketAddress) serverSocket.getLocalSocketAddress(),
-        proxyUser,
-        proxyPassword,
+        new ProxyParameters(
+            (InetSocketAddress) serverSocket.getLocalSocketAddress(), NO_USER, NO_PW),
         tooManyPingsRunnable,
         transportTracer);
     clientTransport.start(transportListener);
@@ -1557,9 +1551,8 @@ public class OkHttpClientTransportTest {
         hostnameVerifier,
         ConnectionSpec.CLEARTEXT,
         DEFAULT_MAX_MESSAGE_SIZE,
-        (InetSocketAddress) serverSocket.getLocalSocketAddress(),
-        proxyUser,
-        proxyPassword,
+        new ProxyParameters(
+            (InetSocketAddress) serverSocket.getLocalSocketAddress(), NO_USER, NO_PW),
         tooManyPingsRunnable,
         transportTracer);
     clientTransport.start(transportListener);
@@ -1607,9 +1600,8 @@ public class OkHttpClientTransportTest {
         hostnameVerifier,
         ConnectionSpec.CLEARTEXT,
         DEFAULT_MAX_MESSAGE_SIZE,
-        (InetSocketAddress) serverSocket.getLocalSocketAddress(),
-        proxyUser,
-        proxyPassword,
+        new ProxyParameters(
+            (InetSocketAddress) serverSocket.getLocalSocketAddress(), NO_USER, NO_PW),
         tooManyPingsRunnable,
         transportTracer);
     clientTransport.start(transportListener);
@@ -1629,34 +1621,6 @@ public class OkHttpClientTransportTest {
   }
 
   @Test
-  public void proxy_unresolvedProxyAddress() throws Exception {
-    clientTransport = new OkHttpClientTransport(
-        InetSocketAddress.createUnresolved("theservice", 80),
-        "authority",
-        "userAgent",
-        executor,
-        sslSocketFactory,
-        hostnameVerifier,
-        ConnectionSpec.CLEARTEXT,
-        DEFAULT_MAX_MESSAGE_SIZE,
-        InetSocketAddress.createUnresolved("unresolvedproxy", 80),
-        proxyUser,
-        proxyPassword,
-        tooManyPingsRunnable,
-        transportTracer);
-    clientTransport.start(transportListener);
-
-    ArgumentCaptor<Status> captor = ArgumentCaptor.forClass(Status.class);
-    verify(transportListener, timeout(TIME_OUT_MS)).transportShutdown(captor.capture());
-    Status error = captor.getValue();
-    assertTrue("Status didn't contain proxy: " + captor.getValue(),
-        error.getDescription().contains("proxy"));
-    assertEquals("Not UNAVAILABLE: " + captor.getValue(),
-        Status.UNAVAILABLE.getCode(), error.getCode());
-    verify(transportListener, timeout(TIME_OUT_MS)).transportTerminated();
-  }
-
-  @Test
   public void goAway_notUtf8() throws Exception {
     initTransport();
     // 0xFF is never permitted in UTF-8. 0xF0 should have 3 continuations following, and 0x0a isn't