From b9e694a991fc74e83a21f7078a9b152001e7e679 Mon Sep 17 00:00:00 2001 From: Arturo Mejia Date: Wed, 12 Aug 2020 15:51:53 -0400 Subject: [PATCH] Closes issue #7983: Generate a file name when the content provider doesn't provide one. --- .../gecko/prompt/GeckoPromptDelegate.kt | 15 +-- .../gecko/prompt/GeckoPromptDelegateTest.kt | 16 ++- .../gecko/prompt/GeckoPromptDelegate.kt | 15 +-- .../gecko/prompt/GeckoPromptDelegateTest.kt | 16 ++- .../gecko/prompt/GeckoPromptDelegate.kt | 15 +-- .../gecko/prompt/GeckoPromptDelegateTest.kt | 15 ++- .../components/support/ktx/android/net/Uri.kt | 57 ++++++++ .../support/ktx/android/net/UriTest.kt | 126 ++++++++++++++++++ docs/changelog.md | 2 + 9 files changed, 214 insertions(+), 63 deletions(-) diff --git a/components/browser/engine-gecko-beta/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt b/components/browser/engine-gecko-beta/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt index ae63b646889..bc7373725fc 100644 --- a/components/browser/engine-gecko-beta/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt +++ b/components/browser/engine-gecko-beta/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt @@ -4,10 +4,8 @@ package mozilla.components.browser.engine.gecko.prompt -import android.content.ContentResolver import android.content.Context import android.net.Uri -import android.provider.OpenableColumns import androidx.annotation.VisibleForTesting import mozilla.components.browser.engine.gecko.GeckoEngineSession import mozilla.components.concept.storage.Login @@ -18,7 +16,7 @@ import mozilla.components.concept.engine.prompt.PromptRequest.MultipleChoice import mozilla.components.concept.engine.prompt.PromptRequest.SingleChoice import mozilla.components.concept.engine.prompt.ShareData import mozilla.components.support.base.log.logger.Logger -import mozilla.components.support.ktx.kotlin.sanitizeFileName +import mozilla.components.support.ktx.android.net.getFileName import mozilla.components.support.ktx.kotlin.toDate import org.mozilla.geckoview.AllowOrDeny import org.mozilla.geckoview.GeckoResult @@ -550,17 +548,6 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe } return Uri.parse("file:///${temporalFile.absolutePath}") } - - private fun Uri.getFileName(contentResolver: ContentResolver): String { - val returnUri = this - var fileName = "" - contentResolver.query(returnUri, null, null, null, null)?.use { cursor -> - val nameIndex = cursor.getColumnIndex(OpenableColumns.DISPLAY_NAME) - cursor.moveToFirst() - fileName = cursor.getString(nameIndex) - } - return fileName.sanitizeFileName() - } } @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) diff --git a/components/browser/engine-gecko-beta/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt b/components/browser/engine-gecko-beta/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt index fef20687d16..416104306d6 100644 --- a/components/browser/engine-gecko-beta/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt +++ b/components/browser/engine-gecko-beta/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt @@ -24,6 +24,9 @@ import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import org.mockito.ArgumentMatchers.any +import org.mockito.Mockito.spy +import org.mockito.Mockito.doReturn import org.mozilla.gecko.util.GeckoBundle import org.mozilla.geckoview.GeckoRuntime import org.mozilla.geckoview.GeckoSession @@ -35,8 +38,7 @@ import org.mozilla.geckoview.GeckoSession.PromptDelegate.DateTimePrompt.Type.WEE import org.mozilla.geckoview.GeckoSession.PromptDelegate.FilePrompt.Capture.ANY import org.mozilla.geckoview.GeckoSession.PromptDelegate.FilePrompt.Capture.NONE import org.mozilla.geckoview.GeckoSession.PromptDelegate.FilePrompt.Capture.USER -import org.robolectric.Shadows.shadowOf -import java.io.FileInputStream +import java.io.InputStream import java.security.InvalidParameterException import java.util.Calendar import java.util.Calendar.YEAR @@ -549,17 +551,17 @@ class GeckoPromptDelegateTest { @Test fun `Calling onFilePrompt must provide a FilePicker PromptRequest`() { - val context = testContext - + val context = spy(testContext) + val contentResolver = spy(context.contentResolver) val mockSession = GeckoEngineSession(runtime) var onSingleFileSelectedWasCalled = false var onMultipleFilesSelectedWasCalled = false var onDismissWasCalled = false val mockUri: Uri = mock() - val mockFileInput: FileInputStream = mock() - val shadowContentResolver = shadowOf(context.contentResolver) - shadowContentResolver.registerInputStream(mockUri, mockFileInput) + doReturn(contentResolver).`when`(context).contentResolver + doReturn(mock()).`when`(contentResolver).openInputStream(any()) + var filePickerRequest: PromptRequest.File = mock() val promptDelegate = GeckoPromptDelegate(mockSession) diff --git a/components/browser/engine-gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt b/components/browser/engine-gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt index ae63b646889..bc7373725fc 100644 --- a/components/browser/engine-gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt +++ b/components/browser/engine-gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt @@ -4,10 +4,8 @@ package mozilla.components.browser.engine.gecko.prompt -import android.content.ContentResolver import android.content.Context import android.net.Uri -import android.provider.OpenableColumns import androidx.annotation.VisibleForTesting import mozilla.components.browser.engine.gecko.GeckoEngineSession import mozilla.components.concept.storage.Login @@ -18,7 +16,7 @@ import mozilla.components.concept.engine.prompt.PromptRequest.MultipleChoice import mozilla.components.concept.engine.prompt.PromptRequest.SingleChoice import mozilla.components.concept.engine.prompt.ShareData import mozilla.components.support.base.log.logger.Logger -import mozilla.components.support.ktx.kotlin.sanitizeFileName +import mozilla.components.support.ktx.android.net.getFileName import mozilla.components.support.ktx.kotlin.toDate import org.mozilla.geckoview.AllowOrDeny import org.mozilla.geckoview.GeckoResult @@ -550,17 +548,6 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe } return Uri.parse("file:///${temporalFile.absolutePath}") } - - private fun Uri.getFileName(contentResolver: ContentResolver): String { - val returnUri = this - var fileName = "" - contentResolver.query(returnUri, null, null, null, null)?.use { cursor -> - val nameIndex = cursor.getColumnIndex(OpenableColumns.DISPLAY_NAME) - cursor.moveToFirst() - fileName = cursor.getString(nameIndex) - } - return fileName.sanitizeFileName() - } } @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) diff --git a/components/browser/engine-gecko-nightly/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt b/components/browser/engine-gecko-nightly/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt index fef20687d16..416104306d6 100644 --- a/components/browser/engine-gecko-nightly/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt +++ b/components/browser/engine-gecko-nightly/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt @@ -24,6 +24,9 @@ import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import org.mockito.ArgumentMatchers.any +import org.mockito.Mockito.spy +import org.mockito.Mockito.doReturn import org.mozilla.gecko.util.GeckoBundle import org.mozilla.geckoview.GeckoRuntime import org.mozilla.geckoview.GeckoSession @@ -35,8 +38,7 @@ import org.mozilla.geckoview.GeckoSession.PromptDelegate.DateTimePrompt.Type.WEE import org.mozilla.geckoview.GeckoSession.PromptDelegate.FilePrompt.Capture.ANY import org.mozilla.geckoview.GeckoSession.PromptDelegate.FilePrompt.Capture.NONE import org.mozilla.geckoview.GeckoSession.PromptDelegate.FilePrompt.Capture.USER -import org.robolectric.Shadows.shadowOf -import java.io.FileInputStream +import java.io.InputStream import java.security.InvalidParameterException import java.util.Calendar import java.util.Calendar.YEAR @@ -549,17 +551,17 @@ class GeckoPromptDelegateTest { @Test fun `Calling onFilePrompt must provide a FilePicker PromptRequest`() { - val context = testContext - + val context = spy(testContext) + val contentResolver = spy(context.contentResolver) val mockSession = GeckoEngineSession(runtime) var onSingleFileSelectedWasCalled = false var onMultipleFilesSelectedWasCalled = false var onDismissWasCalled = false val mockUri: Uri = mock() - val mockFileInput: FileInputStream = mock() - val shadowContentResolver = shadowOf(context.contentResolver) - shadowContentResolver.registerInputStream(mockUri, mockFileInput) + doReturn(contentResolver).`when`(context).contentResolver + doReturn(mock()).`when`(contentResolver).openInputStream(any()) + var filePickerRequest: PromptRequest.File = mock() val promptDelegate = GeckoPromptDelegate(mockSession) diff --git a/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt b/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt index ae63b646889..bc7373725fc 100644 --- a/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt +++ b/components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt @@ -4,10 +4,8 @@ package mozilla.components.browser.engine.gecko.prompt -import android.content.ContentResolver import android.content.Context import android.net.Uri -import android.provider.OpenableColumns import androidx.annotation.VisibleForTesting import mozilla.components.browser.engine.gecko.GeckoEngineSession import mozilla.components.concept.storage.Login @@ -18,7 +16,7 @@ import mozilla.components.concept.engine.prompt.PromptRequest.MultipleChoice import mozilla.components.concept.engine.prompt.PromptRequest.SingleChoice import mozilla.components.concept.engine.prompt.ShareData import mozilla.components.support.base.log.logger.Logger -import mozilla.components.support.ktx.kotlin.sanitizeFileName +import mozilla.components.support.ktx.android.net.getFileName import mozilla.components.support.ktx.kotlin.toDate import org.mozilla.geckoview.AllowOrDeny import org.mozilla.geckoview.GeckoResult @@ -550,17 +548,6 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe } return Uri.parse("file:///${temporalFile.absolutePath}") } - - private fun Uri.getFileName(contentResolver: ContentResolver): String { - val returnUri = this - var fileName = "" - contentResolver.query(returnUri, null, null, null, null)?.use { cursor -> - val nameIndex = cursor.getColumnIndex(OpenableColumns.DISPLAY_NAME) - cursor.moveToFirst() - fileName = cursor.getString(nameIndex) - } - return fileName.sanitizeFileName() - } } @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) diff --git a/components/browser/engine-gecko/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt b/components/browser/engine-gecko/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt index fef20687d16..bee9a45066a 100644 --- a/components/browser/engine-gecko/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt +++ b/components/browser/engine-gecko/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt @@ -24,6 +24,9 @@ import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import org.mockito.ArgumentMatchers.any +import org.mockito.Mockito.spy +import org.mockito.Mockito.doReturn import org.mozilla.gecko.util.GeckoBundle import org.mozilla.geckoview.GeckoRuntime import org.mozilla.geckoview.GeckoSession @@ -35,8 +38,7 @@ import org.mozilla.geckoview.GeckoSession.PromptDelegate.DateTimePrompt.Type.WEE import org.mozilla.geckoview.GeckoSession.PromptDelegate.FilePrompt.Capture.ANY import org.mozilla.geckoview.GeckoSession.PromptDelegate.FilePrompt.Capture.NONE import org.mozilla.geckoview.GeckoSession.PromptDelegate.FilePrompt.Capture.USER -import org.robolectric.Shadows.shadowOf -import java.io.FileInputStream +import java.io.InputStream import java.security.InvalidParameterException import java.util.Calendar import java.util.Calendar.YEAR @@ -549,17 +551,16 @@ class GeckoPromptDelegateTest { @Test fun `Calling onFilePrompt must provide a FilePicker PromptRequest`() { - val context = testContext - + val context = spy(testContext) + val contentResolver = spy(context.contentResolver) val mockSession = GeckoEngineSession(runtime) var onSingleFileSelectedWasCalled = false var onMultipleFilesSelectedWasCalled = false var onDismissWasCalled = false val mockUri: Uri = mock() - val mockFileInput: FileInputStream = mock() - val shadowContentResolver = shadowOf(context.contentResolver) - shadowContentResolver.registerInputStream(mockUri, mockFileInput) + doReturn(contentResolver).`when`(context).contentResolver + doReturn(mock()).`when`(contentResolver).openInputStream(any()) var filePickerRequest: PromptRequest.File = mock() val promptDelegate = GeckoPromptDelegate(mockSession) diff --git a/components/support/ktx/src/main/java/mozilla/components/support/ktx/android/net/Uri.kt b/components/support/ktx/src/main/java/mozilla/components/support/ktx/android/net/Uri.kt index cf9659c9042..3970e62e25a 100644 --- a/components/support/ktx/src/main/java/mozilla/components/support/ktx/android/net/Uri.kt +++ b/components/support/ktx/src/main/java/mozilla/components/support/ktx/android/net/Uri.kt @@ -8,8 +8,13 @@ import android.content.ContentResolver import android.content.Context import android.net.Uri import android.os.Build +import android.provider.OpenableColumns +import android.webkit.MimeTypeMap +import androidx.annotation.VisibleForTesting +import mozilla.components.support.ktx.kotlin.sanitizeFileName import java.io.File import java.io.IOException +import java.util.UUID private val commonPrefixes = listOf("www.", "mobile.", "m.") @@ -85,3 +90,55 @@ fun Uri.isUnderPrivateAppDirectory(context: Context): Boolean { else -> false } } + +/** + * Return a file name for [this] give Uri. + * @return A file name for the content, or generated file name if the URL is invalid or the type is unknown + */ +fun Uri.getFileName(contentResolver: ContentResolver): String { + return when (this.scheme) { + ContentResolver.SCHEME_FILE -> File(path ?: "").name.sanitizeFileName() + ContentResolver.SCHEME_CONTENT -> getFileNameForContentUris(contentResolver) + else -> { + generateFileName(getFileExtension(contentResolver)) + } + } +} + +/** + * Return a file extension for [this] give Uri (only supports content:// schemes). + * @return A file extension for the content, or empty string if the URL is invalid or the type is unknown + */ +fun Uri.getFileExtension(contentResolver: ContentResolver): String { + return MimeTypeMap.getSingleton().getExtensionFromMimeType(contentResolver.getType(this)) ?: "" +} + +@VisibleForTesting +internal fun Uri.getFileNameForContentUris(contentResolver: ContentResolver): String { + var fileName = "" + contentResolver.query(this, null, null, null, null)?.use { cursor -> + val nameIndex = cursor.getColumnIndex(OpenableColumns.DISPLAY_NAME) + val fileExtension = getFileExtension(contentResolver) + fileName = if (nameIndex == -1) { + generateFileName(fileExtension) + } else { + cursor.moveToFirst() + cursor.getString(nameIndex) ?: generateFileName(fileExtension) + } + } + return fileName.sanitizeFileName() +} + +/** + * Generate a file name using a randomUUID + the current timestamp. + */ +@VisibleForTesting +internal fun generateFileName(fileExtension: String = ""): String { + val randomId = UUID.randomUUID().toString().removePrefix("-").trim() + val timeStamp = System.currentTimeMillis() + return if (fileExtension.isNotEmpty()) { + "$randomId$timeStamp.$fileExtension" + } else { + "$randomId$timeStamp" + } +} diff --git a/components/support/ktx/src/test/java/mozilla/components/support/ktx/android/net/UriTest.kt b/components/support/ktx/src/test/java/mozilla/components/support/ktx/android/net/UriTest.kt index db4b6dc442e..d10d2e762cc 100644 --- a/components/support/ktx/src/test/java/mozilla/components/support/ktx/android/net/UriTest.kt +++ b/components/support/ktx/src/test/java/mozilla/components/support/ktx/android/net/UriTest.kt @@ -4,14 +4,22 @@ package mozilla.components.support.ktx.android.net +import android.content.ContentResolver +import android.database.Cursor +import android.webkit.MimeTypeMap import androidx.core.net.toUri import androidx.test.ext.junit.runners.AndroidJUnit4 +import mozilla.components.support.test.mock import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Test import org.junit.runner.RunWith +import org.mockito.ArgumentMatchers.anyInt +import org.mockito.Mockito.any +import org.mockito.Mockito.doReturn +import org.robolectric.Shadows @RunWith(AndroidJUnit4::class) class UriTest { @@ -109,4 +117,122 @@ class UriTest { assertTrue("https://foo.bar:443/bobo".toUri().sameOriginAs("https://foo.bar:443/obob".toUri())) assertTrue("https://foo.bar:333".toUri().sameOriginAs("https://foo.bar:333".toUri())) } + + @Test + fun testGenerateFileName() { + val fileExtension = "txt" + var fileName = generateFileName(fileExtension) + + assertTrue(fileName.contains(fileExtension)) + + fileName = generateFileName() + + assertFalse(fileName.contains(".")) + } + + @Test + fun testGetFileExtension() { + val resolver = mock() + val uri = "content://media/external/file/37162".toUri() + + Shadows.shadowOf(MimeTypeMap.getSingleton()).addExtensionMimeTypMapping("txt", "text/plain") + + doReturn("text/plain").`when`(resolver).getType(any()) + + assertEquals("txt", uri.getFileExtension(resolver)) + } + + @Test + fun `getFileNameForContentUris for urls with DISPLAY_NAME`() { + val resolver = mock() + val uri = "content://media/external/file/37162".toUri() + val cursor = mock() + + Shadows.shadowOf(MimeTypeMap.getSingleton()).addExtensionMimeTypMapping("txt", "text/plain") + doReturn("text/plain").`when`(resolver).getType(any()) + + doReturn(cursor).`when`(resolver).query(any(), any(), any(), any(), any()) + doReturn(1).`when`(cursor).getColumnIndex(any()) + doReturn("myFile.txt").`when`(cursor).getString(anyInt()) + + assertEquals("myFile.txt", uri.getFileNameForContentUris(resolver)) + } + + @Test + fun `getFileNameForContentUris for urls without DISPLAY_NAME`() { + val resolver = mock() + val uri = "content://media/external/file/37162".toUri() + val cursor = mock() + + Shadows.shadowOf(MimeTypeMap.getSingleton()).addExtensionMimeTypMapping("txt", "text/plain") + doReturn("text/plain").`when`(resolver).getType(any()) + + doReturn(cursor).`when`(resolver).query(any(), any(), any(), any(), any()) + doReturn(-1).`when`(cursor).getColumnIndex(any()) + + val fileName = uri.getFileNameForContentUris(resolver) + + assertTrue(fileName.contains(".txt")) + assertTrue(fileName.isNotEmpty()) + } + + @Test + fun `getFileNameForContentUris for urls with null DISPLAY_NAME`() { + val resolver = mock() + val uri = "content://media/external/file/37162".toUri() + val cursor = mock() + + Shadows.shadowOf(MimeTypeMap.getSingleton()).addExtensionMimeTypMapping("txt", "text/plain") + doReturn("text/plain").`when`(resolver).getType(any()) + + doReturn(cursor).`when`(resolver).query(any(), any(), any(), any(), any()) + doReturn(1).`when`(cursor).getColumnIndex(any()) + doReturn(null).`when`(cursor).getString(anyInt()) + + val fileName = uri.getFileNameForContentUris(resolver) + + assertTrue(fileName.contains(".txt")) + assertTrue(fileName.isNotEmpty()) + } + + @Test + fun `getFileName for file uri schemes`() { + val resolver = mock() + val uri = "file:///home/user/myfile.html".toUri() + + assertEquals("myfile.html", uri.getFileName(resolver)) + } + + @Test + fun `getFileName for content uri schemes`() { + val resolver = mock() + val uri = "content://media/external/file/37162".toUri() + val cursor = mock() + + Shadows.shadowOf(MimeTypeMap.getSingleton()).addExtensionMimeTypMapping("txt", "text/plain") + doReturn("text/plain").`when`(resolver).getType(any()) + + doReturn(cursor).`when`(resolver).query(any(), any(), any(), any(), any()) + doReturn(1).`when`(cursor).getColumnIndex(any()) + doReturn(null).`when`(cursor).getString(anyInt()) + + val fileName = uri.getFileName(resolver) + + assertTrue(fileName.contains(".txt")) + assertTrue(fileName.isNotEmpty()) + } + + @Test + fun `getFileName for UNKNOWN uri schemes will generate file name`() { + val resolver = mock() + val uri = "UNKNOWN://media/external/file/37162".toUri() + + Shadows.shadowOf(MimeTypeMap.getSingleton()).addExtensionMimeTypMapping("txt", "text/plain") + doReturn("text/plain").`when`(resolver).getType(any()) + + val fileName = uri.getFileName(resolver) + + assertTrue(fileName.contains(".txt")) + assertTrue(fileName.isNotEmpty()) + } } diff --git a/docs/changelog.md b/docs/changelog.md index b0fe02a5519..1c46cb2410b 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -12,6 +12,8 @@ permalink: /changelog/ * [Gecko](https://github.com/mozilla-mobile/android-components/blob/master/buildSrc/src/main/java/Gecko.kt) * [Configuration](https://github.com/mozilla-mobile/android-components/blob/master/buildSrc/src/main/java/Config.kt) +* **browser-engine-gecko**, **browser-engine-gecko-beta**, **browser-engine-gecko-nightly** + * Fixed issue [#7983](https://github.com/mozilla-mobile/android-components/issues/7983), crash when a file name wasn't provided when uploading a file. # 54.0.0