diff --git a/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java index a5279cc0c8..b960dbd0b3 100644 --- a/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java +++ b/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()); } diff --git a/core/src/main/java/io/grpc/internal/DnsNameResolver.java b/core/src/main/java/io/grpc/internal/DnsNameResolver.java index e4c68cfd02..1588388dd3 100644 --- a/core/src/main/java/io/grpc/internal/DnsNameResolver.java +++ b/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 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; } diff --git a/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java b/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java index 9ee23e2993..3184a33cea 100644 --- a/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java +++ b/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; } diff --git a/core/src/main/java/io/grpc/internal/GrpcUtil.java b/core/src/main/java/io/grpc/internal/GrpcUtil.java index 7248b74781..6bd8d43388 100644 --- a/core/src/main/java/io/grpc/internal/GrpcUtil.java +++ b/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 { diff --git a/core/src/main/java/io/grpc/internal/InternalSubchannel.java b/core/src/main/java/io/grpc/internal/InternalSubchannel.java index 4bfd9ca2ce..baefc81c5d 100644 --- a/core/src/main/java/io/grpc/internal/InternalSubchannel.java +++ b/core/src/main/java/io/grpc/internal/InternalSubchannel.java @@ -150,8 +150,6 @@ final class InternalSubchannel implements Instrumented { @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 { BackoffPolicy.Provider backoffPolicyProvider, ClientTransportFactory transportFactory, ScheduledExecutorService scheduledExecutor, Supplier 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 { 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 { connectingTimer.reset().start(); } List 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), diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java index 0e49db3eca..10fe727910 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java @@ -151,8 +151,6 @@ final class ManagedChannelImpl extends ManagedChannel implements Instrumented oobExecutorPool, Supplier stopwatchSupplier, List 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 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; } diff --git a/core/src/main/java/io/grpc/internal/ProxyDetectorImpl.java b/core/src/main/java/io/grpc/internal/ProxyDetectorImpl.java index 41bb894f71..56c7d7f0ce 100644 --- a/core/src/main/java/io/grpc/internal/ProxyDetectorImpl.java +++ b/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())); } /** diff --git a/core/src/main/java/io/grpc/internal/ProxyParameters.java b/core/src/main/java/io/grpc/internal/ProxyParameters.java index dcf8841ff0..2a70e2cc47 100644 --- a/core/src/main/java/io/grpc/internal/ProxyParameters.java +++ b/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; } diff --git a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java b/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java index dc9d282d1d..fe2cfa6b6f 100644 --- a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java +++ b/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) { diff --git a/core/src/test/java/io/grpc/internal/InternalSubchannelTest.java b/core/src/test/java/io/grpc/internal/InternalSubchannelTest.java index ecd7ffd49a..f35ed49446 100644 --- a/core/src/test/java/io/grpc/internal/InternalSubchannelTest.java +++ b/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() { diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java index 82e3f71677..ba04974efb 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java @@ -143,7 +143,7 @@ public class ManagedChannelImplIdlenessTest { builder, mockTransportFactory, new FakeBackoffPolicyProvider(), oobExecutorPool, timer.getStopwatchSupplier(), Collections.emptyList(), - GrpcUtil.NOOP_PROXY_DETECTOR, CallTracer.getDefaultFactory()); + CallTracer.getDefaultFactory()); newTransports = TestUtils.captureTransports(mockTransportFactory); for (int i = 0; i < 2; i++) { diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java index 863a9ad215..a6c2bb2d55 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java +++ b/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) { diff --git a/core/src/test/java/io/grpc/internal/ProxyDetectorImplTest.java b/core/src/test/java/io/grpc/internal/ProxyDetectorImplTest.java index 656d307657..9b4acd722d 100644 --- a/core/src/test/java/io/grpc/internal/ProxyDetectorImplTest.java +++ b/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 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() { @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); } } diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java index e13e917f7f..d74defcc8c 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 ac3187948a..5a0a70d10e 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 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 1a323af40e..b9d7061779 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); @@ -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 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();