[components] Closes mozilla-mobile/android-components#8567: Prevent c…

…rashes when trying to add to the system databases
mozilla-mobile · Feb 17, 2022 · d4d7fe3 · d4d7fe3
1 parent 905d767
commit d4d7fe3
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 23 deletions.
diff --git a/...nloads/src/main/java/mozilla/components/feature/downloads/AbstractFetchDownloadService.kt b/...nloads/src/main/java/mozilla/components/feature/downloads/AbstractFetchDownloadService.kt
@@ -451,7 +451,6 @@ abstract class AbstractFetchDownloadService : Service() {
                 ?: throw IllegalStateException("A fileName for a download is required")
             val file = File(download.filePath)
             // addCompletedDownload can't handle any non http(s) urls
-            val url = if (!download.isScheme(listOf("http", "https"))) null else download.url.toUri()
             scope.launch {
                 addCompletedDownload(
                     title = fileName,
@@ -462,8 +461,7 @@ abstract class AbstractFetchDownloadService : Service() {
                     length = download.contentLength ?: file.length(),
                     // Only show notifications if our channel is blocked
                     showNotification = !DownloadNotification.isChannelEnabled(context),
-                    uri = url,
-                    referer = download.referrerUrl?.toUri()
+                    download
                 )
             }
         }
@@ -479,21 +477,25 @@ abstract class AbstractFetchDownloadService : Service() {
         path: String,
         length: Long,
         showNotification: Boolean,
-        uri: Uri?,
-        referer: Uri?
+        download: DownloadState
     ) {
-        context.addCompletedDownload(
-            title = title,
-            description = description,
-            isMediaScannerScannable = isMediaScannerScannable,
-            mimeType = mimeType,
-            path = path,
-            length = length,
-            // Only show notifications if our channel is blocked
-            showNotification = showNotification,
-            uri = uri,
-            referer = referer
-        )
+        try {
+            val url = if (!download.isScheme(listOf("http", "https"))) null else download.url.toUri()
+            context.addCompletedDownload(
+                title = title,
+                description = description,
+                isMediaScannerScannable = isMediaScannerScannable,
+                mimeType = mimeType,
+                path = path,
+                length = length,
+                // Only show notifications if our channel is blocked
+                showNotification = showNotification,
+                uri = url,
+                referer = download.referrerUrl?.toUri()
+            )
+        } catch (e: IllegalArgumentException) {
+            logger.error("Unable add the download to the system database", e)
+        }
     }
 
     @VisibleForTesting

diff --git a/...ds/src/test/java/mozilla/components/feature/downloads/AbstractFetchDownloadServiceTest.kt b/...ds/src/test/java/mozilla/components/feature/downloads/AbstractFetchDownloadServiceTest.kt
@@ -68,6 +68,7 @@ import org.junit.Assert.assertNotEquals
 import org.junit.Assert.assertNotNull
 import org.junit.Assert.assertNull
 import org.junit.Assert.assertTrue
+import org.junit.Assert.fail
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
@@ -1470,8 +1471,7 @@ class AbstractFetchDownloadServiceTest {
             path = any(),
             length = anyLong(),
             showNotification = anyBoolean(),
-            uri = any(),
-            referer = any()
+            download = any()
         )
     }
 
@@ -1499,8 +1499,7 @@ class AbstractFetchDownloadServiceTest {
             path = any(),
             length = anyLong(),
             showNotification = anyBoolean(),
-            uri = any(),
-            referer = any()
+            download = any()
         )
         doReturn(true).`when`(service).shouldUseScopedStorage()
 
@@ -1514,8 +1513,7 @@ class AbstractFetchDownloadServiceTest {
             path = any(),
             length = anyLong(),
             showNotification = anyBoolean(),
-            uri = any(),
-            referer = any()
+            download = any()
         )
     }
 
@@ -1580,6 +1578,41 @@ class AbstractFetchDownloadServiceTest {
         verify(downloadManager).addCompletedDownload(anyString(), anyString(), anyBoolean(), anyString(), anyString(), anyLong(), anyBoolean(), isNull(), any())
     }
 
+    @Test
+    @Config(sdk = [Build.VERSION_CODES.P], shadows = [ShadowFileProvider::class])
+    @Suppress("Deprecation")
+    fun `GIVEN a download that throws an exception WHEN adding to the system database THEN handle the exception`() =
+        runBlockingTest {
+            val download = DownloadState(
+                url = "url",
+                fileName = "example.apk",
+                destinationDirectory = folder.root.path
+            )
+
+            val service = spy(object : AbstractFetchDownloadService() {
+                override val httpClient = client
+                override val store = browserStore
+            })
+
+            val spyContext = spy(testContext)
+            val downloadManager: DownloadManager = mock()
+
+            doReturn(spyContext).`when`(service).context
+            doReturn(downloadManager).`when`(spyContext).getSystemService<DownloadManager>()
+
+            doAnswer { throw IllegalArgumentException() }.`when`(downloadManager)
+                .addCompletedDownload(
+                    anyString(), anyString(), anyBoolean(), anyString(),
+                    anyString(), anyLong(), anyBoolean(), isNull(), any()
+                )
+
+            try {
+                service.addToDownloadSystemDatabaseCompat(download, this)
+            } catch (e: IOException) {
+                fail()
+            }
+        }
+
     @Test
     @Config(sdk = [Build.VERSION_CODES.P], shadows = [ShadowFileProvider::class])
     @Suppress("Deprecation")

diff --git a/android-components/docs/changelog.md b/android-components/docs/changelog.md
@@ -16,6 +16,8 @@ permalink: /changelog/
 * **support-utils**
   * 🌟️️ **Added new Browsers constant for Fennec `Browsers.FIREFOX_FENNEC_NIGHTLY`.
   * ⚠️ **This is a breaking change**: `Browsers.FIREFOX_NIGHTLY` now points to `org.mozilla.fenix`, for fennec nightly use `Browsers.FIREFOX_FENNEC_NIGHTLY` [#11682](https://github.com/mozilla-mobile/android-components/pull/11682).
+* **feature-downloads**:
+  * 🚒 Bug fixed [issue #8567](https://github.com/mozilla-mobile/android-components/issues/8567) - Prevent crashes when trying to add to the system databases.
 
 * **concept-engine**
   * 🌟️️ Add `EngineSessionStateStorage`, describing a storage of `EngineSessionState` instances.