Closes mozilla-mobile#9553: Sanitize downloads content type

Amejia481 · Jan 29, 2021 · 41f8350 · 41f8350
1 parent a8a2833
commit 41f8350
Show file tree

Hide file tree

Showing 7 changed files with 65 additions and 9 deletions.
diff --git a/...ne-gecko-beta/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt b/...ne-gecko-beta/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt
@@ -821,7 +821,7 @@ class GeckoEngineSession(
                     onExternalResource(
                             url = url,
                             contentLength = contentLength,
-                            contentType = contentType,
+                            contentType = DownloadUtils.sanitizeMimeType(contentType),
                             fileName = fileName.sanitizeFileName(),
                             response = response,
                             isPrivate = privateMode

diff --git a/...gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt b/...gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt
@@ -821,7 +821,7 @@ class GeckoEngineSession(
                     onExternalResource(
                             url = url,
                             contentLength = contentLength,
-                            contentType = contentType,
+                            contentType = DownloadUtils.sanitizeMimeType(contentType),
                             fileName = fileName.sanitizeFileName(),
                             response = response,
                             isPrivate = privateMode

diff --git a/.../engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt b/.../engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngineSession.kt
@@ -821,7 +821,7 @@ class GeckoEngineSession(
                     onExternalResource(
                             url = url,
                             contentLength = contentLength,
-                            contentType = contentType,
+                            contentType = DownloadUtils.sanitizeMimeType(contentType),
                             fileName = fileName.sanitizeFileName(),
                             response = response,
                             isPrivate = privateMode

diff --git a/...nloads/src/main/java/mozilla/components/feature/downloads/AbstractFetchDownloadService.kt b/...nloads/src/main/java/mozilla/components/feature/downloads/AbstractFetchDownloadService.kt
@@ -936,7 +936,7 @@ abstract class AbstractFetchDownloadService : Service() {
             )
 
             val newIntent = Intent(ACTION_VIEW).apply {
-                setDataAndType(constructedFilePath, contentType ?: "*/*")
+                setDataAndType(constructedFilePath, getSafeContentType(context, constructedFilePath, contentType))
                 flags = Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_GRANT_READ_URI_PERMISSION
             }
 
@@ -948,6 +948,21 @@ abstract class AbstractFetchDownloadService : Service() {
             }
         }
 
+        @VisibleForTesting
+        internal fun getSafeContentType(context: Context, constructedFilePath: Uri, contentType: String?): String {
+            val contentTypeFromFile = context.contentResolver.getType(constructedFilePath)
+            val resultContentType = if (!contentTypeFromFile.isNullOrEmpty()) {
+                contentTypeFromFile
+            } else {
+                if (!contentType.isNullOrEmpty()) {
+                    contentType
+                } else {
+                    "*/*"
+                }
+            }
+            return (DownloadUtils.sanitizeMimeType(resultContentType) ?: "*/*")
+        }
+
         private const val FILE_PROVIDER_EXTENSION = ".feature.downloads.fileprovider"
         private const val CHUNK_SIZE = 4 * 1024
         private const val PARTIAL_CONTENT_STATUS = 206

diff --git a/...ds/src/test/java/mozilla/components/feature/downloads/AbstractFetchDownloadServiceTest.kt b/...ds/src/test/java/mozilla/components/feature/downloads/AbstractFetchDownloadServiceTest.kt
@@ -9,6 +9,7 @@ import android.app.DownloadManager.EXTRA_DOWNLOAD_ID
 import android.app.Notification
 import android.app.NotificationManager
 import android.app.Service
+import android.content.ContentResolver
 import android.content.Context
 import android.content.Intent
 import android.os.Build
@@ -1683,4 +1684,45 @@ class AbstractFetchDownloadServiceTest {
         assertEquals(15, downloadJobState.currentBytesCopied)
         assertEquals(AbstractFetchDownloadService.CopyInChuckStatus.COMPLETED, status)
     }
+
+    @Test
+    fun `getSafeContentType - WHEN the file content type is available THEN use it`() {
+        val contentTypeFromFile = "application/pdf; qs=0.001"
+        val spyContext = spy(testContext)
+        val contentResolver = mock<ContentResolver>()
+
+        doReturn(contentTypeFromFile).`when`(contentResolver).getType(any())
+        doReturn(contentResolver).`when`(spyContext).contentResolver
+
+        val result = AbstractFetchDownloadService.getSafeContentType(spyContext, mock(), "any")
+
+        assertEquals("application/pdf", result)
+    }
+
+    @Test
+    fun `getSafeContentType - WHEN the file content type is not available THEN use the provided content type`() {
+        val contentType = " application/pdf "
+        val spyContext = spy(testContext)
+        val contentResolver = mock<ContentResolver>()
+
+        doReturn(null).`when`(contentResolver).getType(any())
+        doReturn(contentResolver).`when`(spyContext).contentResolver
+
+        val result = AbstractFetchDownloadService.getSafeContentType(spyContext, mock(), contentType)
+
+        assertEquals("application/pdf", result)
+    }
+
+    @Test
+    fun `getSafeContentType - WHEN none of the provided content types are available THEN return a generic content type`() {
+        val spyContext = spy(testContext)
+        val contentResolver = mock<ContentResolver>()
+
+        doReturn(null).`when`(contentResolver).getType(any())
+        doReturn(contentResolver).`when`(spyContext).contentResolver
+
+        val result = AbstractFetchDownloadService.getSafeContentType(spyContext, mock(), null)
+
+        assertEquals("*/*", result)
+    }
 }
diff --git a/components/support/utils/src/main/java/mozilla/components/support/utils/DownloadUtils.kt b/components/support/utils/src/main/java/mozilla/components/support/utils/DownloadUtils.kt
@@ -7,7 +7,6 @@ package mozilla.components.support.utils
 import android.net.Uri
 import android.os.Environment
 import android.webkit.MimeTypeMap
-import androidx.annotation.VisibleForTesting
 import java.io.ByteArrayOutputStream
 import java.io.File
 import java.io.UnsupportedEncodingException
@@ -172,17 +171,16 @@ object DownloadUtils {
 
     // Some site add extra information after the mimetype, for example 'application/pdf; qs=0.001'
     // we just want to extract the mimeType and ignore the rest.
-    @VisibleForTesting
-    internal fun sanitizeMimeType(mimeType: String?): String? {
-        return if (mimeType != null) {
+    fun sanitizeMimeType(mimeType: String?): String? {
+        return (if (mimeType != null) {
             if (mimeType.contains(";")) {
                 mimeType.substringBefore(";")
             } else {
                 mimeType
             }
         } else {
             null
-        }
+        })?.trim()
     }
 
     /**

diff --git a/docs/changelog.md b/docs/changelog.md
@@ -15,6 +15,7 @@ permalink: /changelog/
 * **feature-downloads**:
   * 🚒 Bug fixed [issue #9441](https://github.com/mozilla-mobile/android-components/issues/9441) - Don't ask for redundant system files permission if not required.
   * 🚒 Bug fixed [issue #9526](https://github.com/mozilla-mobile/android-components/issues/9526) - Downloads with generic content types use the correct file extension.
+  * 🚒 Bug fixed [issue #9553](https://github.com/mozilla-mobile/android-components/issues/9553) - Multiple files were unable to be opened after being download.
 
 * **feature-webauthn**
   * 🆕 New component to enable support for WebAuthn specification with `WebAuthnFeature`.