Merge #8104

8104: Closes issue #7983: Generate a file name when the content provider doesn't provide one. r=pocmo a=Amejia481 When we get an `uri` from a file provider, we were trusting the provider to always provide a valid file name (`DISPLAY_NAME`) that is not always true, as we can see crashes on #7983, now when we can't find the file name, we are trying to generate a file name as it's required for the [file to be updated](https://github.com/mozilla-mobile/android-components/blob/0aff44eb2797c05d7bc28edc9cb0ebc5409371b4/components/browser/engine-gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegate.kt#L541). Co-authored-by: Arturo Mejia <arturomejiamarmol@gmail.com>
mozilla-mobile · Aug 17, 2020 · 4202c70 · 4202c70
2 parents 51a2d65 + a1cd163
commit 4202c70
Show file tree

Hide file tree

Showing 9 changed files with 247 additions and 63 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
@@ -32,8 +30,10 @@ import org.mozilla.geckoview.GeckoSession.PromptDelegate.DateTimePrompt.Type.TIM
 import org.mozilla.geckoview.GeckoSession.PromptDelegate.DateTimePrompt.Type.WEEK
 import org.mozilla.geckoview.GeckoSession.PromptDelegate.PromptResponse
 import org.mozilla.geckoview.Autocomplete
+import java.io.File
 import java.io.FileOutputStream
 import java.io.IOException
+import java.io.InputStream
 import java.security.InvalidParameterException
 import java.text.SimpleDateFormat
 import java.util.Date
@@ -541,25 +541,19 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
         val temporalFile = java.io.File(cacheUploadDirectory, getFileName(contentResolver))
         try {
             contentResolver.openInputStream(this)!!.use { inStream ->
-                FileOutputStream(temporalFile).use { outStream ->
-                    inStream.copyTo(outStream)
-                }
+                copyFile(temporalFile, inStream)
             }
         } catch (e: IOException) {
             Logger("GeckoPromptDelegate").warn("Could not convert uri to file uri", e)
         }
         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)
+    @VisibleForTesting
+    internal fun copyFile(temporalFile: File, inStream: InputStream): Long {
+        return FileOutputStream(temporalFile).use { outStream ->
+            inStream.copyTo(outStream)
         }
-        return fileName.sanitizeFileName()
     }
 }
 

diff --git a/...a/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt b/...a/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt
@@ -13,6 +13,7 @@ import mozilla.components.concept.engine.prompt.PromptRequest
 import mozilla.components.concept.engine.prompt.PromptRequest.MultipleChoice
 import mozilla.components.concept.engine.prompt.PromptRequest.SingleChoice
 import mozilla.components.support.ktx.kotlin.toDate
+import mozilla.components.support.test.any
 import mozilla.components.support.test.mock
 import mozilla.components.support.test.robolectric.testContext
 import mozilla.components.support.test.whenever
@@ -24,6 +25,8 @@ import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
+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,7 +38,6 @@ 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.security.InvalidParameterException
 import java.util.Calendar
@@ -549,20 +551,24 @@ 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<FileInputStream>()).`when`(contentResolver).openInputStream(mozilla.components.support.test.any())
+
         var filePickerRequest: PromptRequest.File = mock()
 
-        val promptDelegate = GeckoPromptDelegate(mockSession)
+        val promptDelegate = spy(GeckoPromptDelegate(mockSession))
+
+        // Prevent the file from being copied
+        doReturn(0L).`when`(promptDelegate).copyFile(any(), any())
+
         mockSession.register(object : EngineSession.Observer {
             override fun onPromptRequest(promptRequest: PromptRequest) {
                 filePickerRequest = promptRequest as PromptRequest.File

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
@@ -32,8 +30,10 @@ import org.mozilla.geckoview.GeckoSession.PromptDelegate.DateTimePrompt.Type.TIM
 import org.mozilla.geckoview.GeckoSession.PromptDelegate.DateTimePrompt.Type.WEEK
 import org.mozilla.geckoview.GeckoSession.PromptDelegate.PromptResponse
 import org.mozilla.geckoview.Autocomplete
+import java.io.File
 import java.io.FileOutputStream
 import java.io.IOException
+import java.io.InputStream
 import java.security.InvalidParameterException
 import java.text.SimpleDateFormat
 import java.util.Date
@@ -541,25 +541,19 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
         val temporalFile = java.io.File(cacheUploadDirectory, getFileName(contentResolver))
         try {
             contentResolver.openInputStream(this)!!.use { inStream ->
-                FileOutputStream(temporalFile).use { outStream ->
-                    inStream.copyTo(outStream)
-                }
+                copyFile(temporalFile, inStream)
             }
         } catch (e: IOException) {
             Logger("GeckoPromptDelegate").warn("Could not convert uri to file uri", e)
         }
         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)
+    @VisibleForTesting
+    internal fun copyFile(temporalFile: File, inStream: InputStream): Long {
+        return FileOutputStream(temporalFile).use { outStream ->
+            inStream.copyTo(outStream)
         }
-        return fileName.sanitizeFileName()
     }
 }
 

diff --git a/...y/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt b/...y/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt
@@ -13,6 +13,7 @@ import mozilla.components.concept.engine.prompt.PromptRequest
 import mozilla.components.concept.engine.prompt.PromptRequest.MultipleChoice
 import mozilla.components.concept.engine.prompt.PromptRequest.SingleChoice
 import mozilla.components.support.ktx.kotlin.toDate
+import mozilla.components.support.test.any
 import mozilla.components.support.test.mock
 import mozilla.components.support.test.robolectric.testContext
 import mozilla.components.support.test.whenever
@@ -24,6 +25,8 @@ import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
+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,7 +38,6 @@ 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.security.InvalidParameterException
 import java.util.Calendar
@@ -549,20 +551,24 @@ 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<FileInputStream>()).`when`(contentResolver).openInputStream(any())
+
         var filePickerRequest: PromptRequest.File = mock()
 
