Commit c0159f71 authored by Sawyer Blatz's avatar Sawyer Blatz
Browse files

Fixes #5046: Updates guessFileName to use uniqueFileName

parent 8acc806a
......@@ -566,7 +566,8 @@ class SystemEngineView @JvmOverloads constructor(
internal fun createDownloadListener(): DownloadListener {
return DownloadListener { url, userAgent, contentDisposition, mimetype, contentLength ->
session?.internalNotifyObservers {
val fileName = DownloadUtils.guessFileName(contentDisposition, url, mimetype)
val fileName = DownloadUtils.guessFileName(contentDisposition, null, url, mimetype)
val cookie = CookieManager.getInstance().getCookie(url)
onExternalResource(url, fileName, contentLength, mimetype, cookie, userAgent)
}
......
......@@ -45,6 +45,7 @@ import mozilla.components.feature.downloads.facts.emitNotificationPauseFact
import mozilla.components.feature.downloads.facts.emitNotificationCancelFact
import mozilla.components.feature.downloads.facts.emitNotificationTryAgainFact
import mozilla.components.feature.downloads.facts.emitNotificationOpenFact
import mozilla.components.support.utils.DownloadUtils
import java.io.File
import java.io.FileOutputStream
import java.io.IOException
......@@ -329,15 +330,24 @@ abstract class AbstractFetchDownloadService : Service() {
*
* Encapsulates different behaviour depending on the SDK version.
*/
@Suppress("Deprecation")
internal fun useFileStream(
download: DownloadState,
append: Boolean,
block: (OutputStream) -> Unit
) {
// Update the file name to ensure it doesn't collide with one already on disk]
val downloadWithUniqueName = download.fileName?.let {
download.copy(fileName = DownloadUtils.uniqueFileName(
Environment.getExternalStoragePublicDirectory(download.destinationDirectory),
it
))
} ?: download
if (SDK_INT >= Build.VERSION_CODES.Q) {
useFileStreamScopedStorage(download, block)
useFileStreamScopedStorage(downloadWithUniqueName, block)
} else {
useFileStreamLegacy(download, append, block)
useFileStreamLegacy(downloadWithUniqueName, append, block)
}
}
......
......@@ -33,7 +33,7 @@ abstract class DownloadDialogFragment : AppCompatDialogFragment() {
fun setDownload(download: DownloadState) {
val args = arguments ?: Bundle()
args.putString(KEY_FILE_NAME, download.fileName
?: DownloadUtils.guessFileName(null, download.url, download.contentType))
?: DownloadUtils.guessFileName(null, download.destinationDirectory, download.url, download.contentType))
args.putString(KEY_URL, download.url)
args.putLong(KEY_CONTENT_LENGTH, download.contentLength ?: 0)
arguments = args
......
......@@ -5,7 +5,6 @@
package mozilla.components.feature.downloads
import android.content.Context
import android.os.Environment
import android.widget.Toast
import androidx.annotation.ColorRes
import androidx.annotation.VisibleForTesting
......@@ -31,7 +30,6 @@ import mozilla.components.support.base.feature.PermissionsFeature
import mozilla.components.support.ktx.android.content.appName
import mozilla.components.support.ktx.android.content.isPermissionGranted
import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifChanged
import java.io.File
/**
* Feature implementation to provide download functionality for the selected
......@@ -96,15 +94,7 @@ class DownloadsFeature(
.collect { state ->
val download = state.content.download
if (download != null) {
// Update the file name to ensure it doesn't collide with one already on disk]
val downloadWithUniqueName = download.fileName?.let {
download.copy(fileName = uniqueFileName(
Environment.getExternalStoragePublicDirectory(download.destinationDirectory),
it
))
} ?: download
processDownload(state, downloadWithUniqueName)
processDownload(state, download)
}
}
}
......@@ -220,24 +210,6 @@ class DownloadsFeature(
block(Pair(state, download))
}
/**
* Checks if the file exists so as not to overwrite one already in downloads
*/
private fun uniqueFileName(directory: File, fileName: String): String {
val fileExtension = fileName.substringAfterLast(".")
val baseFileName = fileName.replace(fileExtension, "")
var potentialFileName = File(directory, fileName)
var copyVersionNumber = 1
while (potentialFileName.exists()) {
potentialFileName = File(directory, "$baseFileName($copyVersionNumber).$fileExtension")
copyVersionNumber += 1
}
return potentialFileName.name
}
/**
* Styling for the download dialog prompt
*/
......
......@@ -37,7 +37,7 @@ internal fun DownloadState.withResponse(headers: Headers, stream: InputStream?):
return copy(
fileName = if (fileName.isNullOrBlank()) {
DownloadUtils.guessFileName(contentDisposition, url, contentType)
DownloadUtils.guessFileName(contentDisposition, destinationDirectory, url, contentType)
} else {
fileName
},
......
......@@ -141,7 +141,7 @@ private fun DownloadState.toAndroidRequest(cookie: String): SystemRequest {
}
val fileName = if (fileName.isNullOrBlank()) {
DownloadUtils.guessFileName(null, url, contentType)
DownloadUtils.guessFileName(null, destinationDirectory, url, contentType)
} else {
fileName
}
......
......@@ -5,8 +5,10 @@
package mozilla.components.support.utils
import android.net.Uri
import android.os.Environment
import android.webkit.MimeTypeMap
import java.io.ByteArrayOutputStream
import java.io.File
import java.io.UnsupportedEncodingException
import java.util.regex.Pattern
......@@ -40,17 +42,50 @@ object DownloadUtils {
* This method is largely identical to [android.webkit.URLUtil.guessFileName]
* which unfortunately does not implement RFC 5987.
*/
@JvmStatic
fun guessFileName(contentDisposition: String?, url: String?, mimeType: String?): String {
val filename = extractFileNameFromUrl(contentDisposition, url)
// Split filename between base and extension
@Suppress("Deprecation")
@JvmStatic
fun guessFileName(
contentDisposition: String?,
destinationDirectory: String?,
url: String?,
mimeType: String?
): String {
// Split fileName between base and extension
// Add an extension if filename does not have one
return if (filename.contains('.')) {
changeExtension(filename, mimeType)
val extractedFileName = extractFileNameFromUrl(contentDisposition, url)
val fileName = if (extractedFileName.contains('.')) {
changeExtension(extractedFileName, mimeType)
} else {
filename + createExtension(mimeType)
extractedFileName + createExtension(mimeType)
}
return destinationDirectory?.let {
uniqueFileName(Environment.getExternalStoragePublicDirectory(destinationDirectory), fileName)
} ?: fileName
}
/**
* Checks if the file exists so as not to overwrite one already in the destination directory
*/
fun uniqueFileName(directory: File, fileName: String): String {
var fileExtension = ".${fileName.substringAfterLast(".")}"
// Check if an extension was found or not
if (fileExtension == ".$fileName") { fileExtension = "" }
val baseFileName = fileName.replace(fileExtension, "")
var potentialFileName = File(directory, fileName)
var copyVersionNumber = 1
while (potentialFileName.exists()) {
potentialFileName = File(directory, "$baseFileName($copyVersionNumber)$fileExtension")
copyVersionNumber += 1
}
return potentialFileName.name
}
private fun extractFileNameFromUrl(contentDisposition: String?, url: String?): String {
......
......@@ -7,15 +7,20 @@ package mozilla.components.support.utils
import android.webkit.MimeTypeMap
import androidx.test.ext.junit.runners.AndroidJUnit4
import org.junit.Assert.assertEquals
import org.junit.Rule
import org.junit.Test
import org.junit.rules.TemporaryFolder
import org.junit.runner.RunWith
import org.robolectric.Shadows
@RunWith(AndroidJUnit4::class)
class DownloadUtilsTest {
@Rule @JvmField
val folder = TemporaryFolder()
private fun assertContentDisposition(expected: String, contentDisposition: String) {
assertEquals(expected, DownloadUtils.guessFileName(contentDisposition, null, null))
assertEquals(expected, DownloadUtils.guessFileName(contentDisposition, null, null, null))
}
@Test
......@@ -55,6 +60,22 @@ class DownloadUtilsTest {
}
}
@Test
fun uniqueFilenameNoExtension() {
assertEquals("test", DownloadUtils.uniqueFileName(folder.root, "test"))
folder.newFile("test")
assertEquals("test(1)", DownloadUtils.uniqueFileName(folder.root, "test"))
}
@Test
fun uniqueFilename() {
assertEquals("test.zip", DownloadUtils.uniqueFileName(folder.root, "test.zip"))
folder.newFile("test.zip")
assertEquals("test(1).zip", DownloadUtils.uniqueFileName(folder.root, "test.zip"))
}
@Test
fun guessFileName_url() {
assertUrl("downloadfile.bin", "http://example.com/")
......@@ -69,15 +90,15 @@ class DownloadUtilsTest {
Shadows.shadowOf(MimeTypeMap.getSingleton()).addExtensionMimeTypMapping("zip", "application/zip")
Shadows.shadowOf(MimeTypeMap.getSingleton()).addExtensionMimeTypMapping("tar.gz", "application/gzip")
assertEquals("file.jpg", DownloadUtils.guessFileName(null, "http://example.com/file.jpg", "image/jpeg"))
assertEquals("file.jpg", DownloadUtils.guessFileName(null, "http://example.com/file.bin", "image/jpeg"))
assertEquals("file.jpg", DownloadUtils.guessFileName(null, null, "http://example.com/file.jpg", "image/jpeg"))
assertEquals("file.jpg", DownloadUtils.guessFileName(null, null, "http://example.com/file.bin", "image/jpeg"))
assertEquals(
"Caesium-wahoo-v3.6-b792615ced1b.zip",
DownloadUtils.guessFileName(null, "https://download.msfjarvis.website/caesium/wahoo/beta/Caesium-wahoo-v3.6-b792615ced1b.zip", "application/zip")
DownloadUtils.guessFileName(null, null, "https://download.msfjarvis.website/caesium/wahoo/beta/Caesium-wahoo-v3.6-b792615ced1b.zip", "application/zip")
)
assertEquals(
"compressed.TAR.GZ",
DownloadUtils.guessFileName(null, "http://example.com/compressed.TAR.GZ", "application/gzip")
DownloadUtils.guessFileName(null, null, "http://example.com/compressed.TAR.GZ", "application/gzip")
)
}
......@@ -85,7 +106,7 @@ class DownloadUtilsTest {
private val CONTENT_DISPOSITION_TYPES = listOf("attachment", "inline")
private fun assertUrl(expected: String, url: String) {
assertEquals(expected, DownloadUtils.guessFileName(null, url, null))
assertEquals(expected, DownloadUtils.guessFileName(null, null, url, null))
}
}
}
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment