Skip to content

Commit

Permalink
Closes mozilla-mobile#5091: Fix clashing file providers in feature-pr…
Browse files Browse the repository at this point in the history
…ompts and

feature-downloads.
  • Loading branch information
Amejia481 committed Nov 19, 2019
1 parent a0f8d4f commit 3ca9cc6
Show file tree
Hide file tree
Showing 14 changed files with 77 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@ data class DownloadState(
val contentLength: Long? = null,
val userAgent: String? = null,
val destinationDirectory: String = Environment.DIRECTORY_DOWNLOADS,
val filePath: String =
Environment.getExternalStoragePublicDirectory(destinationDirectory).path + "/" + fileName,
val referrerUrl: String? = null,
val skipConfirmation: Boolean = false,
val id: Long = Random.nextLong()
)
) {
val filePath: String get() =
Environment.getExternalStoragePublicDirectory(destinationDirectory).path + "/" + fileName
}
11 changes: 11 additions & 0 deletions components/feature/downloads/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,15 @@
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />
<uses-permission android:name="android.permission.FOREGROUND_SERVICE" />
<uses-permission android:name="android.permission.DOWNLOAD_WITHOUT_NOTIFICATION" />
<application>
<provider
android:name="mozilla.components.feature.downloads.provider.FileProvider"
android:authorities="${applicationId}.feature.downloads.fileprovider"
android:exported="false"
android:grantUriPermissions="true">
<meta-data
android:name="android.support.FILE_PROVIDER_PATHS"
android:resource="@xml/feature_downloads_file_paths" />
</provider>
</application>
</manifest>
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ abstract class AbstractFetchDownloadService : Service() {
}
}

private const val FILE_PROVIDER_EXTENSION = ".fileprovider"
private const val FILE_PROVIDER_EXTENSION = ".feature.downloads.fileprovider"
private const val CHUNK_SIZE = 4 * 1024
private const val PARTIAL_CONTENT_STATUS = 206
private const val OK_STATUS = 200
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
package mozilla.components.feature.downloads.provider

