Address feedback

microsoft · Jan 4, 2019 · c974f69 · c974f69
1 parent f94875e
commit c974f69
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 13 deletions.
diff --git a/sdk/appcenter/proguard-rules.pro b/sdk/appcenter/proguard-rules.pro
@@ -8,5 +8,5 @@
 }
 
 -keepclassmembers class * implements javax.net.ssl.SSLSocketFactory {
-    private javax.net.ssl.SSLSocketFactory delegate;
+    private final javax.net.ssl.SSLSocketFactory delegate;
 }
diff --git a/sdk/appcenter/src/main/java/com/microsoft/appcenter/http/DefaultHttpClientCallTask.java b/sdk/appcenter/src/main/java/com/microsoft/appcenter/http/DefaultHttpClientCallTask.java
@@ -174,6 +174,14 @@ private String doHttpCall() throws Exception {
              * Make sure we use TLS 1.2 when the device supports it but not enabled by default.
              * Don't hardcode TLS version when enabled by default to avoid unnecessary wrapping and
              * to support future versions of TLS such as say 1.3 without having to patch this code.
+             *
+             * TLS 1.2 was enabled by default only on Android 5.0:
+             * https://developer.android.com/about/versions/android-5.0-changes#ssl
+             * https://developer.android.com/reference/javax/net/ssl/SSLSocket#default-configuration-for-different-android-versions
+             *
+             * There is a problem that TLS 1.2 is still disabled by default on some Samsung devices
+             * with API 21, so apply the rule to this API level as well.
+             * See https://github.com/square/okhttp/issues/2372#issuecomment-244807676
              */
             if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.LOLLIPOP) {
                 urlConnection.setSSLSocketFactory(new TLS1_2SocketFactory());

diff --git a/sdk/appcenter/src/main/java/com/microsoft/appcenter/http/TLS1_2SocketFactory.java b/sdk/appcenter/src/main/java/com/microsoft/appcenter/http/TLS1_2SocketFactory.java
@@ -30,21 +30,24 @@ class TLS1_2SocketFactory extends SSLSocketFactory {
     /**
      * Socket factory.
      *
-     * DO NOT RENAME IT! See https://github.com/square/okhttp/issues/2323
+     * Do not rename it! See https://github.com/square/okhttp/issues/2323
      */
-    private SSLSocketFactory delegate;
+    private final SSLSocketFactory delegate;
 
     TLS1_2SocketFactory() {
+        SSLSocketFactory socketFactory = null;
         try {
+
+            /*
+             * Explicitly specify protocol for SSL context.
+             * See https://www.java.com/en/configure_crypto.html#enableTLSv1_2
+             */
             SSLContext sc = SSLContext.getInstance(TLS1_2_PROTOCOL);
             sc.init(null, null, null);
-            delegate = sc.getSocketFactory();
-        } catch (KeyManagementException ignored) {
-        } catch (NoSuchAlgorithmException ignored) {
-        }
-        if (delegate == null) {
-            delegate = getDefaultSSLSocketFactory();
+            socketFactory = sc.getSocketFactory();
+        } catch (KeyManagementException | NoSuchAlgorithmException ignored) {
         }
+        delegate = socketFactory != null ? socketFactory : getDefaultSSLSocketFactory();
     }
 
     /**

diff --git a/sdk/appcenter/src/test/java/com/microsoft/appcenter/http/Tls1_2SocketFactoryTest.java b/sdk/appcenter/src/test/java/com/microsoft/appcenter/http/Tls1_2SocketFactoryTest.java
@@ -78,22 +78,22 @@ public void createFactory() throws Exception {
                 .thenReturn(sslContext)
                 .thenThrow(new NoSuchAlgorithmException());
 
-        // Mock default factory.
+        /* Mock default factory. */
         mockStatic(HttpsURLConnection.class);
         when(HttpsURLConnection.getDefaultSSLSocketFactory())
                 .thenReturn(sslSocketFactory);
 
-        // Get factory from context.
+        /* Get factory from context. */
         assertNotNull(new TLS1_2SocketFactory());
         verifyStatic(never());
         HttpsURLConnection.getDefaultSSLSocketFactory();
 
-        // KeyManagementException
+        /* KeyManagementException */
         assertNotNull(new TLS1_2SocketFactory());
         verifyStatic(times(1));
         HttpsURLConnection.getDefaultSSLSocketFactory();
 
-        // NoSuchAlgorithmException
+        /* NoSuchAlgorithmException */
         assertNotNull(new TLS1_2SocketFactory());
         verifyStatic(times(2));
         HttpsURLConnection.getDefaultSSLSocketFactory();