Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Commit

Permalink
Closes #9553: Sanitize downloads content type
Browse files Browse the repository at this point in the history
  • Loading branch information
Amejia481 authored and mergify[bot] committed Feb 1, 2021
1 parent a9044cf commit 37d1148
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 9 deletions.
Expand Up @@ -821,7 +821,7 @@ class GeckoEngineSession(
onExternalResource(
url = url,
contentLength = contentLength,
contentType = contentType,
contentType = DownloadUtils.sanitizeMimeType(contentType),
fileName = fileName.sanitizeFileName(),
response = response,
isPrivate = privateMode
Expand Down
Expand Up @@ -821,7 +821,7 @@ class GeckoEngineSession(
onExternalResource(
url = url,
contentLength = contentLength,
contentType = contentType,
contentType = DownloadUtils.sanitizeMimeType(contentType),
fileName = fileName.sanitizeFileName(),
response = response,
isPrivate = privateMode
Expand Down
Expand Up @@ -821,7 +821,7 @@ class GeckoEngineSession(
onExternalResource(
url = url,
contentLength = contentLength,
contentType = contentType,
contentType = DownloadUtils.sanitizeMimeType(contentType),
fileName = fileName.sanitizeFileName(),
response = response,
isPrivate = privateMode
Expand Down
Expand Up @@ -934,7 +934,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
}

Expand All @@ -946,6 +946,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
Expand Down
Expand Up @@ -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
Expand Down Expand Up @@ -1682,4 +1683,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)
}
}
Expand Up @@ -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
Expand Down Expand Up @@ -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()
}

/**
Expand Down
1 change: 1 addition & 0 deletions docs/changelog.md
Expand Up @@ -22,6 +22,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`.
Expand Down

0 comments on commit 37d1148

Please sign in to comment.