/**
* A file provider to provide functionality for the feature downloads component.
*
* We need this class to create a fully qualified class name that doesn't clash with other
* file providers in other components see https://stackoverflow.com/a/43444164/5533820.
*
* Be aware, when creating new file resources avoid using common names like "@xml/file_paths",
* as other file providers could be using the same names and this could case unexpected behaviors.
* As a convention try to use unique names like using the name of the component as a prefix of the
* name of the file, like component_xxx_file_paths.xml.
*/
/** @suppress */
class FileProvider : androidx.core.content.FileProvider()
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- This Source Code Form is subject to the terms of the Mozilla Public
- License, v. 2.0. If a copy of the MPL was not distributed with this file,
- You can obtain one at http://mozilla.org/MPL/2.0/. -->
<paths>
<external-path name="Download" path="." />
</paths>
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ class AbstractFetchDownloadServiceTest {
fun `verifyDownload sets the download to failed if it is not complete`() = runBlocking {
val downloadState = DownloadState(
url = "mozilla.org/mozilla.txt",
filePath = "mozilla.txt",
contentLength = 50L
)

Expand All @@ -130,7 +129,6 @@ class AbstractFetchDownloadServiceTest {
fun `verifyDownload does NOT set the download to failed if it is paused`() = runBlocking {
val downloadState = DownloadState(
url = "mozilla.org/mozilla.txt",
filePath = "mozilla.txt",
contentLength = 50L
)

Expand All @@ -152,7 +150,6 @@ class AbstractFetchDownloadServiceTest {
fun `verifyDownload does NOT set the download to failed if it is complete`() = runBlocking {
val downloadState = DownloadState(
url = "mozilla.org/mozilla.txt",
filePath = "mozilla.txt",
contentLength = 50L
)

Expand All @@ -174,7 +171,6 @@ class AbstractFetchDownloadServiceTest {
fun `verifyDownload does NOT set the download to failed if it is cancelled`() = runBlocking {
val downloadState = DownloadState(
url = "mozilla.org/mozilla.txt",
filePath = "mozilla.txt",
contentLength = 50L
)

Expand All @@ -196,7 +192,6 @@ class AbstractFetchDownloadServiceTest {
fun `verifyDownload does NOT set the download to failed if it is status COMPLETED`() = runBlocking {
val downloadState = DownloadState(
url = "mozilla.org/mozilla.txt",
filePath = "mozilla.txt",
contentLength = 50L
)

Expand Down
6 changes: 3 additions & 3 deletions components/feature/prompts/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
package="mozilla.components.feature.prompts">
<application>
<provider
android:authorities="${applicationId}.fileprovider"
android:name="androidx.core.content.FileProvider"
android:name="mozilla.components.feature.prompts.provider.FileProvider"
android:authorities="${applicationId}.feature.prompts.fileprovider"
android:exported="false"
android:grantUriPermissions="true">
<meta-data
android:name="android.support.FILE_PROVIDER_PATHS"
android:resource="@xml/file_paths" />
android:resource="@xml/feature_prompts_file_paths" />
</provider>
</application>
</manifest>
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,13 @@ internal sealed class MimeType(
val intent = Intent(ACTION_IMAGE_CAPTURE).withDeviceSupport(context) ?: return null

val photoFile = try {
val filename = SimpleDateFormat("yyyy-MM-dd HH.mm.ss", US).format(Date())
val filename = SimpleDateFormat("yyyy-MM-ddHH.mm.ss", US).format(Date())
createTempFile(filename, ".jpg", context.cacheDir)
} catch (e: IOException) {
return null
}

val photoUri = getUri(context, "${context.packageName}.fileprovider", photoFile)
val photoUri = getUri(context, "${context.packageName}.feature.prompts.fileprovider", photoFile)

return intent.apply { putExtra(EXTRA_OUTPUT, photoUri) }.addCaptureHint(request.captureMode)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
package mozilla.components.feature.prompts.provider

/**
* A file provider to provide functionality for the feature prompts component.
*
* We need this class to create a fully qualified class name that doesn't clash with other
* file providers in other components see https://stackoverflow.com/a/43444164/5533820.
*
* Be aware, when creating new file resources avoid using common names like "@xml/file_paths",
* as other file providers could be using the same names and this could case unexpected behaviors.
* As a convention try to use unique names like using the name of the component as a prefix of the
* name of the file, like component_xxx_file_paths.xml.
*/
/** @suppress */
class FileProvider : androidx.core.content.FileProvider()
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- This Source Code Form is subject to the terms of the Mozilla Public
- License, v. 2.0. If a copy of the MPL was not distributed with this file,
- You can obtain one at http://mozilla.org/MPL/2.0/. -->
<paths>
<cache-path name="feature-prompts-images" path="." />
</paths>
4 changes: 0 additions & 4 deletions components/feature/prompts/src/main/res/xml/file_paths.xml

This file was deleted.

6 changes: 6 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ permalink: /changelog/
* ⚠️ **This is a breaking change**:
* Several `FxaAccountManager` methods have been made internal, and are no longer part of the public API of this module: `createSyncManager`, `getAccountStorage`.
* **browser-state**
* ⚠️ **This is a breaking change**: `DownloadState` doesn't include the property `filePath` in its constructor anymore, now it is a computed property. As the previous behavior caused some situations where `fileName` was initially null and after assigned a value to produce `filePath` values like "/storage/emulated/0/Download/null" [for more info](https://sentry.prod.mozaws.net/operations/reference-browser/issues/6609701/).

* **feature-prompts** and **feature-downloads**
* Fix [issue #6439](https://github.com/mozilla-mobile/fenix/issues/6439) "Crash when downloading Image"

# 21.0.0

* [Commits](https://github.com/mozilla-mobile/android-components/compare/v20.0.0...v21.0.0)
Expand Down
10 changes: 0 additions & 10 deletions samples/browser/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -105,16 +105,6 @@
</service>

<service android:name=".downloads.DownloadService" />
<provider
android:name="androidx.core.content.FileProvider"
android:authorities="${applicationId}.fileprovider"
android:exported="false"
android:grantUriPermissions="true">
<meta-data
android:name="android.support.FILE_PROVIDER_PATHS"
android:resource="@xml/file_paths" />

</provider>

</application>

Expand Down
5 changes: 0 additions & 5 deletions samples/browser/src/main/res/xml/file_paths.xml

This file was deleted.

0 comments on commit 3ca9cc6

Please sign in to comment.