Merge pull request square#2686 from square/jwilson.0702.cleartext

Always pass a host to NetworkSecurityPolicy.isCleartextTrafficPermitted
hieronymus777 · Jul 3, 2016 · 738f7fa · 738f7fa
2 parents f98ff25 + 1f58b14
commit 738f7fa
Show file tree

Hide file tree

Showing 5 changed files with 20 additions and 23 deletions.
diff --git a/okhttp-tests/src/test/java/okhttp3/CallTest.java b/okhttp-tests/src/test/java/okhttp3/CallTest.java
@@ -1063,7 +1063,7 @@ private void postBodyRetransmittedAfterAuthorizationFail(String body) throws Exc
       client.newCall(request).execute();
       fail();
     } catch (UnknownServiceException expected) {
-      assertTrue(expected.getMessage().contains("CLEARTEXT communication not supported"));
+      assertEquals("CLEARTEXT communication not enabled for client", expected.getMessage());
     }
   }
 

diff --git a/okhttp/src/main/java/okhttp3/OkHttpClient.java b/okhttp/src/main/java/okhttp3/OkHttpClient.java
@@ -65,16 +65,10 @@ public class OkHttpClient implements Cloneable, Call.Factory {
   private static final List<Protocol> DEFAULT_PROTOCOLS = Util.immutableList(
       Protocol.HTTP_2, Protocol.SPDY_3, Protocol.HTTP_1_1);
 
-  private static final List<ConnectionSpec> DEFAULT_CONNECTION_SPECS;
+  private static final List<ConnectionSpec> DEFAULT_CONNECTION_SPECS = Util.immutableList(
+      ConnectionSpec.MODERN_TLS, ConnectionSpec.COMPATIBLE_TLS, ConnectionSpec.CLEARTEXT);
 
   static {
-    List<ConnectionSpec> connSpecs = new ArrayList<>(Arrays.asList(ConnectionSpec.MODERN_TLS,
-        ConnectionSpec.COMPATIBLE_TLS));
-    if (Platform.get().isCleartextTrafficPermitted()) {
-      connSpecs.add(ConnectionSpec.CLEARTEXT);
-    }
-    DEFAULT_CONNECTION_SPECS = Util.immutableList(connSpecs);
-
     Internal.instance = new Internal() {
       @Override public void addLenient(Headers.Builder builder, String line) {
         builder.addLenient(line);

diff --git a/okhttp/src/main/java/okhttp3/internal/connection/RealConnection.java b/okhttp/src/main/java/okhttp3/internal/connection/RealConnection.java
@@ -93,10 +93,16 @@ public void connect(int connectTimeout, int readTimeout, int writeTimeout,
     RouteException routeException = null;
     ConnectionSpecSelector connectionSpecSelector = new ConnectionSpecSelector(connectionSpecs);
 
-    if (route.address().sslSocketFactory() == null
-        && !connectionSpecs.contains(ConnectionSpec.CLEARTEXT)) {
-      throw new RouteException(new UnknownServiceException(
-          "CLEARTEXT communication not supported: " + connectionSpecs));
+    if (route.address().sslSocketFactory() == null) {
+      if (!connectionSpecs.contains(ConnectionSpec.CLEARTEXT)) {
+        throw new RouteException(new UnknownServiceException(
+            "CLEARTEXT communication not enabled for client"));
+      }
+      String host = route.address().url().host();
+      if (!Platform.get().isCleartextTrafficPermitted(host)) {
+        throw new RouteException(new UnknownServiceException(
+            "CLEARTEXT communication to " + host + " not permitted by network security policy"));
+      }
     }
 
     while (protocol == null) {

diff --git a/okhttp/src/main/java/okhttp3/internal/platform/AndroidPlatform.java b/okhttp/src/main/java/okhttp3/internal/platform/AndroidPlatform.java
@@ -132,20 +132,17 @@ public AndroidPlatform(Class<?> sslParametersClass, OptionalMethod<Socket> setUs
     }
   }
 
-  @Override public boolean isCleartextTrafficPermitted() {
+  @Override public boolean isCleartextTrafficPermitted(String hostname) {
     try {
       Class<?> networkPolicyClass = Class.forName("android.security.NetworkSecurityPolicy");
       Method getInstanceMethod = networkPolicyClass.getMethod("getInstance");
       Object networkSecurityPolicy = getInstanceMethod.invoke(null);
       Method isCleartextTrafficPermittedMethod = networkPolicyClass
-          .getMethod("isCleartextTrafficPermitted");
-      boolean cleartextPermitted = (boolean) isCleartextTrafficPermittedMethod
-          .invoke(networkSecurityPolicy);
-      return cleartextPermitted;
-    } catch (ClassNotFoundException e) {
-      return super.isCleartextTrafficPermitted();
-    } catch (NoSuchMethodException | IllegalAccessException | IllegalArgumentException
-        | InvocationTargetException e) {
+          .getMethod("isCleartextTrafficPermitted", String.class);
+      return (boolean) isCleartextTrafficPermittedMethod.invoke(networkSecurityPolicy, hostname);
+    } catch (ClassNotFoundException | NoSuchMethodException e) {
+      return super.isCleartextTrafficPermitted(hostname);
+    } catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException e) {
       throw new AssertionError();
     }
   }

diff --git a/okhttp/src/main/java/okhttp3/internal/platform/Platform.java b/okhttp/src/main/java/okhttp3/internal/platform/Platform.java
@@ -129,7 +129,7 @@ public void log(int level, String message, Throwable t) {
     logger.log(logLevel, message, t);
   }
 
-  public boolean isCleartextTrafficPermitted() {
+  public boolean isCleartextTrafficPermitted(String hostname) {
     return true;
   }