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.

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

@@ -396,7 +396,6 @@ public abstract class AbstractManagedChannelImplBuilder
         SharedResourcePool.forResource(GrpcUtil.SHARED_CHANNEL_EXECUTOR),
         GrpcUtil.STOPWATCH_SUPPLIER,
         getEffectiveInterceptors(),
-        GrpcUtil.getProxyDetector(),
         CallTracer.getDefaultFactory());
   }
 

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

@@ -26,6 +26,7 @@ import io.grpc.EquivalentAddressGroup;
 import io.grpc.NameResolver;
 import io.grpc.Status;
 import io.grpc.internal.SharedResourceHolder.Resource;
+import java.io.IOException;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
@@ -71,13 +72,15 @@ final class DnsNameResolver extends NameResolver {
   @VisibleForTesting
   static boolean enableJndi = Boolean.parseBoolean(JNDI_PROPERTY);
 
+  @VisibleForTesting
+  final ProxyDetector proxyDetector;
+
   private DelegateResolver delegateResolver = pickDelegateResolver();
 
   private final String authority;
   private final String host;
   private final int port;
   private final Resource<ExecutorService> executorResource;
-  private final ProxyDetector proxyDetector;
   @GuardedBy("this")
   private boolean shutdown;
   @GuardedBy("this")
@@ -145,9 +148,23 @@ final class DnsNameResolver extends NameResolver {
         }
         try {
           InetSocketAddress destination = InetSocketAddress.createUnresolved(host, port);
-          ProxyParameters proxy = proxyDetector.proxyFor(destination);
+          ProxyParameters proxy;
+          try {
+            proxy = proxyDetector.proxyFor(destination);
+          } catch (IOException e) {
+            savedListener.onError(
+                Status.UNAVAILABLE.withDescription("Unable to resolve host " + host).withCause(e));
+            return;
+          }
           if (proxy != null) {
-            EquivalentAddressGroup server = new EquivalentAddressGroup(destination);
+            EquivalentAddressGroup server =
+                new EquivalentAddressGroup(
+                    new PairSocketAddress(
+                        destination,
+                        Attributes
+                            .newBuilder()
+                            .set(ProxyDetector.PROXY_PARAMS_KEY, proxy)
+                            .build()));
             savedListener.onAddresses(Collections.singletonList(server), Attributes.EMPTY);
             return;
           }

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

@@ -52,7 +52,7 @@ public final class DnsNameResolverProvider extends NameResolverProvider {
           name,
           params,
           GrpcUtil.SHARED_CHANNEL_EXECUTOR,
-          GrpcUtil.getProxyDetector());
+          GrpcUtil.getDefaultProxyDetector());
     } else {
       return null;
     }

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

