diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/downloader/UriUtilsFilenameExtractorTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/downloader/UriUtilsFilenameExtractorTest.kt new file mode 100644 index 000000000000..7da76d7a3227 --- /dev/null +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/downloader/UriUtilsFilenameExtractorTest.kt @@ -0,0 +1,152 @@ +/* + * Copyright (c) 2020 DuckDuckGo + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.duckduckgo.app.browser.downloader + +import org.junit.Assert.assertEquals +import org.junit.Test + +class UriUtilsFilenameExtractorTest { + + private val testee: FilenameExtractor = FilenameExtractor() + + @Test + fun whenUrlEndsWithFilenameAsJpgNoMimeOrContentDispositionThenFilenameShouldBeExtracted() { + val url = "https://example.com/realFilename.jpg" + val mimeType: String? = null + val contentDisposition: String? = null + val extracted = testee.extract(buildPendingDownload(url, contentDisposition, mimeType)) + assertEquals("realFilename.jpg", extracted) + } + + @Test + fun whenUrlContainsFilenameButContainsAdditionalPathSegmentsThenFilenameShouldBeExtracted() { + val url = "https://foo.example.com/path/images/b/b1/realFilename.jpg/other/stuff" + val mimeType: String? = null + val contentDisposition: String? = null + val extracted = testee.extract(buildPendingDownload(url, contentDisposition, mimeType)) + assertEquals("realFilename.jpg", extracted) + } + + @Test + fun whenUrlContainsFilenameButContainsAdditionalPathSegmentsAndQueryParamsThenFilenameShouldBeExtracted() { + val url = "https://foo.example.com/path/images/b/b1/realFilename.jpg/other/stuff?cb=123" + val mimeType: String? = null + val contentDisposition: String? = null + val extracted = testee.extract(buildPendingDownload(url, contentDisposition, mimeType)) + assertEquals("realFilename.jpg", extracted) + } + + @Test + fun whenUrlContainsFilenameButContainsAdditionalPathSegmentsAndQueryParamsWhichLookLikeAFilenameThenFilenameShouldBeExtracted() { + val url = "https://foo.example.com/path/images/b/b1/realFilename.jpg/other/stuff?cb=123.com" + val mimeType: String? = null + val contentDisposition: String? = null + val extracted = testee.extract(buildPendingDownload(url, contentDisposition, mimeType)) + assertEquals("realFilename.jpg", extracted) + } + + @Test + fun whenUrlContainsFilenameButContentDispositionSaysOtherwiseThenExtractFromContentDisposition() { + val url = "https://example.com/filename.jpg" + val mimeType: String? = null + val contentDisposition: String? = "Content-Disposition: attachment; filename=fromDisposition.jpg" + val extracted = testee.extract(buildPendingDownload(url, contentDisposition, mimeType)) + assertEquals("fromDisposition.jpg", extracted) + } + + @Test + fun whenFilenameEndsInBinThenThatIsExtracted() { + val url = "https://example.com/realFilename.bin" + val mimeType: String? = null + val contentDisposition: String? = null + val extracted = testee.extract(buildPendingDownload(url, contentDisposition, mimeType)) + assertEquals("realFilename.bin", extracted) + } + + @Test + fun whenUrlContainsNoFileNameButLotsOfPathsSegmentsThenFirstSegmentNameIsUsed() { + val url = "https://example.com/foo/bar/files" + val mimeType: String? = null + val contentDisposition: String? = null + val extracted = testee.extract(buildPendingDownload(url, contentDisposition, mimeType)) + assertEquals("foo.bin", extracted) + } + + @Test + fun whenFilenameEndsInBinWithASlashThenThatIsExtracted() { + val url = "https://example.com/realFilename.bin/" + val mimeType: String? = null + val contentDisposition: String? = null + val extracted = testee.extract(buildPendingDownload(url, contentDisposition, mimeType)) + assertEquals("realFilename.bin", extracted) + } + + @Test + fun whenFilenameContainsBinThenThatIsExtracted() { + val url = "https://example.com/realFilename.bin/foo/bar" + val mimeType: String? = null + val contentDisposition: String? = null + val extracted = testee.extract(buildPendingDownload(url, contentDisposition, mimeType)) + assertEquals("realFilename.bin", extracted) + } + + @Test + fun whenUrlIsEmptyStringAndNoOtherDataProvidedThenDefaultNameAndBinFiletypeReturned() { + val url = "" + val mimeType: String? = null + val contentDisposition: String? = null + val extracted = testee.extract(buildPendingDownload(url, contentDisposition, mimeType)) + assertEquals("downloadfile.bin", extracted) + } + + @Test + fun whenUrlIsEmptyStringAndMimeTypeProvidedThenDefaultNameAndFiletypeFromMimeReturned() { + val url = "" + val mimeType: String? = "image/jpeg" + val contentDisposition: String? = null + val extracted = testee.extract(buildPendingDownload(url, contentDisposition, mimeType)) + assertEquals("downloadfile.jpg", extracted) + } + + @Test + fun whenUrlIsEmptyStringAndContentDispositionProvidedThenExtractFromContentDisposition() { + val url = "" + val mimeType: String? = null + val contentDisposition: String? = "Content-Disposition: attachment; filename=fromDisposition.jpg" + val extracted = testee.extract(buildPendingDownload(url, contentDisposition, mimeType)) + assertEquals("fromDisposition.jpg", extracted) + } + + @Test + fun whenNoFilenameAndNoPathSegmentsThenDomainNameReturned() { + val url = "http://example.com" + val mimeType: String? = null + val contentDisposition: String? = null + val extracted = testee.extract(buildPendingDownload(url, contentDisposition, mimeType)) + assertEquals("example.com", extracted) + } + + private fun buildPendingDownload(url: String, contentDisposition: String?, mimeType: String?): FileDownloader.PendingFileDownload { + return FileDownloader.PendingFileDownload( + url = url, + contentDisposition = contentDisposition, + mimeType = mimeType, + subfolder = "aFolder", + userAgent = "aUserAgent" + ) + } +} diff --git a/app/src/main/java/com/duckduckgo/app/browser/DownloadConfirmationFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/DownloadConfirmationFragment.kt index 9e1177a9f984..c2eee6d1e324 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/DownloadConfirmationFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/DownloadConfirmationFragment.kt @@ -22,7 +22,7 @@ import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import com.duckduckgo.app.browser.downloader.FileDownloader.PendingFileDownload -import com.duckduckgo.app.browser.downloader.guessFileName +import com.duckduckgo.app.browser.downloader.FilenameExtractor import com.duckduckgo.app.browser.downloader.isDataUrl import com.duckduckgo.app.global.view.gone import com.duckduckgo.app.global.view.leftDrawable @@ -32,12 +32,16 @@ import dagger.android.support.AndroidSupportInjection import kotlinx.android.synthetic.main.download_confirmation.view.* import timber.log.Timber import java.io.File +import javax.inject.Inject class DownloadConfirmationFragment : BottomSheetDialogFragment() { val listener: DownloadConfirmationDialogListener get() = parentFragment as DownloadConfirmationDialogListener + @Inject + lateinit var filenameExtractor: FilenameExtractor + private var file: File? = null private val pendingDownload: PendingFileDownload by lazy { @@ -57,7 +61,7 @@ class DownloadConfirmationFragment : BottomSheetDialogFragment() { } private fun setupDownload() { - file = if (!pendingDownload.isDataUrl) File(pendingDownload.directory, pendingDownload.guessFileName()) else null + file = if (!pendingDownload.isDataUrl) File(pendingDownload.directory, filenameExtractor.extract(pendingDownload)) else null } private fun setupViews(view: View) { diff --git a/app/src/main/java/com/duckduckgo/app/browser/downloader/FileDownloader.kt b/app/src/main/java/com/duckduckgo/app/browser/downloader/FileDownloader.kt index 72430a69ee90..7a37a367fb7a 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/downloader/FileDownloader.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/downloader/FileDownloader.kt @@ -22,7 +22,6 @@ import android.webkit.URLUtil import androidx.annotation.WorkerThread import com.duckduckgo.app.browser.downloader.FileDownloader.FileDownloadListener import com.duckduckgo.app.browser.downloader.FileDownloader.PendingFileDownload -import timber.log.Timber import java.io.File import java.io.Serializable import javax.inject.Inject @@ -67,12 +66,6 @@ class AndroidFileDownloader @Inject constructor( } } -fun PendingFileDownload.guessFileName(): String { - val guessedFileName = URLUtil.guessFileName(url, contentDisposition, mimeType) - Timber.i("Guessed filename of $guessedFileName for url $url") - return guessedFileName -} - val PendingFileDownload.isDataUrl get() = URLUtil.isDataUrl(url) val PendingFileDownload.isNetworkUrl get() = URLUtil.isNetworkUrl(url) diff --git a/app/src/main/java/com/duckduckgo/app/browser/downloader/FilenameExtractor.kt b/app/src/main/java/com/duckduckgo/app/browser/downloader/FilenameExtractor.kt new file mode 100644 index 000000000000..78c170ae6de7 --- /dev/null +++ b/app/src/main/java/com/duckduckgo/app/browser/downloader/FilenameExtractor.kt @@ -0,0 +1,100 @@ +/* + * Copyright (c) 2020 DuckDuckGo + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.duckduckgo.app.browser.downloader + +import android.webkit.URLUtil +import androidx.core.net.toUri +import com.duckduckgo.app.browser.downloader.FileDownloader.PendingFileDownload +import com.duckduckgo.app.browser.downloader.FilenameExtractor.GuessQuality.NotGoodEnough +import com.duckduckgo.app.browser.downloader.FilenameExtractor.GuessQuality.TriedAllOptions +import timber.log.Timber +import javax.inject.Inject + +class FilenameExtractor @Inject constructor() { + + fun extract(pendingDownload: PendingFileDownload): String { + val url = pendingDownload.url + val mimeType = pendingDownload.mimeType + val contentDisposition = pendingDownload.contentDisposition + + val firstGuess = guessFilename(url, contentDisposition, mimeType) + val guesses = Guesses(bestGuess = null, latestGuess = firstGuess) + val baseUrl = url.toUri().host ?: "" + var pathSegments = pathSegments(url) + + while (evaluateGuessQuality(guesses, pathSegments) != TriedAllOptions) { + pathSegments = pathSegments.dropLast(1) + guesses.latestGuess = guessFilename(baseUrl + "/" + pathSegments.rebuildUrl(), contentDisposition, mimeType) + } + + return bestGuess(guesses) + } + + private fun evaluateGuessQuality(guesses: Guesses, pathSegments: List): GuessQuality { + val latestGuess = guesses.latestGuess + + // if it contains a '.' then it's a good chance of a filetype and we can update our best guess + if (latestGuess.contains(".")) { + guesses.bestGuess = latestGuess + } + + if (pathSegments.size < 2) return TriedAllOptions + + return NotGoodEnough + } + + private fun guessFilename(url: String, contentDisposition: String?, mimeType: String?): String { + val tidiedUrl = url.removeSuffix("/") + var guessedFilename = URLUtil.guessFileName(tidiedUrl, contentDisposition, mimeType) + + // we only want to keep the default .bin filetype on the guess if the URL actually has that too + if (guessedFilename.endsWith(DEFAULT_FILE_TYPE) && !tidiedUrl.endsWith(DEFAULT_FILE_TYPE)) { + guessedFilename = guessedFilename.removeSuffix(DEFAULT_FILE_TYPE) + } + + Timber.v("From URL [$url], guessed [$guessedFilename]") + return guessedFilename + } + + private fun bestGuess(guesses: Guesses): String { + val guess = guesses.bestGuess ?: guesses.latestGuess + if (!guess.contains(".")) { + return guess + DEFAULT_FILE_TYPE + } + return guess + } + + private fun pathSegments(url: String): List { + return url.toUri().pathSegments ?: emptyList() + } + + private fun List.rebuildUrl(): String { + return joinToString(separator = "/") + } + + companion object { + private const val DEFAULT_FILE_TYPE = ".bin" + } + + sealed class GuessQuality { + object NotGoodEnough : GuessQuality() + object TriedAllOptions : GuessQuality() + } + + data class Guesses(var latestGuess: String, var bestGuess: String? = null) + +} diff --git a/app/src/main/java/com/duckduckgo/app/browser/downloader/NetworkFileDownloader.kt b/app/src/main/java/com/duckduckgo/app/browser/downloader/NetworkFileDownloader.kt index 23ec543764e8..cbb4ef2104ee 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/downloader/NetworkFileDownloader.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/downloader/NetworkFileDownloader.kt @@ -25,7 +25,7 @@ import com.duckduckgo.app.browser.R import com.duckduckgo.app.browser.downloader.FileDownloader.PendingFileDownload import javax.inject.Inject -class NetworkFileDownloader @Inject constructor(private val context: Context) { +class NetworkFileDownloader @Inject constructor(private val context: Context, private val filenameExtractor: FilenameExtractor) { fun download(pendingDownload: PendingFileDownload, callback: FileDownloader.FileDownloadListener) { @@ -34,7 +34,7 @@ class NetworkFileDownloader @Inject constructor(private val context: Context) { return } - val guessedFileName = pendingDownload.guessFileName() + val guessedFileName = filenameExtractor.extract(pendingDownload) val request = DownloadManager.Request(pendingDownload.url.toUri()).apply { allowScanningByMediaScanner()