From c902730b75085cecdd40cd5817453d8af0eff4e9 Mon Sep 17 00:00:00 2001
From: Sam Judd <judds@google.com>
Date: Wed, 20 Dec 2017 07:36:50 -0800
Subject: [PATCH] Catch NPEs and RuntimeExceptions thrown in
 getActiveNetworkInfo.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The framework seems to throw these on various devices and versions of
Android, though mostly on Samsung between APIs 22 and 24.

The deleted test was passing because of an exception in isConnected, not
in registerReceiver so it was incorrect. There doesn’t seem to be a way
to force Robolectric to throw when registering receivers.
---
 .../manager/DefaultConnectivityMonitor.java   | 34 +++++++++++--------
 .../DefaultConnectivityMonitorTest.java       |  9 -----
 2 files changed, 19 insertions(+), 24 deletions(-)

diff --git a/library/src/main/java/com/bumptech/glide/manager/DefaultConnectivityMonitor.java b/library/src/main/java/com/bumptech/glide/manager/DefaultConnectivityMonitor.java
index f04a0ad718..d49c8503e0 100644
--- a/library/src/main/java/com/bumptech/glide/manager/DefaultConnectivityMonitor.java
+++ b/library/src/main/java/com/bumptech/glide/manager/DefaultConnectivityMonitor.java
@@ -26,16 +26,7 @@ final class DefaultConnectivityMonitor implements ConnectivityMonitor {
     @Override
     public void onReceive(Context context, Intent intent) {
       boolean wasConnected = isConnected;
-      try {
-        isConnected = isConnected(context);
-      } catch (SecurityException e) {
-        // See #1405.
-        if (Log.isLoggable(TAG, Log.WARN)) {
-          Log.w(TAG, "Failed to determine connectivity status when connectivity changed", e);
-        }
-        // Default to true;
-        isConnected = true;
-      }
+      isConnected = isConnected(context);
       if (wasConnected != isConnected) {
         if (Log.isLoggable(TAG, Log.DEBUG)) {
           Log.d(TAG, "connectivity changed, isConnected: " + isConnected);
@@ -56,14 +47,15 @@ private void register() {
       return;
     }
 
+    // Initialize isConnected.
+    isConnected = isConnected(context);
     try {
       // See #1405
-      isConnected = isConnected(context);
       context.registerReceiver(connectivityReceiver,
           new IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION));
       isRegistered = true;
     } catch (SecurityException e) {
-      // See #1417.
+      // See #1417, registering the receiver can throw SecurityException.
       if (Log.isLoggable(TAG, Log.WARN)) {
         Log.w(TAG, "Failed to register", e);
       }
@@ -85,9 +77,21 @@ private void unregister() {
   @SuppressLint("MissingPermission")
   boolean isConnected(Context context) {
     ConnectivityManager connectivityManager =
-        (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE);
-    NetworkInfo networkInfo =
-        Preconditions.checkNotNull(connectivityManager).getActiveNetworkInfo();
+        Preconditions.checkNotNull(
+            (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE));
+    NetworkInfo networkInfo;
+    try {
+      networkInfo = connectivityManager.getActiveNetworkInfo();
+    } catch (RuntimeException e) {
+      // #1405 shows that this throws a SecurityException.
+      // b/70869360 shows that this throws NullPointerException on APIs 22, 23, and 24.
+      // b/70869360 also shows that this throws RuntimeException on API 24 and 25.
+      if (Log.isLoggable(TAG, Log.WARN)) {
+        Log.w(TAG, "Failed to determine connectivity status when connectivity changed", e);
+      }
+      // Default to true;
+      return true;
+    }
     return networkInfo != null && networkInfo.isConnected();
   }
 
diff --git a/library/src/test/java/com/bumptech/glide/manager/DefaultConnectivityMonitorTest.java b/library/src/test/java/com/bumptech/glide/manager/DefaultConnectivityMonitorTest.java
index 4504f9df31..79ebb79ce8 100644
--- a/library/src/test/java/com/bumptech/glide/manager/DefaultConnectivityMonitorTest.java
+++ b/library/src/test/java/com/bumptech/glide/manager/DefaultConnectivityMonitorTest.java
@@ -125,15 +125,6 @@ public void register_withMissingPermission_doesNotThrow() {
     monitor.onStart();
   }
 
-  @Test
-  public void register_withMissingPermission_doesNotRegisterReceiver() {
-    harness.shadowConnectivityManager.isNetworkPermissionGranted = false;
-
-    monitor.onStart();
-
-    assertThat(getConnectivityReceivers()).isEmpty();
-  }
-
   @Test
   public void onReceive_withMissingPermission_doesNotThrow() {
     monitor.onStart();