Closes issue mozilla-mobile#7983: Generate a file name when the conte…

…nt provider doesn't provide one.
Amejia481 · Aug 12, 2020 · bf62df7 · bf62df7
1 parent 0aff44e
commit bf62df7
Show file tree

Hide file tree

Showing 6 changed files with 188 additions and 42 deletions.
diff --git a/...-beta/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt b/...-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/...ghtly/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt b/...ghtly/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/...gecko/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt b/...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/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
@@ -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<ContentResolver>()
+        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<ContentResolver>()
+        val uri = "content://media/external/file/37162".toUri()
+        val cursor = mock<Cursor>()
+
+        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<ContentResolver>()
+        val uri = "content://media/external/file/37162".toUri()
+        val cursor = mock<Cursor>()
+
+        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<ContentResolver>()
+        val uri = "content://media/external/file/37162".toUri()
+        val cursor = mock<Cursor>()
+
+        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<ContentResolver>()
+        val uri = "file:///home/user/myfile.html".toUri()
+
+        assertEquals("myfile.html", uri.getFileName(resolver))
+    }
+
+    @Test
+    fun `getFileName for content uri schemes`() {
+        val resolver = mock<ContentResolver>()
+        val uri = "content://media/external/file/37162".toUri()
+        val cursor = mock<Cursor>()
+
+        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<ContentResolver>()
+        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
@@ -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