-        val promptDelegate = GeckoPromptDelegate(mockSession)
+        val promptDelegate = spy(GeckoPromptDelegate(mockSession))
+
+        // Prevent the file from being copied
+        doReturn(0L).`when`(promptDelegate).copyFile(any(), any())
+
         mockSession.register(object : EngineSession.Observer {
             override fun onPromptRequest(promptRequest: PromptRequest) {
                 filePickerRequest = promptRequest as PromptRequest.File

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
@@ -32,8 +30,10 @@ import org.mozilla.geckoview.GeckoSession.PromptDelegate.DateTimePrompt.Type.TIM
 import org.mozilla.geckoview.GeckoSession.PromptDelegate.DateTimePrompt.Type.WEEK
 import org.mozilla.geckoview.GeckoSession.PromptDelegate.PromptResponse
 import org.mozilla.geckoview.Autocomplete
+import java.io.File
 import java.io.FileOutputStream
 import java.io.IOException
+import java.io.InputStream
 import java.security.InvalidParameterException
 import java.text.SimpleDateFormat
 import java.util.Date
@@ -541,25 +541,19 @@ internal class GeckoPromptDelegate(private val geckoEngineSession: GeckoEngineSe
         val temporalFile = java.io.File(cacheUploadDirectory, getFileName(contentResolver))
         try {
             contentResolver.openInputStream(this)!!.use { inStream ->
-                FileOutputStream(temporalFile).use { outStream ->
-                    inStream.copyTo(outStream)
-                }
+                copyFile(temporalFile, inStream)
             }
         } catch (e: IOException) {
             Logger("GeckoPromptDelegate").warn("Could not convert uri to file uri", e)
         }
         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)
+    @VisibleForTesting
+    internal fun copyFile(temporalFile: File, inStream: InputStream): Long {
+        return FileOutputStream(temporalFile).use { outStream ->
+            inStream.copyTo(outStream)
         }
-        return fileName.sanitizeFileName()
     }
 }
 

diff --git a/...o/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt b/...o/src/test/java/mozilla/components/browser/engine/gecko/prompt/GeckoPromptDelegateTest.kt
@@ -13,6 +13,7 @@ import mozilla.components.concept.engine.prompt.PromptRequest
 import mozilla.components.concept.engine.prompt.PromptRequest.MultipleChoice
 import mozilla.components.concept.engine.prompt.PromptRequest.SingleChoice
 import mozilla.components.support.ktx.kotlin.toDate
+import mozilla.components.support.test.any
 import mozilla.components.support.test.mock
 import mozilla.components.support.test.robolectric.testContext
 import mozilla.components.support.test.whenever
@@ -24,6 +25,8 @@ import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
+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,7 +38,6 @@ 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.security.InvalidParameterException
 import java.util.Calendar
@@ -549,20 +551,23 @@ 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<FileInputStream>()).`when`(contentResolver).openInputStream(any())
         var filePickerRequest: PromptRequest.File = mock()
 
-        val promptDelegate = GeckoPromptDelegate(mockSession)
+        val promptDelegate = spy(GeckoPromptDelegate(mockSession))
+
+        // Prevent the file from being copied
+        doReturn(0L).`when`(promptDelegate).copyFile(any(), any())
+
         mockSession.register(object : EngineSession.Observer {
             override fun onPromptRequest(promptRequest: PromptRequest) {
                 filePickerRequest = promptRequest as PromptRequest.File