@@ -249,7 +249,7 @@ public final class GrpcUtil {
   /**
    * Returns a proxy detector appropriate for the current environment.
    */
-  public static ProxyDetector getProxyDetector() {
+  public static ProxyDetector getDefaultProxyDetector() {
     if (IS_RESTRICTED_APPENGINE) {
       return NOOP_PROXY_DETECTOR;
     } else {

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

@@ -150,8 +150,6 @@ final class InternalSubchannel implements Instrumented<ChannelStats> {
   @GuardedBy("lock")
   private ConnectivityStateInfo state = ConnectivityStateInfo.forNonError(IDLE);
 
-  private final ProxyDetector proxyDetector;
-
   @GuardedBy("lock")
   private Status shutdownReason;
 
@@ -159,7 +157,7 @@ final class InternalSubchannel implements Instrumented<ChannelStats> {
       BackoffPolicy.Provider backoffPolicyProvider,
       ClientTransportFactory transportFactory, ScheduledExecutorService scheduledExecutor,
       Supplier<Stopwatch> stopwatchSupplier, ChannelExecutor channelExecutor, Callback callback,
-      ProxyDetector proxyDetector, Channelz channelz, CallTracer callsTracer) {
+      Channelz channelz, CallTracer callsTracer) {
     this.addressGroup = Preconditions.checkNotNull(addressGroup, "addressGroup");
     this.authority = authority;
     this.userAgent = userAgent;
@@ -169,7 +167,6 @@ final class InternalSubchannel implements Instrumented<ChannelStats> {
     this.connectingTimer = stopwatchSupplier.get();
     this.channelExecutor = channelExecutor;
     this.callback = callback;
-    this.proxyDetector = proxyDetector;
     this.channelz = channelz;
     this.callsTracer = callsTracer;
   }
@@ -211,9 +208,14 @@ final class InternalSubchannel implements Instrumented<ChannelStats> {
       connectingTimer.reset().start();
     }
     List<SocketAddress> addrs = addressGroup.getAddresses();
-    final SocketAddress address = addrs.get(addressIndex);
+    SocketAddress address = addrs.get(addressIndex);
+
+    ProxyParameters proxy = null;
+    if (address instanceof PairSocketAddress) {
+      proxy = ((PairSocketAddress) address).getAttributes().get(ProxyDetector.PROXY_PARAMS_KEY);
+      address = ((PairSocketAddress) address).getAddress();
+    }
 
-    ProxyParameters proxy = proxyDetector.proxyFor(address);
     ConnectionClientTransport transport =
         new CallTracingTransport(
             transportFactory.newClientTransport(address, authority, userAgent, proxy),

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

@@ -151,8 +151,6 @@ final class ManagedChannelImpl extends ManagedChannel implements Instrumented<Ch
   // Only null after channel is terminated. Must be assigned from the channelExecutor.
   private NameResolver nameResolver;
 
-  private final ProxyDetector proxyDetector;
-
   // Must be accessed from the channelExecutor.
   private boolean nameResolverStarted;
 
@@ -548,7 +546,6 @@ final class ManagedChannelImpl extends ManagedChannel implements Instrumented<Ch
       ObjectPool<? extends Executor> oobExecutorPool,
       Supplier<Stopwatch> stopwatchSupplier,
       List<ClientInterceptor> interceptors,
-      ProxyDetector proxyDetector,
       CallTracer.Factory callTracerFactory) {
     this.target = checkNotNull(builder.target, "target");
     this.nameResolverFactory = builder.getNameResolverFactory();
@@ -583,7 +580,6 @@ final class ManagedChannelImpl extends ManagedChannel implements Instrumented<Ch
     this.decompressorRegistry = checkNotNull(builder.decompressorRegistry, "decompressorRegistry");
     this.compressorRegistry = checkNotNull(builder.compressorRegistry, "compressorRegistry");
     this.userAgent = builder.userAgent;
-    this.proxyDetector = proxyDetector;
 
     this.maxRetryAttempts = builder.maxRetryAttempts;
     this.maxHedgedAttempts = builder.maxHedgedAttempts;
@@ -1015,7 +1011,6 @@ final class ManagedChannelImpl extends ManagedChannel implements Instrumented<Ch
                 inUseStateAggregator.updateObjectInUse(is, false);
               }
             },
-            proxyDetector,
             channelz,
             callTracerFactory.create());
       channelz.addSubchannel(internalSubchannel);
@@ -1100,7 +1095,6 @@ final class ManagedChannelImpl extends ManagedChannel implements Instrumented<Ch
               oobChannel.handleSubchannelStateChange(newState);
             }
           },
-          proxyDetector,
           channelz,
           callTracerFactory.create());
       channelz.addSubchannel(oobChannel);

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

@@ -0,0 +1,46 @@
+/*
+ * Copyright 2018, gRPC Authors All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.grpc.internal;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
+import io.grpc.Attributes;
+import java.net.SocketAddress;
+
+/**
+ * A data structure to associate a {@link SocketAddress} with {@link Attributes}.
+ */
+final class PairSocketAddress extends SocketAddress {
+  private static final long serialVersionUID = -6854992294603212793L;
+
+  private final SocketAddress address;
+  private final Attributes attributes;
+
+  @VisibleForTesting
+  PairSocketAddress(SocketAddress address, Attributes attributes) {
+    this.address = Preconditions.checkNotNull(address);
+    this.attributes = Preconditions.checkNotNull(attributes);
+  }
+
+  public Attributes getAttributes() {
+    return attributes;
+  }
+
+  public SocketAddress getAddress() {
+    return address;
+  }
+}

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

@@ -16,18 +16,24 @@
 
 package io.grpc.internal;
 
+import io.grpc.Attributes;
+import java.io.IOException;
 import java.net.SocketAddress;
 import javax.annotation.Nullable;
 
 /**
  * A utility class to detect which proxy, if any, should be used for a given
- * {@link java.net.SocketAddress}.
+ * {@link java.net.SocketAddress}. This class performs network requests to resolve address names,
+ * and should only be used in places that are expected to do IO such as the
+ * {@link io.grpc.NameResolver}.
  */
 public interface ProxyDetector {
+  Attributes.Key<ProxyParameters> PROXY_PARAMS_KEY = Attributes.Key.of("proxy-params-key");
   /**
    * Given a target address, returns which proxy address should be used. If no proxy should be
-   * used, then return value will be null.
+   * used, then return value will be null. The address of the {@link ProxyParameters} is always
+   * resolved. This throws if the proxy address cannot be resolved.
    */
   @Nullable
-  ProxyParameters proxyFor(SocketAddress targetServerAddress);
+  ProxyParameters proxyFor(SocketAddress targetServerAddress) throws IOException;
 }

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

@@ -20,6 +20,7 @@ import static com.google.common.base.Preconditions.checkNotNull;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Supplier;
+import java.io.IOException;
 import java.net.Authenticator;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
@@ -108,7 +109,7 @@ class ProxyDetectorImpl implements ProxyDetector {
 
   @Nullable
   @Override
-  public ProxyParameters proxyFor(SocketAddress targetServerAddress) {
+  public ProxyParameters proxyFor(SocketAddress targetServerAddress) throws IOException {
     if (override != null) {
       return override;
     }
@@ -118,7 +119,7 @@ class ProxyDetectorImpl implements ProxyDetector {
     return detectProxy((InetSocketAddress) targetServerAddress);
   }
 
-  private ProxyParameters detectProxy(InetSocketAddress targetAddr) {
+  private ProxyParameters detectProxy(InetSocketAddress targetAddr) throws IOException {
     URI uri;
     String host;
     try {
@@ -167,12 +168,21 @@ class ProxyDetectorImpl implements ProxyDetector {
         promptString,
         null);
 
+    final InetSocketAddress resolvedProxyAddr;
+    if (proxyAddr.isUnresolved()) {
+      InetAddress resolvedAddress = InetAddress.getByName(proxyAddr.getHostName());
+      resolvedProxyAddr = new InetSocketAddress(resolvedAddress, proxyAddr.getPort());
+    } else {
+      resolvedProxyAddr = proxyAddr;
+    }
+
     if (auth == null) {
-      return new ProxyParameters(proxyAddr, null, null);
+      return new ProxyParameters(resolvedProxyAddr, null, null);
     }
 
     // TODO(spencerfang): users of ProxyParameters should clear the password when done
-    return new ProxyParameters(proxyAddr, auth.getUserName(), new String(auth.getPassword()));
+    return new ProxyParameters(
+        resolvedProxyAddr, auth.getUserName(), new String(auth.getPassword()));
   }
 
   /**

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

@@ -29,11 +29,16 @@ public final class ProxyParameters {
   @Nullable public final String username;
   @Nullable public final String password;
 
-  ProxyParameters(
+  /** Creates an instance. */
+  public ProxyParameters(
       InetSocketAddress proxyAddress,
       @Nullable String username,
       @Nullable String password) {
-    this.proxyAddress = Preconditions.checkNotNull(proxyAddress);
+    Preconditions.checkNotNull(proxyAddress);
+    // The resolution must be done by the ProxyParameters producer, because consumers
+    // may not be allowed to do IO.
+    Preconditions.checkState(!proxyAddress.isUnresolved());
+    this.proxyAddress = proxyAddress;
     this.username = username;
     this.password = password;
   }

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

@@ -19,6 +19,7 @@ package io.grpc.internal;
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.Matchers.any;
@@ -247,7 +248,7 @@ public class DnsNameResolverTest {
     final int port = 81;
     ProxyDetector alwaysDetectProxy = mock(ProxyDetector.class);
     ProxyParameters proxyParameters = new ProxyParameters(
-        InetSocketAddress.createUnresolved("proxy.example.com", 1000),
+        new InetSocketAddress(InetAddress.getByName("10.0.0.1"), 1000),
         "username",
         "password");
     when(alwaysDetectProxy.proxyFor(any(SocketAddress.class)))
@@ -263,8 +264,10 @@ public class DnsNameResolverTest {
     assertThat(result).hasSize(1);
     EquivalentAddressGroup eag = result.get(0);
     assertThat(eag.getAddresses()).hasSize(1);
-    SocketAddress socketAddress = eag.getAddresses().get(0);
-    assertTrue(((InetSocketAddress) socketAddress).isUnresolved());
+
+    PairSocketAddress socketAddress = (PairSocketAddress) eag.getAddresses().get(0);
+    assertSame(proxyParameters, socketAddress.getAttributes().get(ProxyDetector.PROXY_PARAMS_KEY));
+    assertTrue(((InetSocketAddress) socketAddress.getAddress()).isUnresolved());
   }
 
   private void testInvalidUri(URI uri) {

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

@@ -28,7 +28,6 @@ import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
-import static org.mockito.Matchers.eq;
 import static org.mockito.Matchers.same;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
@@ -37,19 +36,16 @@ import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
 
-import io.grpc.ConnectivityState;
 import io.grpc.ConnectivityStateInfo;
 import io.grpc.EquivalentAddressGroup;
 import io.grpc.Status;
 import io.grpc.internal.InternalSubchannel.CallTracingTransport;
 import io.grpc.internal.TestUtils.MockClientTransportInfo;
-import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.util.Arrays;
 import java.util.LinkedList;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.atomic.AtomicInteger;
-import javax.annotation.Nullable;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -865,32 +861,6 @@ public class InternalSubchannelTest {
     assertEquals(3, runnableInvokes.get());
   }
 
-  @Test
-  public void proxyTest() {
-    final SocketAddress addr1 = mock(SocketAddress.class);
-    final ProxyParameters proxy = new ProxyParameters(
-        InetSocketAddress.createUnresolved("proxy.example.com", 1000), "username", "password");
-    ProxyDetector proxyDetector = new ProxyDetector() {
-      @Nullable
-      @Override
-      public ProxyParameters proxyFor(SocketAddress targetServerAddress) {
-        if (targetServerAddress == addr1) {
-          return proxy;
-        } else {
-          return null;
-        }
-      }
-    };
-    createInternalSubChannelWithProxy(proxyDetector, addr1);
-    assertEquals(ConnectivityState.IDLE, internalSubchannel.getState());
-    assertNoCallbackInvoke();
-    assertNull(internalSubchannel.obtainActiveTransport());
-    assertExactCallbackInvokes("onStateChange:CONNECTING");
-    assertEquals(ConnectivityState.CONNECTING, internalSubchannel.getState());
-    verify(mockTransportFactory).newClientTransport(
-        eq(addr1), eq(AUTHORITY), eq(USER_AGENT), eq(proxy));
-  }
-
   @Test
   public void resetConnectBackoff() throws Exception {
     SocketAddress addr = mock(SocketAddress.class);
@@ -962,16 +932,11 @@ public class InternalSubchannelTest {
   }
 
   private void createInternalSubchannel(SocketAddress ... addrs) {
-    createInternalSubChannelWithProxy(GrpcUtil.NOOP_PROXY_DETECTOR, addrs);
-  }
-
-  private void createInternalSubChannelWithProxy(
-      ProxyDetector proxyDetector, SocketAddress ... addrs) {
     addressGroup = new EquivalentAddressGroup(Arrays.asList(addrs));
     internalSubchannel = new InternalSubchannel(addressGroup, AUTHORITY, USER_AGENT,
         mockBackoffPolicyProvider, mockTransportFactory, fakeClock.getScheduledExecutorService(),
         fakeClock.getStopwatchSupplier(), channelExecutor, mockInternalSubchannelCallback,
-        proxyDetector, channelz, CallTracer.getDefaultFactory().create());
+        channelz, CallTracer.getDefaultFactory().create());
   }
 
   private void assertNoCallbackInvoke() {

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

@@ -143,7 +143,7 @@ public class ManagedChannelImplIdlenessTest {
         builder, mockTransportFactory, new FakeBackoffPolicyProvider(),
         oobExecutorPool, timer.getStopwatchSupplier(),
         Collections.<ClientInterceptor>emptyList(),
-        GrpcUtil.NOOP_PROXY_DETECTOR, CallTracer.getDefaultFactory());
+        CallTracer.getDefaultFactory());
     newTransports = TestUtils.captureTransports(mockTransportFactory);
 
     for (int i = 0; i < 2; i++) {

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

@@ -242,7 +242,7 @@ public class ManagedChannelImplTest {
     checkState(channel == null);
     channel = new ManagedChannelImpl(
         builder, mockTransportFactory, new FakeBackoffPolicyProvider(),
-        oobExecutorPool, timer.getStopwatchSupplier(), interceptors, GrpcUtil.NOOP_PROXY_DETECTOR,
+        oobExecutorPool, timer.getStopwatchSupplier(), interceptors,
         channelStatsFactory);
 
     if (requestConnection) {

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

@@ -18,8 +18,10 @@ package io.grpc.internal;
 
 import static junit.framework.TestCase.assertFalse;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -42,18 +44,18 @@ import org.mockito.MockitoAnnotations;
 
 @RunWith(JUnit4.class)
 public class ProxyDetectorImplTest {
-  private InetSocketAddress destination = InetSocketAddress.createUnresolved(
-      "destination",
-      5678
-  );
-
+  private static final String NO_USER = null;
+  private static final String NO_PW = null;
   @Mock private ProxySelector proxySelector;
   @Mock private ProxyDetectorImpl.AuthenticationProvider authenticator;
+  private InetSocketAddress destination = InetSocketAddress.createUnresolved("10.10.10.10", 5678);
   private Supplier<ProxySelector> proxySelectorSupplier;
   private ProxyDetector proxyDetector;
+  private InetSocketAddress unresolvedProxy;
+  private ProxyParameters proxyParmeters;
 
   @Before
-  public void setUp() {
+  public void setUp() throws Exception {
     MockitoAnnotations.initMocks(this);
     proxySelectorSupplier = new Supplier<ProxySelector>() {
       @Override
@@ -62,11 +64,17 @@ public class ProxyDetectorImplTest {
       }
     };
     proxyDetector = new ProxyDetectorImpl(proxySelectorSupplier, authenticator, null);
+    int proxyPort = 1234;
+    unresolvedProxy = InetSocketAddress.createUnresolved("10.0.0.1", proxyPort);
+    proxyParmeters = new ProxyParameters(
+        new InetSocketAddress(InetAddress.getByName(unresolvedProxy.getHostName()), proxyPort),
+        NO_USER,
+        NO_PW);
   }
 
   @Test
   public void override_hostPort() throws Exception {
-    final String overrideHost = "override";
+    final String overrideHost = "10.99.99.99";
     final int overridePort = 1234;
     final String overrideHostWithPort = overrideHost + ":" + overridePort;
     ProxyDetectorImpl proxyDetector = new ProxyDetectorImpl(
@@ -77,13 +85,15 @@ public class ProxyDetectorImplTest {
     assertNotNull(detected);
     assertEquals(
         new ProxyParameters(
-            InetSocketAddress.createUnresolved(overrideHost, overridePort), null, null),
+            new InetSocketAddress(InetAddress.getByName(overrideHost), overridePort),
+            NO_USER,
+            NO_PW),
         detected);
   }
 
   @Test
   public void override_hostOnly() throws Exception {
-    final String overrideHostWithoutPort = "override";
+    final String overrideHostWithoutPort = "10.99.99.99";
     final int defaultPort = 80;
     ProxyDetectorImpl proxyDetector = new ProxyDetectorImpl(
         proxySelectorSupplier,
@@ -93,7 +103,10 @@ public class ProxyDetectorImplTest {
     assertNotNull(detected);
     assertEquals(
         new ProxyParameters(
-            InetSocketAddress.createUnresolved(overrideHostWithoutPort, defaultPort), null, null),
+            new InetSocketAddress(
+                InetAddress.getByName(overrideHostWithoutPort), defaultPort),
+            NO_USER,
+            NO_PW),
         detected);
   }
 
@@ -105,43 +118,50 @@ public class ProxyDetectorImplTest {
   }
 
   @Test
-  public void detectProxyForUnresolved() throws Exception {
-    final InetSocketAddress proxyAddress = InetSocketAddress.createUnresolved("proxy", 1234);
-    Proxy proxy = new Proxy(Proxy.Type.HTTP, proxyAddress);
+  public void detectProxyForUnresolvedDestination() throws Exception {
+    Proxy proxy = new Proxy(Proxy.Type.HTTP, unresolvedProxy);
     when(proxySelector.select(any(URI.class))).thenReturn(ImmutableList.of(proxy));
 
     ProxyParameters detected = proxyDetector.proxyFor(destination);
     assertNotNull(detected);
-    assertEquals(new ProxyParameters(proxyAddress, null, null), detected);
+    assertEquals(proxyParmeters, detected);
   }
 
   @Test
-  public void detectProxyForResolved() throws Exception {
-    InetSocketAddress resolved =
-        new InetSocketAddress(InetAddress.getByAddress(new byte[]{10, 0, 0, 1}), 10);
+  public void detectProxyForResolvedDestination() throws Exception {
+    InetSocketAddress resolved = new InetSocketAddress(InetAddress.getByName("10.1.2.3"), 10);
     assertFalse(resolved.isUnresolved());
     destination = resolved;
 
-    final InetSocketAddress proxyAddress = InetSocketAddress.createUnresolved("proxy", 1234);
-    Proxy proxy = new Proxy(Proxy.Type.HTTP, proxyAddress);
+    Proxy proxy = new Proxy(Proxy.Type.HTTP, unresolvedProxy);
     when(proxySelector.select(any(URI.class))).thenReturn(ImmutableList.of(proxy));
 
     ProxyParameters detected = proxyDetector.proxyFor(destination);
     assertNotNull(detected);
-    assertEquals(new ProxyParameters(proxyAddress, null, null), detected);
+    assertEquals(proxyParmeters, detected);
+  }
+
+  @Test
+  public void unresolvedProxyAddressBecomesResolved() throws Exception {
+    InetSocketAddress unresolvedProxy = InetSocketAddress.createUnresolved("10.0.0.100", 1234);
+    assertTrue(unresolvedProxy.isUnresolved());
+    Proxy proxy1 = new java.net.Proxy(java.net.Proxy.Type.HTTP, unresolvedProxy);
+    when(proxySelector.select(any(URI.class))).thenReturn(ImmutableList.of(proxy1));
+    ProxyParameters proxy = proxyDetector.proxyFor(destination);
+    assertFalse(proxy.proxyAddress.isUnresolved());
   }
 
   @Test
   public void pickFirstHttpProxy() throws Exception {
-    final InetSocketAddress proxyAddress = InetSocketAddress.createUnresolved("proxy1", 1111);
-    InetSocketAddress otherProxy = InetSocketAddress.createUnresolved("proxy2", 2222);
-    Proxy proxy1 = new java.net.Proxy(java.net.Proxy.Type.HTTP, proxyAddress);
+    InetSocketAddress otherProxy = InetSocketAddress.createUnresolved("10.0.0.2", 11111);
+    assertNotEquals(unresolvedProxy, otherProxy);
+    Proxy proxy1 = new java.net.Proxy(java.net.Proxy.Type.HTTP, unresolvedProxy);
     Proxy proxy2 = new java.net.Proxy(java.net.Proxy.Type.HTTP, otherProxy);
     when(proxySelector.select(any(URI.class))).thenReturn(ImmutableList.of(proxy1, proxy2));
 
     ProxyParameters detected = proxyDetector.proxyFor(destination);
     assertNotNull(detected);
-    assertEquals(new ProxyParameters(proxyAddress, null, null), detected);
+    assertEquals(proxyParmeters, detected);
   }
 
   // Mainly for InProcessSocketAddress
@@ -152,10 +172,7 @@ public class ProxyDetectorImplTest {
 
   @Test
   public void authRequired() throws Exception {
-    final String proxyHost = "proxyhost";
-    final int proxyPort = 1234;
-    final InetSocketAddress proxyAddress = InetSocketAddress.createUnresolved(proxyHost, proxyPort);
-    Proxy proxy = new java.net.Proxy(java.net.Proxy.Type.HTTP, proxyAddress);
+    Proxy proxy = new java.net.Proxy(java.net.Proxy.Type.HTTP, unresolvedProxy);
     final String proxyUser = "testuser";
     final String proxyPassword = "testpassword";
     PasswordAuthentication auth = new PasswordAuthentication(
@@ -171,7 +188,13 @@ public class ProxyDetectorImplTest {
     when(proxySelector.select(any(URI.class))).thenReturn(ImmutableList.of(proxy));
 
     ProxyParameters detected = proxyDetector.proxyFor(destination);
-    assertNotNull(detected);
-    assertEquals(new ProxyParameters(proxyAddress, proxyUser, proxyPassword), detected);
+    assertEquals(
+        new ProxyParameters(
+            new InetSocketAddress(
+                InetAddress.getByName(unresolvedProxy.getHostName()),
+                unresolvedProxy.getPort()),
+            proxyUser,
+            proxyPassword),
+        detected);
   }
 }

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) {

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) {

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);
@@ -1628,34 +1620,6 @@ public class OkHttpClientTransportTest {
     verify(transportListener, timeout(TIME_OUT_MS)).transportTerminated();
   }
 
-  @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();