Merge pull request #448 from square/jwilson_0115_tls_modes

Don't attempt TLS modes we know will fail.

okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/ByteArrayPoolTest.java

@@ -18,9 +18,9 @@ package com.squareup.okhttp.internal.spdy;
 
 import org.junit.Test;
 
-import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
 
 public class ByteArrayPoolTest {
   @Test public void testReusesBuffer() {

okhttp/src/main/java/com/squareup/okhttp/Route.java

@@ -75,11 +75,6 @@ public class Route {
     return modernTls;
   }
 
-  /** Returns a copy of this route with flipped TLS mode. */
-  Route flipTlsMode() {
-    return new Route(address, proxy, inetSocketAddress, !modernTls);
-  }
-
   @Override public boolean equals(Object obj) {
     if (obj instanceof Route) {
       Route other = (Route) obj;

okhttp/src/main/java/com/squareup/okhttp/RouteDatabase.java

@@ -15,10 +15,8 @@
  */
 package com.squareup.okhttp;
 
-import java.io.IOException;
 import java.util.LinkedHashSet;
 import java.util.Set;
-import javax.net.ssl.SSLHandshakeException;
 
 /**
  * A blacklist of failed routes to avoid when creating a new connection to a
@@ -31,14 +29,8 @@ public final class RouteDatabase {
   private final Set<Route> failedRoutes = new LinkedHashSet<Route>();
 
   /** Records a failure connecting to {@code failedRoute}. */
-  public synchronized void failed(Route failedRoute, IOException failure) {
+  public synchronized void failed(Route failedRoute) {
     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());
-    }
   }
 
   /** Records success connecting to {@code failedRoute}. */

okhttp/src/main/java/com/squareup/okhttp/internal/http/RouteSelector.java

@@ -33,6 +33,7 @@ import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.NoSuchElementException;
+import javax.net.ssl.SSLHandshakeException;
 
 import static com.squareup.okhttp.internal.Util.getEffectivePort;
 
@@ -148,7 +149,16 @@ public final class RouteSelector {
       proxySelector.connectFailed(uri, failedRoute.getProxy().address(), failure);
     }
 
-    routeDatabase.failed(failedRoute, failure);
+    routeDatabase.failed(failedRoute);
+
+    // If the previously returned route's problem was not related to TLS, and
+    // the next route only changes the TLS mode, we shouldn't even attempt it.
+    // This suppresses it in both this selector and also in the route database.
+    if (hasNextTlsMode() && !(failure instanceof SSLHandshakeException)) {
+      boolean modernTls = nextTlsMode() == TLS_MODE_MODERN;
+      Route routeToSuppress = new Route(address, lastProxy, lastInetSocketAddress, modernTls);
+      routeDatabase.failed(routeToSuppress);
+    }
   }
 
   /** Resets {@link #nextProxy} to the first option. */

okhttp/src/test/java/com/squareup/okhttp/internal/http/HttpOverSpdyTest.java

@@ -15,7 +15,6 @@
  */
 package com.squareup.okhttp.internal.http;
 
-
 import com.squareup.okhttp.HttpResponseCache;
 import com.squareup.okhttp.OkHttpClient;
 import com.squareup.okhttp.internal.RecordingAuthenticator;
@@ -25,9 +24,7 @@ import com.squareup.okhttp.mockwebserver.MockResponse;
 import com.squareup.okhttp.mockwebserver.MockWebServer;
 import com.squareup.okhttp.mockwebserver.RecordedRequest;
 import com.squareup.okhttp.mockwebserver.SocketPolicy;
-
 import java.io.ByteArrayOutputStream;
-import java.io.DataInputStream;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
@@ -46,9 +43,6 @@ import java.util.UUID;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
-import java.util.concurrent.LinkedBlockingQueue;
-import java.util.concurrent.ThreadPoolExecutor;
-import java.util.concurrent.TimeUnit;
 import java.util.zip.GZIPOutputStream;
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLContext;

okhttp/src/test/java/com/squareup/okhttp/internal/http/RouteSelectorTest.java

@@ -107,7 +107,7 @@ public final class RouteSelectorTest {
     dns.inetAddresses = makeFakeAddresses(255, 1);
     Connection connection = routeSelector.next("GET");
     RouteDatabase routeDatabase = new RouteDatabase();
-    routeDatabase.failed(connection.getRoute(), new IOException());
+    routeDatabase.failed(connection.getRoute());
     routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns, routeDatabase);
     assertConnection(routeSelector.next("GET"), address, NO_PROXY, dns.inetAddresses[0], uriPort,
         false);
@@ -281,6 +281,7 @@ public final class RouteSelectorTest {
     assertFalse(routeSelector.hasNext());
   }
 
+  // https://github.com/square/okhttp/issues/442
   @Test public void nonSslErrorAddsAllTlsModesToFailedRoute() throws Exception {
     Address address = new Address(uriHost, uriPort, socketFactory, hostnameVerifier, authenticator,
         Proxy.NO_PROXY, transports);
@@ -292,6 +293,7 @@ public final class RouteSelectorTest {
     Connection connection = routeSelector.next("GET");
     routeSelector.connectFailed(connection, new IOException("Non SSL exception"));
     assertTrue(routeDatabase.failedRoutesCount() == 2);
+    assertFalse(routeSelector.hasNext());
   }
 
   @Test public void sslErrorAddsOnlyFailedTlsModeToFailedRoute() throws Exception {
@@ -305,6 +307,7 @@ public final class RouteSelectorTest {
     Connection connection = routeSelector.next("GET");
     routeSelector.connectFailed(connection, new SSLHandshakeException("SSL exception"));
     assertTrue(routeDatabase.failedRoutesCount() == 1);
+    assertTrue(routeSelector.hasNext());
   }
 
   @Test public void multipleProxiesMultipleInetAddressesMultipleTlsModes() throws Exception {
@@ -372,7 +375,7 @@ public final class RouteSelectorTest {
     // Check that we do indeed have more than one route.
     assertTrue(regularRoutes.size() > 1);
     // Add first regular route as failed.
-    routeDatabase.failed(regularRoutes.get(0).getRoute(), new SSLHandshakeException("none"));
+    routeDatabase.failed(regularRoutes.get(0).getRoute());
     // Reset selector
     routeSelector = new RouteSelector(address, uri, proxySelector, pool, dns, routeDatabase);