From 1ece746c815a26782042b1fca6069eb527d1029b Mon Sep 17 00:00:00 2001 From: Marcelo Cortes Date: Wed, 10 Apr 2013 10:27:26 -0700 Subject: [PATCH] RouteSelector tries previously failed routes last #106 --- .../java/com/squareup/okhttp/Connection.java | 75 ++++------- .../com/squareup/okhttp/ConnectionPool.java | 2 +- .../com/squareup/okhttp/OkHttpClient.java | 11 +- .../main/java/com/squareup/okhttp/Route.java | 91 +++++++++++++ .../okhttp/internal/http/HttpEngine.java | 10 +- .../internal/http/HttpURLConnectionImpl.java | 14 +- .../internal/http/HttpsURLConnectionImpl.java | 15 ++- .../okhttp/internal/http/RouteSelector.java | 55 ++++++-- .../squareup/okhttp/ConnectionPoolTest.java | 18 +-- .../internal/http/RouteSelectorTest.java | 123 ++++++++++++++---- 10 files changed, 306 insertions(+), 108 deletions(-) create mode 100644 okhttp/src/main/java/com/squareup/okhttp/Route.java diff --git a/okhttp/src/main/java/com/squareup/okhttp/Connection.java b/okhttp/src/main/java/com/squareup/okhttp/Connection.java index 90424ffe7..986ef2ed0 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/Connection.java +++ b/okhttp/src/main/java/com/squareup/okhttp/Connection.java @@ -28,7 +28,6 @@ import java.io.Closeable; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.net.InetSocketAddress; import java.net.Proxy; import java.net.Socket; import java.net.URL; @@ -76,10 +75,7 @@ public final class Connection implements Closeable { 'h', 't', 't', 'p', '/', '1', '.', '1' }; - private final Address address; - private final Proxy proxy; - private final InetSocketAddress inetSocketAddress; - private final boolean modernTls; + private final Route route; private Socket socket; private InputStream in; @@ -89,15 +85,8 @@ public final class Connection implements Closeable { private int httpMinorVersion = 1; // Assume HTTP/1.1 private long idleStartTimeNs; - public Connection(Address address, Proxy proxy, InetSocketAddress inetSocketAddress, - boolean modernTls) { - if (address == null) throw new NullPointerException("address == null"); - if (proxy == null) throw new NullPointerException("proxy == null"); - if (inetSocketAddress == null) throw new NullPointerException("inetSocketAddress == null"); - this.address = address; - this.proxy = proxy; - this.inetSocketAddress = inetSocketAddress; - this.modernTls = modernTls; + public Connection(Route route) { + this.route = route; } public void connect(int connectTimeout, int readTimeout, TunnelRequest tunnelRequest) @@ -106,13 +95,13 @@ public final class Connection implements Closeable { throw new IllegalStateException("already connected"); } connected = true; - socket = (proxy.type() != Proxy.Type.HTTP) ? new Socket(proxy) : new Socket(); - socket.connect(inetSocketAddress, connectTimeout); + socket = (route.proxy.type() != Proxy.Type.HTTP) ? new Socket(route.proxy) : new Socket(); + socket.connect(route.inetSocketAddress, connectTimeout); socket.setSoTimeout(readTimeout); in = socket.getInputStream(); out = socket.getOutputStream(); - if (address.sslSocketFactory != null) { + if (route.address.sslSocketFactory != null) { upgradeToTls(tunnelRequest); } @@ -136,16 +125,16 @@ public final class Connection implements Closeable { } // Create the wrapper over connected socket. - socket = address.sslSocketFactory - .createSocket(socket, address.uriHost, address.uriPort, true /* autoClose */); + socket = route.address.sslSocketFactory + .createSocket(socket, route.address.uriHost, route.address.uriPort, true /* autoClose */); SSLSocket sslSocket = (SSLSocket) socket; - if (modernTls) { - platform.enableTlsExtensions(sslSocket, address.uriHost); + if (route.modernTls) { + platform.enableTlsExtensions(sslSocket, route.address.uriHost); } else { platform.supportTlsIntolerantServer(sslSocket); } - if (modernTls) { + if (route.modernTls) { platform.setNpnProtocols(sslSocket, NPN_PROTOCOLS); } @@ -153,18 +142,19 @@ public final class Connection implements Closeable { sslSocket.startHandshake(); // Verify that the socket's certificates are acceptable for the target host. - if (!address.hostnameVerifier.verify(address.uriHost, sslSocket.getSession())) { - throw new IOException("Hostname '" + address.uriHost + "' was not verified"); + if (!route.address.hostnameVerifier.verify(route.address.uriHost, sslSocket.getSession())) { + throw new IOException("Hostname '" + route.address.uriHost + "' was not verified"); } out = sslSocket.getOutputStream(); in = sslSocket.getInputStream(); byte[] selectedProtocol; - if (modernTls && (selectedProtocol = platform.getNpnSelectedProtocol(sslSocket)) != null) { + if (route.modernTls && (selectedProtocol = platform.getNpnSelectedProtocol(sslSocket)) != null) { if (Arrays.equals(selectedProtocol, SPDY3)) { sslSocket.setSoTimeout(0); // SPDY timeouts are set per-stream. - spdyConnection = new SpdyConnection.Builder(address.getUriHost(), true, in, out).build(); + spdyConnection = new SpdyConnection.Builder(route.address.getUriHost(), true, in, out) + .build(); } else if (!Arrays.equals(selectedProtocol, HTTP_11)) { throw new IOException( "Unexpected NPN transport " + new String(selectedProtocol, "ISO-8859-1")); @@ -181,29 +171,9 @@ public final class Connection implements Closeable { socket.close(); } - /** - * Returns the proxy that this connection is using. - * - * Warning: This may be different than the proxy returned - * by {@link #getAddress}! That is the proxy that the user asked to be - * connected to; this returns the proxy that they were actually connected - * to. The two may disagree when a proxy selector selects a different proxy - * for a connection. - */ - public Proxy getProxy() { - return proxy; - } - - public Address getAddress() { - return address; - } - - public InetSocketAddress getSocketAddress() { - return inetSocketAddress; - } - - public boolean isModernTls() { - return modernTls; + /** Returns the route used by this connection. */ + public Route getRoute() { + return route; } /** @@ -284,7 +254,7 @@ public final class Connection implements Closeable { * we must avoid buffering bytes intended for the higher-level protocol. */ public boolean requiresTunnel() { - return address.sslSocketFactory != null && proxy != null && proxy.type() == Proxy.Type.HTTP; + return route.address.sslSocketFactory != null && route.proxy.type() == Proxy.Type.HTTP; } /** @@ -304,9 +274,8 @@ public final class Connection implements Closeable { case HTTP_PROXY_AUTH: requestHeaders = new RawHeaders(requestHeaders); URL url = new URL("https", tunnelRequest.host, tunnelRequest.port, "/"); - boolean credentialsFound = - HttpAuthenticator.processAuthHeader(HTTP_PROXY_AUTH, responseHeaders, requestHeaders, - proxy, url); + boolean credentialsFound = HttpAuthenticator.processAuthHeader(HTTP_PROXY_AUTH, + responseHeaders, requestHeaders, route.proxy, url); if (credentialsFound) { continue; } else { diff --git a/okhttp/src/main/java/com/squareup/okhttp/ConnectionPool.java b/okhttp/src/main/java/com/squareup/okhttp/ConnectionPool.java index 4eff4ec74..e9e210103 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/ConnectionPool.java +++ b/okhttp/src/main/java/com/squareup/okhttp/ConnectionPool.java @@ -164,7 +164,7 @@ public class ConnectionPool { for (ListIterator i = connections.listIterator(connections.size()); i.hasPrevious(); ) { Connection connection = i.previous(); - if (!connection.getAddress().equals(address) + if (!connection.getRoute().getAddress().equals(address) || !connection.isAlive() || System.nanoTime() - connection.getIdleStartTimeNs() >= keepAliveDurationNs) { continue; diff --git a/okhttp/src/main/java/com/squareup/okhttp/OkHttpClient.java b/okhttp/src/main/java/com/squareup/okhttp/OkHttpClient.java index 325165d18..7834bd6b2 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/OkHttpClient.java +++ b/okhttp/src/main/java/com/squareup/okhttp/OkHttpClient.java @@ -25,6 +25,9 @@ import java.net.Proxy; import java.net.ProxySelector; import java.net.ResponseCache; import java.net.URL; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.Set; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.SSLSocketFactory; @@ -32,6 +35,7 @@ import javax.net.ssl.SSLSocketFactory; /** Configures and creates HTTP connections. */ public final class OkHttpClient { private Proxy proxy; + private Set failedRoutes = Collections.synchronizedSet(new LinkedHashSet()); private ProxySelector proxySelector; private CookieHandler cookieHandler; private ResponseCache responseCache; @@ -180,21 +184,22 @@ public final class OkHttpClient { String protocol = url.getProtocol(); OkHttpClient copy = copyWithDefaults(); if (protocol.equals("http")) { - return new HttpURLConnectionImpl(url, copy, copy.okResponseCache()); + return new HttpURLConnectionImpl(url, copy, copy.okResponseCache(), copy.failedRoutes); } else if (protocol.equals("https")) { - return new HttpsURLConnectionImpl(url, copy, copy.okResponseCache()); + return new HttpsURLConnectionImpl(url, copy, copy.okResponseCache(), copy.failedRoutes); } else { throw new IllegalArgumentException("Unexpected protocol: " + protocol); } } /** - * Returns a copy of this OkHttpClient that uses the system-wide default for + * Returns a shallow copy of this OkHttpClient that uses the system-wide default for * each field that hasn't been explicitly configured. */ private OkHttpClient copyWithDefaults() { OkHttpClient result = new OkHttpClient(); result.proxy = proxy; + result.failedRoutes = failedRoutes; result.proxySelector = proxySelector != null ? proxySelector : ProxySelector.getDefault(); result.cookieHandler = cookieHandler != null ? cookieHandler : CookieHandler.getDefault(); result.responseCache = responseCache != null ? responseCache : ResponseCache.getDefault(); diff --git a/okhttp/src/main/java/com/squareup/okhttp/Route.java b/okhttp/src/main/java/com/squareup/okhttp/Route.java new file mode 100644 index 000000000..d644a225b --- /dev/null +++ b/okhttp/src/main/java/com/squareup/okhttp/Route.java @@ -0,0 +1,91 @@ +/* + * Copyright (C) 2013 The Android Open Source Project + * + * 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 com.squareup.okhttp; + +import java.net.InetSocketAddress; +import java.net.Proxy; + +/** Represents the route used by a connection to reach an endpoint. */ +public class Route { + final Address address; + final Proxy proxy; + final InetSocketAddress inetSocketAddress; + final boolean modernTls; + + public Route(Address address, Proxy proxy, InetSocketAddress inetSocketAddress, + boolean modernTls) { + if (address == null) throw new NullPointerException("address == null"); + if (proxy == null) throw new NullPointerException("proxy == null"); + if (inetSocketAddress == null) throw new NullPointerException("inetSocketAddress == null"); + this.address = address; + this.proxy = proxy; + this.inetSocketAddress = inetSocketAddress; + this.modernTls = modernTls; + } + + /** Returns the {@link Address} of this route. */ + public Address getAddress() { + return address; + } + + /** + * Returns the {@link Proxy} of this route. + * + * Warning: This may be different than the proxy returned + * by {@link #getAddress}! That is the proxy that the user asked to be + * connected to; this returns the proxy that they were actually connected + * to. The two may disagree when a proxy selector selects a different proxy + * for a connection. + */ + public Proxy getProxy() { + return proxy; + } + + /** Returns the {@link InetSocketAddress} of this route. */ + public InetSocketAddress getSocketAddress() { + return inetSocketAddress; + } + + /** Returns true if this route uses modern tls. */ + public boolean isModernTls() { + return modernTls; + } + + /** Returns a copy of this route with flipped tls mode. */ + public Route flipTlsMode() { + return new Route(address, proxy, inetSocketAddress, !modernTls); + } + + @Override public boolean equals(Object obj) { + if (obj instanceof Route) { + Route other = (Route) obj; + return (address.equals(other.address) + && proxy.equals(other.proxy) + && inetSocketAddress.equals(other.inetSocketAddress) + && modernTls == other.modernTls); + } + return false; + } + + @Override public int hashCode() { + int result = 17; + result = 31 * result + address.hashCode(); + result = 31 * result + proxy.hashCode(); + result = 31 * result + inetSocketAddress.hashCode(); + result = result + (modernTls ? (31 * result) : 0); + return result; + } +} diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java index 5218655e7..7a06dca53 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java @@ -197,6 +197,7 @@ public class HttpEngine { sendSocketRequest(); } else if (connection != null) { policy.connectionPool.recycle(connection); + policy.getFailedRoutes().remove(connection.getRoute()); connection = null; } } @@ -278,16 +279,17 @@ public class HttpEngine { } Address address = new Address(uriHost, getEffectivePort(uri), sslSocketFactory, hostnameVerifier, policy.requestedProxy); - routeSelector = - new RouteSelector(address, uri, policy.proxySelector, policy.connectionPool, Dns.DEFAULT); + routeSelector = new RouteSelector(address, uri, policy.proxySelector, policy.connectionPool, + Dns.DEFAULT, policy.getFailedRoutes()); } connection = routeSelector.next(); if (!connection.isConnected()) { connection.connect(policy.getConnectTimeout(), policy.getReadTimeout(), getTunnelConfig()); policy.connectionPool.maybeShare(connection); + policy.getFailedRoutes().remove(connection.getRoute()); } connected(connection); - if (connection.getProxy() != policy.requestedProxy) { + if (connection.getRoute().getProxy() != policy.requestedProxy) { // Update the request line if the proxy changed; it may need a host name. requestHeaders.getHeaders().setRequestLine(getRequestLine()); } @@ -573,7 +575,7 @@ public class HttpEngine { protected boolean includeAuthorityInRequestLine() { return connection == null ? policy.usingProxy() // A proxy was requested. - : connection.getProxy().type() == Proxy.Type.HTTP; // A proxy was selected. + : connection.getRoute().getProxy().type() == Proxy.Type.HTTP; // A proxy was selected. } public static String getDefaultUserAgent() { diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpURLConnectionImpl.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpURLConnectionImpl.java index 72f1128ce..1da7cf513 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpURLConnectionImpl.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpURLConnectionImpl.java @@ -20,6 +20,7 @@ package com.squareup.okhttp.internal.http; import com.squareup.okhttp.Connection; import com.squareup.okhttp.ConnectionPool; import com.squareup.okhttp.OkHttpClient; +import com.squareup.okhttp.Route; import com.squareup.okhttp.internal.Util; import java.io.FileNotFoundException; import java.io.IOException; @@ -38,6 +39,7 @@ import java.security.Permission; import java.security.cert.CertificateException; import java.util.List; import java.util.Map; +import java.util.Set; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLHandshakeException; import javax.net.ssl.SSLSocketFactory; @@ -81,6 +83,8 @@ public class HttpURLConnectionImpl extends HttpURLConnection { /* SSL configuration; necessary for HTTP requests that get redirected to HTTPS. */ SSLSocketFactory sslSocketFactory; HostnameVerifier hostnameVerifier; + final Set failedRoutes; + private final RawHeaders rawRequestHeaders = new RawHeaders(); @@ -89,9 +93,11 @@ public class HttpURLConnectionImpl extends HttpURLConnection { protected IOException httpEngineFailure; protected HttpEngine httpEngine; - public HttpURLConnectionImpl(URL url, OkHttpClient client, OkResponseCache responseCache) { + public HttpURLConnectionImpl(URL url, OkHttpClient client, OkResponseCache responseCache, + Set failedRoutes) { super(url); this.followProtocolRedirects = client.getFollowProtocolRedirects(); + this.failedRoutes = failedRoutes; this.requestedProxy = client.getProxy(); this.proxySelector = client.getProxySelector(); this.cookieHandler = client.getCookieHandler(); @@ -101,6 +107,10 @@ public class HttpURLConnectionImpl extends HttpURLConnection { this.responseCache = responseCache; } + Set getFailedRoutes() { + return failedRoutes; + } + @Override public final void connect() throws IOException { initHttpEngine(); boolean success; @@ -401,7 +411,7 @@ public class HttpURLConnectionImpl extends HttpURLConnection { */ private Retry processResponseHeaders() throws IOException { Proxy selectedProxy = httpEngine.connection != null - ? httpEngine.connection.getProxy() + ? httpEngine.connection.getRoute().getProxy() : requestedProxy; final int responseCode = getResponseCode(); switch (responseCode) { diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpsURLConnectionImpl.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpsURLConnectionImpl.java index a12416233..235f86295 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpsURLConnectionImpl.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpsURLConnectionImpl.java @@ -18,6 +18,7 @@ package com.squareup.okhttp.internal.http; import com.squareup.okhttp.Connection; import com.squareup.okhttp.OkHttpClient; +import com.squareup.okhttp.Route; import com.squareup.okhttp.TunnelRequest; import java.io.IOException; import java.io.InputStream; @@ -32,6 +33,7 @@ import java.security.Principal; import java.security.cert.Certificate; import java.util.List; import java.util.Map; +import java.util.Set; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.SSLPeerUnverifiedException; @@ -45,9 +47,10 @@ public final class HttpsURLConnectionImpl extends HttpsURLConnection { /** HttpUrlConnectionDelegate allows reuse of HttpURLConnectionImpl. */ private final HttpUrlConnectionDelegate delegate; - public HttpsURLConnectionImpl(URL url, OkHttpClient client, OkResponseCache responseCache) { + public HttpsURLConnectionImpl(URL url, OkHttpClient client, OkResponseCache responseCache, + Set failedRoutes) { super(url); - delegate = new HttpUrlConnectionDelegate(url, client, responseCache); + delegate = new HttpUrlConnectionDelegate(url, client, responseCache, failedRoutes); } @Override public String getCipherSuite() { @@ -399,8 +402,9 @@ public final class HttpsURLConnectionImpl extends HttpsURLConnection { } private final class HttpUrlConnectionDelegate extends HttpURLConnectionImpl { - private HttpUrlConnectionDelegate(URL url, OkHttpClient client, OkResponseCache responseCache) { - super(url, client, responseCache); + private HttpUrlConnectionDelegate(URL url, OkHttpClient client, OkResponseCache responseCache, + Set failedRoutes) { + super(url, client, responseCache, failedRoutes); } @Override protected HttpURLConnection getHttpConnectionToCache() { @@ -425,8 +429,7 @@ public final class HttpsURLConnectionImpl extends HttpsURLConnection { * @param policy the HttpURLConnectionImpl with connection configuration */ public HttpsEngine(HttpURLConnectionImpl policy, String method, RawHeaders requestHeaders, - Connection connection, RetryableOutputStream requestBody) - throws IOException { + Connection connection, RetryableOutputStream requestBody) throws IOException { super(policy, method, requestHeaders, connection, requestBody); this.sslSocket = connection != null ? (SSLSocket) connection.getSocket() : null; } diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/RouteSelector.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/RouteSelector.java index 798cff3bc..ce0a71d84 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/RouteSelector.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/RouteSelector.java @@ -18,6 +18,7 @@ package com.squareup.okhttp.internal.http; import com.squareup.okhttp.Address; import com.squareup.okhttp.Connection; import com.squareup.okhttp.ConnectionPool; +import com.squareup.okhttp.Route; import com.squareup.okhttp.internal.Dns; import java.io.IOException; import java.net.InetAddress; @@ -28,8 +29,11 @@ import java.net.SocketAddress; import java.net.URI; import java.net.UnknownHostException; import java.util.Iterator; +import java.util.LinkedList; import java.util.List; import java.util.NoSuchElementException; +import java.util.Set; +import javax.net.ssl.SSLHandshakeException; import static com.squareup.okhttp.internal.Util.getEffectivePort; @@ -51,6 +55,7 @@ public final class RouteSelector { private final ProxySelector proxySelector; private final ConnectionPool pool; private final Dns dns; + private final Set failedRoutes; /* The most recently attempted route. */ private Proxy lastProxy; @@ -64,19 +69,23 @@ public final class RouteSelector { /* State for negotiating the next InetSocketAddress to use. */ private InetAddress[] socketAddresses; private int nextSocketAddressIndex; - private String socketHost; private int socketPort; /* State for negotiating the next TLS configuration */ private int nextTlsMode = TLS_MODE_NULL; + /* State for negotiating failed routes */ + private final List postponedRoutes; + public RouteSelector(Address address, URI uri, ProxySelector proxySelector, ConnectionPool pool, - Dns dns) { + Dns dns, Set failedRoutes) { this.address = address; this.uri = uri; this.proxySelector = proxySelector; this.pool = pool; this.dns = dns; + this.failedRoutes = failedRoutes; + this.postponedRoutes = new LinkedList(); resetNextProxy(uri, address.getProxy()); } @@ -86,7 +95,7 @@ public final class RouteSelector { * least one route. */ public boolean hasNext() { - return hasNextTlsMode() || hasNextInetSocketAddress() || hasNextProxy(); + return hasNextTlsMode() || hasNextInetSocketAddress() || hasNextProxy() || hasNextPostponed(); } /** @@ -105,7 +114,10 @@ public final class RouteSelector { if (!hasNextTlsMode()) { if (!hasNextInetSocketAddress()) { if (!hasNextProxy()) { - throw new NoSuchElementException(); + if (!hasNextPostponed()) { + throw new NoSuchElementException(); + } + return new Connection(nextPostponed()); } lastProxy = nextProxy(); resetNextInetSocketAddress(lastProxy); @@ -113,9 +125,17 @@ public final class RouteSelector { lastInetSocketAddress = nextInetSocketAddress(); resetNextTlsMode(); } - boolean modernTls = nextTlsMode() == TLS_MODE_MODERN; - return new Connection(address, lastProxy, lastInetSocketAddress, modernTls); + boolean modernTls = nextTlsMode() == TLS_MODE_MODERN; + Route route = new Route(address, lastProxy, lastInetSocketAddress, modernTls); + if (failedRoutes.contains(route)) { + postponedRoutes.add(route); + // We will only recurse in order to skip previously failed routes. They will be + // tried last. + return next(); + } + + return new Connection(route); } /** @@ -123,9 +143,17 @@ public final class RouteSelector { * failure on a connection returned by this route selector. */ public void connectFailed(Connection connection, IOException failure) { - if (connection.getProxy().type() != Proxy.Type.DIRECT && proxySelector != null) { + Route failedRoute = connection.getRoute(); + if (failedRoute.getProxy().type() != Proxy.Type.DIRECT && proxySelector != null) { // Tell the proxy selector when we fail to connect on a fresh connection. - proxySelector.connectFailed(uri, connection.getProxy().address(), failure); + proxySelector.connectFailed(uri, failedRoute.getProxy().address(), failure); + } + + failedRoutes.add(failedRoute); + if (!(failure instanceof SSLHandshakeException)) { + // If the problem was not related to SSL then it will also fail with + // a different Tls mode therefore we can be proactive about it. + failedRoutes.add(failedRoute.flipTlsMode()); } } @@ -175,6 +203,7 @@ public final class RouteSelector { private void resetNextInetSocketAddress(Proxy proxy) throws UnknownHostException { socketAddresses = null; // Clear the addresses. Necessary if getAllByName() below throws! + String socketHost; if (proxy.type() == Proxy.Type.DIRECT) { socketHost = uri.getHost(); socketPort = getEffectivePort(uri); @@ -233,4 +262,14 @@ public final class RouteSelector { throw new AssertionError(); } } + + /** Returns true if there is another postponed route to try. */ + private boolean hasNextPostponed() { + return !postponedRoutes.isEmpty(); + } + + /** Returns the next postponed route to try. */ + private Route nextPostponed() { + return postponedRoutes.remove(0); + } } diff --git a/okhttp/src/test/java/com/squareup/okhttp/ConnectionPoolTest.java b/okhttp/src/test/java/com/squareup/okhttp/ConnectionPoolTest.java index 515453e08..a1acf6d2e 100644 --- a/okhttp/src/test/java/com/squareup/okhttp/ConnectionPoolTest.java +++ b/okhttp/src/test/java/com/squareup/okhttp/ConnectionPoolTest.java @@ -81,19 +81,21 @@ public final class ConnectionPoolTest { spdySocketAddress = new InetSocketAddress(InetAddress.getByName(spdyServer.getHostName()), spdyServer.getPort()); - httpA = new Connection(httpAddress, Proxy.NO_PROXY, httpSocketAddress, true); + Route httpRoute = new Route(httpAddress, Proxy.NO_PROXY, httpSocketAddress, true); + Route spdyRoute = new Route(spdyAddress, Proxy.NO_PROXY, spdySocketAddress, true); + httpA = new Connection(httpRoute); httpA.connect(100, 100, null); - httpB = new Connection(httpAddress, Proxy.NO_PROXY, httpSocketAddress, true); + httpB = new Connection(httpRoute); httpB.connect(100, 100, null); - httpC = new Connection(httpAddress, Proxy.NO_PROXY, httpSocketAddress, true); + httpC = new Connection(httpRoute); httpC.connect(100, 100, null); - httpD = new Connection(httpAddress, Proxy.NO_PROXY, httpSocketAddress, true); + httpD = new Connection(httpRoute); httpD.connect(100, 100, null); - httpE = new Connection(httpAddress, Proxy.NO_PROXY, httpSocketAddress, true); + httpE = new Connection(httpRoute); httpE.connect(100, 100, null); - spdyA = new Connection(spdyAddress, Proxy.NO_PROXY, spdySocketAddress, true); + spdyA = new Connection(spdyRoute); spdyA.connect(100, 100, null); - spdyB = new Connection(spdyAddress, Proxy.NO_PROXY, spdySocketAddress, true); + spdyB = new Connection(spdyRoute); spdyB.connect(100, 100, null); } @@ -115,7 +117,7 @@ public final class ConnectionPoolTest { Connection connection = pool.get(httpAddress); assertNull(connection); - connection = new Connection(httpAddress, Proxy.NO_PROXY, httpSocketAddress, true); + connection = new Connection(new Route(httpAddress, Proxy.NO_PROXY, httpSocketAddress, true)); connection.connect(100, 100, null); assertEquals(0, pool.getConnectionCount()); pool.recycle(connection); diff --git a/okhttp/src/test/java/com/squareup/okhttp/internal/http/RouteSelectorTest.java b/okhttp/src/test/java/com/squareup/okhttp/internal/http/RouteSelectorTest.java index 4dc2e329d..2a241dba8 100644 --- a/okhttp/src/test/java/com/squareup/okhttp/internal/http/RouteSelectorTest.java +++ b/okhttp/src/test/java/com/squareup/okhttp/internal/http/RouteSelectorTest.java @@ -18,6 +18,7 @@ package com.squareup.okhttp.internal.http; import com.squareup.okhttp.Address; import com.squareup.okhttp.Connection; import com.squareup.okhttp.ConnectionPool; +import com.squareup.okhttp.Route; import com.squareup.okhttp.internal.Dns; import com.squareup.okhttp.internal.SslContextBuilder; import java.io.IOException; @@ -30,10 +31,14 @@ import java.net.URI; import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; +import java.util.LinkedHashSet; import java.util.List; import java.util.NoSuchElementException; +import java.util.Set; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLHandshakeException; import javax.net.ssl.SSLSocketFactory; import org.junit.Test; @@ -60,6 +65,7 @@ public final class RouteSelectorTest { private static final SSLSocketFactory socketFactory; private static final HostnameVerifier hostnameVerifier; private static final ConnectionPool pool; + static { try { uri = new URI("http://" + uriHost + ":" + uriPort + "/path"); @@ -77,7 +83,8 @@ public final class RouteSelectorTest { @Test public void singleRoute() throws Exception { Address address = new Address(uriHost, uriPort, null, null, null); - RouteSelector routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns); + RouteSelector routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns, + Collections.EMPTY_SET); assertTrue(routeSelector.hasNext()); dns.inetAddresses = makeFakeAddresses(255, 1); @@ -92,9 +99,31 @@ public final class RouteSelectorTest { } } + @Test public void singleRouteReturnsFailedRoute() throws Exception { + Address address = new Address(uriHost, uriPort, null, null, null); + RouteSelector routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns, + Collections.EMPTY_SET); + + assertTrue(routeSelector.hasNext()); + dns.inetAddresses = makeFakeAddresses(255, 1); + Connection connection = routeSelector.next(); + Set failedRoutes = new LinkedHashSet(); + failedRoutes.add(connection.getRoute()); + routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns, + Collections.EMPTY_SET); + assertConnection(routeSelector.next(), address, NO_PROXY, dns.inetAddresses[0], uriPort, false); + assertFalse(routeSelector.hasNext()); + try { + routeSelector.next(); + fail(); + } catch (NoSuchElementException expected) { + } + } + @Test public void explicitProxyTriesThatProxiesAddressesOnly() throws Exception { Address address = new Address(uriHost, uriPort, null, null, proxyA); - RouteSelector routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns); + RouteSelector routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns, + Collections.EMPTY_SET); assertTrue(routeSelector.hasNext()); dns.inetAddresses = makeFakeAddresses(255, 2); @@ -110,7 +139,8 @@ public final class RouteSelectorTest { @Test public void explicitDirectProxy() throws Exception { Address address = new Address(uriHost, uriPort, null, null, NO_PROXY); - RouteSelector routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns); + RouteSelector routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns, + Collections.EMPTY_SET); assertTrue(routeSelector.hasNext()); dns.inetAddresses = makeFakeAddresses(255, 2); @@ -126,7 +156,8 @@ public final class RouteSelectorTest { Address address = new Address(uriHost, uriPort, null, null, null); proxySelector.proxies = null; - RouteSelector routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns); + RouteSelector routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns, + Collections.EMPTY_SET); proxySelector.assertRequests(uri); assertTrue(routeSelector.hasNext()); @@ -139,7 +170,8 @@ public final class RouteSelectorTest { @Test public void proxySelectorReturnsNoProxies() throws Exception { Address address = new Address(uriHost, uriPort, null, null, null); - RouteSelector routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns); + RouteSelector routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns, + Collections.EMPTY_SET); assertTrue(routeSelector.hasNext()); dns.inetAddresses = makeFakeAddresses(255, 2); @@ -156,7 +188,8 @@ public final class RouteSelectorTest { proxySelector.proxies.add(proxyA); proxySelector.proxies.add(proxyB); - RouteSelector routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns); + RouteSelector routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns, + Collections.EMPTY_SET); proxySelector.assertRequests(uri); // First try the IP addresses of the first proxy, in sequence. @@ -188,7 +221,8 @@ public final class RouteSelectorTest { Address address = new Address(uriHost, uriPort, null, null, null); proxySelector.proxies.add(NO_PROXY); - RouteSelector routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns); + RouteSelector routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns, + Collections.EMPTY_SET); proxySelector.assertRequests(uri); // Only the origin server will be attempted. @@ -206,7 +240,8 @@ public final class RouteSelectorTest { proxySelector.proxies.add(proxyA); proxySelector.proxies.add(proxyB); proxySelector.proxies.add(proxyA); - RouteSelector routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns); + RouteSelector routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns, + Collections.EMPTY_SET); proxySelector.assertRequests(uri); assertTrue(routeSelector.hasNext()); @@ -238,28 +273,38 @@ public final class RouteSelectorTest { assertFalse(routeSelector.hasNext()); } - @Test public void multipleTlsModes() throws Exception { + @Test public void nonSslErrorAddsAllTlsModesToFailedRoute() throws Exception { Address address = new Address(uriHost, uriPort, socketFactory, hostnameVerifier, Proxy.NO_PROXY); - RouteSelector routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns); + Set failedRoutes = new LinkedHashSet(); + RouteSelector routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns, + failedRoutes); - assertTrue(routeSelector.hasNext()); dns.inetAddresses = makeFakeAddresses(255, 1); - assertConnection(routeSelector.next(), address, NO_PROXY, dns.inetAddresses[0], uriPort, true); - dns.assertRequests(uriHost); + Connection connection = routeSelector.next(); + routeSelector.connectFailed(connection, new IOException("Non SSL exception")); + assertTrue(failedRoutes.size() == 2); + } - assertTrue(routeSelector.hasNext()); - assertConnection(routeSelector.next(), address, NO_PROXY, dns.inetAddresses[0], uriPort, false); - dns.assertRequests(); // No more DNS requests since the previous! + @Test public void sslErrorAddsOnlyFailedTlsModeToFailedRoute() throws Exception { + Address address = + new Address(uriHost, uriPort, socketFactory, hostnameVerifier, Proxy.NO_PROXY); + Set failedRoutes = new LinkedHashSet(); + RouteSelector routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns, + failedRoutes); - assertFalse(routeSelector.hasNext()); + dns.inetAddresses = makeFakeAddresses(255, 1); + Connection connection = routeSelector.next(); + routeSelector.connectFailed(connection, new SSLHandshakeException("SSL exception")); + assertTrue(failedRoutes.size() == 1); } @Test public void multipleProxiesMultipleInetAddressesMultipleTlsModes() throws Exception { Address address = new Address(uriHost, uriPort, socketFactory, hostnameVerifier, null); proxySelector.proxies.add(proxyA); proxySelector.proxies.add(proxyB); - RouteSelector routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns); + RouteSelector routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns, + Collections.EMPTY_SET); // Proxy A dns.inetAddresses = makeFakeAddresses(255, 2); @@ -292,13 +337,45 @@ public final class RouteSelectorTest { assertFalse(routeSelector.hasNext()); } + @Test public void failedRoutesAreLast() throws Exception { + Address address = + new Address(uriHost, uriPort, socketFactory, hostnameVerifier, Proxy.NO_PROXY); + + Set failedRoutes = new LinkedHashSet(1); + RouteSelector routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns, + failedRoutes); + dns.inetAddresses = makeFakeAddresses(255, 1); + + // Extract the regular sequence of routes from selector. + List regularRoutes = new ArrayList(); + while (routeSelector.hasNext()) { + regularRoutes.add(routeSelector.next()); + } + + // Check that we do indeed have more than one route. + assertTrue(regularRoutes.size() > 1); + // Add first regular route as failed. + failedRoutes.add(regularRoutes.get(0).getRoute()); + // Reset selector + routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns, failedRoutes); + + List routesWithFailedRoute = new ArrayList(); + while (routeSelector.hasNext()) { + routesWithFailedRoute.add(routeSelector.next()); + } + + assertEquals(regularRoutes.get(0).getRoute(), + routesWithFailedRoute.get(routesWithFailedRoute.size() - 1).getRoute()); + assertEquals(regularRoutes.size(), routesWithFailedRoute.size()); + } + private void assertConnection(Connection connection, Address address, Proxy proxy, InetAddress socketAddress, int socketPort, boolean modernTls) { - assertEquals(address, connection.getAddress()); - assertEquals(proxy, connection.getProxy()); - assertEquals(socketAddress, connection.getSocketAddress().getAddress()); - assertEquals(socketPort, connection.getSocketAddress().getPort()); - assertEquals(modernTls, connection.isModernTls()); + assertEquals(address, connection.getRoute().getAddress()); + assertEquals(proxy, connection.getRoute().getProxy()); + assertEquals(socketAddress, connection.getRoute().getSocketAddress().getAddress()); + assertEquals(socketPort, connection.getRoute().getSocketAddress().getPort()); + assertEquals(modernTls, connection.getRoute().isModernTls()); } private static InetAddress[] makeFakeAddresses(int prefix, int count) {