Skip to content

Fix crash that could happen when trying to download a large data URI image #898

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 10, 2020
Merged

Fix crash that could happen when trying to download a large data URI image #898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2098308
Fix crash on data uri downloaded callback
CDRussell Aug 7, 2020
313479c
Limit length of dialog title to prevent large data URI causing ANRs
CDRussell Aug 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import com.duckduckgo.app.browser.BrowserTabViewModel.Command.Navigate
import com.duckduckgo.app.browser.LongPressHandler.RequiredAction.DownloadFile
import com.duckduckgo.app.browser.LongPressHandler.RequiredAction.OpenInNewTab
import com.duckduckgo.app.browser.addtohome.AddToHomeCapabilityDetector
import com.duckduckgo.app.browser.downloader.FileDownloader
import com.duckduckgo.app.browser.favicon.FaviconDownloader
import com.duckduckgo.app.browser.logindetection.LoginDetected
import com.duckduckgo.app.browser.logindetection.NavigationAwareLoginDetector
Expand All @@ -52,27 +53,22 @@ import com.duckduckgo.app.browser.session.WebViewSessionStorage
import com.duckduckgo.app.cta.db.DismissedCtaDao
import com.duckduckgo.app.cta.model.CtaId
import com.duckduckgo.app.cta.model.DismissedCta
import com.duckduckgo.app.cta.ui.*
import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteDao
import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteEntity
import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteRepository
import com.duckduckgo.app.cta.ui.Cta
import com.duckduckgo.app.cta.ui.CtaViewModel
import com.duckduckgo.app.cta.ui.DaxBubbleCta
import com.duckduckgo.app.cta.ui.DaxDialogCta
import com.duckduckgo.app.cta.ui.HomePanelCta
import com.duckduckgo.app.cta.ui.UseOurAppCta
import com.duckduckgo.app.global.useourapp.UseOurAppDetector.Companion.USE_OUR_APP_DOMAIN
import com.duckduckgo.app.global.useourapp.UseOurAppDetector.Companion.USE_OUR_APP_SHORTCUT_URL
import com.duckduckgo.app.global.db.AppDatabase
import com.duckduckgo.app.global.events.db.UserEventEntity
import com.duckduckgo.app.global.events.db.UserEventKey
import com.duckduckgo.app.global.events.db.UserEventsStore
import com.duckduckgo.app.global.install.AppInstallStore
import com.duckduckgo.app.global.model.Site
import com.duckduckgo.app.global.model.SiteFactory
import com.duckduckgo.app.global.events.db.UserEventKey
import com.duckduckgo.app.notification.model.UseOurAppNotification
import com.duckduckgo.app.global.events.db.UserEventEntity
import com.duckduckgo.app.global.events.db.UserEventsStore
import com.duckduckgo.app.global.useourapp.UseOurAppDetector
import com.duckduckgo.app.global.useourapp.UseOurAppDetector.Companion.USE_OUR_APP_DOMAIN
import com.duckduckgo.app.global.useourapp.UseOurAppDetector.Companion.USE_OUR_APP_SHORTCUT_URL
import com.duckduckgo.app.notification.db.NotificationDao
import com.duckduckgo.app.notification.model.UseOurAppNotification
import com.duckduckgo.app.onboarding.store.AppStage
import com.duckduckgo.app.onboarding.store.OnboardingStore
import com.duckduckgo.app.onboarding.store.UserStageStore
Expand Down Expand Up @@ -209,6 +205,9 @@ class BrowserTabViewModelTest {
@Mock
private lateinit var mockNotificationDao: NotificationDao

@Mock
private lateinit var mockFileDownloader: FileDownloader

private lateinit var mockAutoCompleteApi: AutoCompleteApi

private lateinit var ctaViewModel: CtaViewModel
Expand Down Expand Up @@ -289,7 +288,8 @@ class BrowserTabViewModelTest {
userEventsStore = mockUserEventsStore,
notificationDao = mockNotificationDao,
useOurAppDetector = UseOurAppDetector(mockUserEventsStore),
variantManager = mockVariantManager
variantManager = mockVariantManager,
fileDownloader = mockFileDownloader
)

testee.loadData("abc", null, false)
Expand Down
181 changes: 98 additions & 83 deletions app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,18 @@ import android.webkit.WebView.HitTestResult
import android.webkit.WebView.HitTestResult.*
import android.widget.EditText
import android.widget.TextView
import android.widget.Toast
import androidx.annotation.AnyThread
import androidx.annotation.StringRes
import androidx.appcompat.app.AlertDialog
import androidx.core.content.ContextCompat
import androidx.core.content.FileProvider
import androidx.core.net.toUri
import androidx.core.text.HtmlCompat
import androidx.core.text.HtmlCompat.FROM_HTML_MODE_LEGACY
import androidx.core.view.*
import androidx.fragment.app.Fragment
import androidx.fragment.app.commitNow
import androidx.fragment.app.transaction
import androidx.lifecycle.*
import androidx.recyclerview.widget.LinearLayoutManager
Expand All @@ -56,6 +60,8 @@ import com.duckduckgo.app.bookmarks.ui.EditBookmarkDialogFragment
import com.duckduckgo.app.brokensite.BrokenSiteActivity
import com.duckduckgo.app.brokensite.BrokenSiteData
import com.duckduckgo.app.browser.BrowserTabViewModel.*
import com.duckduckgo.app.browser.BrowserTabViewModel.Command.DownloadCommand
import com.duckduckgo.app.browser.DownloadConfirmationFragment.DownloadConfirmationDialogListener
import com.duckduckgo.app.browser.autocomplete.BrowserAutoCompleteSuggestionsAdapter
import com.duckduckgo.app.browser.downloader.DownloadFailReason
import com.duckduckgo.app.browser.downloader.FileDownloadNotificationManager
Expand Down Expand Up @@ -107,10 +113,9 @@ import org.jetbrains.anko.share
import timber.log.Timber
import java.io.File
import javax.inject.Inject
import kotlin.concurrent.thread
import kotlin.coroutines.CoroutineContext

class BrowserTabFragment : Fragment(), FindListener, CoroutineScope, DaxDialogListener, TrackersAnimatorListener {
class BrowserTabFragment : Fragment(), FindListener, CoroutineScope, DaxDialogListener, TrackersAnimatorListener, DownloadConfirmationDialogListener {

private val supervisorJob = SupervisorJob()

Expand Down Expand Up @@ -249,14 +254,6 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope, DaxDialogLi
removeDaxDialogFromActivity()
renderer = BrowserTabFragmentRenderer()
decorator = BrowserTabFragmentDecorator()
if (savedInstanceState != null) {
updateFragmentListener()
}
}

private fun updateFragmentListener() {
val fragment = fragmentManager?.findFragmentByTag(DOWNLOAD_CONFIRMATION_TAG) as? DownloadConfirmationFragment
fragment?.downloadListener = createDownloadListener()
}

override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? {
Expand Down Expand Up @@ -556,6 +553,35 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope, DaxDialogLi
is Command.ShowWebContent -> webView?.show()
is Command.RefreshUserAgent -> refreshUserAgent(it.host, it.isDesktop)
is Command.AskToFireproofWebsite -> askToFireproofWebsite(requireContext(), it.fireproofWebsite)
is DownloadCommand -> processDownloadCommand(it)
}
}

private fun processDownloadCommand(it: DownloadCommand) {
when (it) {
is DownloadCommand.ScanMediaFiles -> {
context?.applicationContext?.let { context ->
MediaScannerConnection.scanFile(context, arrayOf(it.file.absolutePath), null, null)
}
}
is DownloadCommand.ShowDownloadFinishedNotification -> {
fileDownloadNotificationManager.showDownloadFinishedNotification(it.file.name, it.file.absolutePath.toUri(), it.mimeType)
}
DownloadCommand.ShowDownloadInProgressNotification -> {
fileDownloadNotificationManager.showDownloadInProgressNotification()
}
is DownloadCommand.ShowDownloadFailedNotification -> {
fileDownloadNotificationManager.showDownloadFailedNotification()

val snackbar = Snackbar.make(toolbar, R.string.downloadFailed, Snackbar.LENGTH_INDEFINITE)
if (it.reason == DownloadFailReason.DownloadManagerDisabled) {
snackbar.setText(it.message)
snackbar.setAction(getString(R.string.enable)) {
showDownloadManagerAppSettings()
}
}
snackbar.show()
}
}
}

Expand Down Expand Up @@ -1092,90 +1118,28 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope, DaxDialogLi

@AnyThread
private fun downloadFile(requestUserConfirmation: Boolean) {
val pendingDownload = pendingFileDownload
pendingFileDownload = null
val pendingDownload = pendingFileDownload ?: return

if (pendingDownload == null) {
return
}
pendingFileDownload = null

val downloadListener = createDownloadListener()
if (requestUserConfirmation) {
requestDownloadConfirmation(pendingDownload, downloadListener)
requestDownloadConfirmation(pendingDownload)
} else {
completeDownload(pendingDownload, downloadListener)
continueDownload(pendingDownload)
}
}

private fun closeAndReturnToSourceIfBlankTab() {
if (viewModel.url == null) {
launch {
viewModel.closeAndSelectSourceTab()
}
}
}

private fun createDownloadListener(): FileDownloadListener {
return object : FileDownloadListener {
override fun downloadStarted() {
fileDownloadNotificationManager.showDownloadInProgressNotification()
closeAndReturnToSourceIfBlankTab()
}

override fun downloadFinished(file: File, mimeType: String?) {
MediaScannerConnection.scanFile(context, arrayOf(file.absolutePath), null) { _, uri ->
fileDownloadNotificationManager.showDownloadFinishedNotification(file.name, uri, mimeType)
}
}

override fun downloadFailed(message: String, downloadFailReason: DownloadFailReason) {
Timber.w("Failed to download file [$message]")

fileDownloadNotificationManager.showDownloadFailedNotification()

val snackbar = Snackbar.make(toolbar, R.string.downloadFailed, Snackbar.LENGTH_INDEFINITE)
if (downloadFailReason == DownloadFailReason.DownloadManagerDisabled) {
snackbar.setText(message)
snackbar.setAction(getString(R.string.enable)) {
showDownloadManagerAppSettings()
}
}
snackbar.show()
}

override fun downloadCancelled() {
closeAndReturnToSourceIfBlankTab()
}

override fun downloadOpened() {
closeAndReturnToSourceIfBlankTab()
}

private fun showDownloadManagerAppSettings() {
try {
val intent = Intent(Settings.ACTION_APPLICATION_DETAILS_SETTINGS)
intent.data = DownloadFailReason.DOWNLOAD_MANAGER_SETTINGS_URI
startActivity(intent)
} catch (e: ActivityNotFoundException) {
Timber.w(e, "Could not open DownloadManager settings")
Snackbar.make(toolbar, R.string.downloadManagerIncompatible, Snackbar.LENGTH_INDEFINITE).show()
}
}
}
}

private fun requestDownloadConfirmation(pendingDownload: PendingFileDownload, downloadListener: FileDownloadListener) {
fragmentManager?.let {
if (!it.isStateSaved) {
DownloadConfirmationFragment.instance(pendingDownload, downloadListener).show(it, DOWNLOAD_CONFIRMATION_TAG)
}
private fun requestDownloadConfirmation(pendingDownload: PendingFileDownload) {
val downloadConfirmationFragment = DownloadConfirmationFragment.instance(pendingDownload)
childFragmentManager.findFragmentByTag(DOWNLOAD_CONFIRMATION_TAG)?.let {
Timber.i("Found existing dialog; removing it now")
childFragmentManager.commitNow { remove(it) }
}
downloadConfirmationFragment.show(childFragmentManager, DOWNLOAD_CONFIRMATION_TAG)
}

private fun completeDownload(pendingDownload: PendingFileDownload, callback: FileDownloadListener) {
thread {
fileDownloader.download(pendingDownload, callback)
}
viewModel.download(pendingDownload)
}

private fun launchFilePicker(command: Command.ShowFileChooser) {
Expand Down Expand Up @@ -1787,4 +1751,55 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope, DaxDialogLi
private fun shouldUpdateOmnibarTextInput(viewState: OmnibarViewState, omnibarInput: String?) =
(!viewState.isEditing || omnibarInput.isNullOrEmpty()) && omnibarTextInput.isDifferent(omnibarInput)
}

override fun openExistingFile(file: File?) {
if (file == null) {
Toast.makeText(activity, R.string.downloadConfirmationUnableToOpenFileText, Toast.LENGTH_SHORT).show()
return
}

val intent = context?.let { createIntentToOpenFile(it, file) }
activity?.packageManager?.let { packageManager ->
if (intent?.resolveActivity(packageManager) != null) {
startActivity(intent)
} else {
Timber.e("No suitable activity found")
Toast.makeText(activity, R.string.downloadConfirmationUnableToOpenFileText, Toast.LENGTH_SHORT).show()
}
}
}

override fun replaceExistingFile(file: File?, pendingFileDownload: PendingFileDownload) {
Timber.i("Deleting existing file: $file")
runCatching { file?.delete() }
continueDownload(pendingFileDownload)
}

private fun showDownloadManagerAppSettings() {
try {
val intent = Intent(Settings.ACTION_APPLICATION_DETAILS_SETTINGS)
intent.data = DownloadFailReason.DOWNLOAD_MANAGER_SETTINGS_URI
startActivity(intent)
} catch (e: ActivityNotFoundException) {
Timber.w(e, "Could not open DownloadManager settings")
Snackbar.make(toolbar, R.string.downloadManagerIncompatible, Snackbar.LENGTH_INDEFINITE).show()
}
}

private fun createIntentToOpenFile(context: Context, file: File): Intent? {
val uri = FileProvider.getUriForFile(context, "${BuildConfig.APPLICATION_ID}.provider", file)
val mime = activity?.contentResolver?.getType(uri) ?: return null
val intent = Intent(Intent.ACTION_VIEW)
intent.setDataAndType(uri, mime)
return intent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION)
}

override fun continueDownload(pendingFileDownload: PendingFileDownload) {
Timber.i("Continuing to download $pendingFileDownload")
viewModel.download(pendingFileDownload)
}

override fun cancelDownload() {
viewModel.closeAndReturnToSourceIfBlankTab()
}
}
Loading