2063609 - Uri.getFileName crashes when content providers reject metadata queries or return an empty cursor

Status: RESOLVED FIXED

(Firefox for Android :: Downloads, defect, P2)

Description

[Corentin Bect [:corentin]]

When handling a result from the Android file picker, FilePicker.handleFilePickerIntentResult() calls enqueueForCleanup() so that the temporary file can be deleted later. enqueueForCleanup() attempts to get the file name by calling Uri.getFileName(), but some content providers can throw an exception at this step:

• ContentResolver.query() can throw IllegalArgumentException
• CursorWrapper.getString() can throw CursorIndexOutOfBoundsException if the cursor is empty

These exceptions happen before Firefox processes the selected file, so the upload is interrupted and Firefox crashes.

Example stacks:

• IllegalArgumentException

java.lang.IllegalArgumentException
    at android.database.DatabaseUtils.readExceptionFromParcel
    at android.content.ContentProviderProxy.query
    at android.content.ContentResolver.query
    at mozilla.components.support.ktx.android.net.UriKt.getFileName
    at mozilla.components.feature.prompts.file.FilePicker.enqueueForCleanup

• CursorIndexOutOfBoundsException

android.database.CursorIndexOutOfBoundsException
    at android.database.AbstractCursor.checkPosition
    at android.database.AbstractWindowedCursor.getString
    at android.database.CursorWrapper.getString
    at mozilla.components.support.ktx.android.net.UriKt.getFileName
    at mozilla.components.feature.prompts.file.FilePicker.enqueueForCleanup

Expected behavior

Failure to retrieve filename metadata should not crash Firefox. Such exceptions should be either caught, or a fallback file name should be generated to process so that the uploaded file is processed

Any additional information?

These exceptions were noticed when investigation this bug. These are ones of many causes of java.lang.RuntimeException: at android.app.ActivityThread.deliverResults described in this meta ticket

Comment 1

[Corentin Bect [:corentin]]

Attachment: Bug 2063609 - Prevent crash when a content provider fails to return a file name

FilePicker.enqueueForCleanup() calls Uri.getFileName() to name the temporary file that will be deleted later. Content URIs are processed in getFileNameForContentUris(), which used to query the content provider for OpenableColumns.DISPLAY_NAME column without accounting for some failures:

• ContentResolver.query() re throws whatever the content provider throws. For instance, an unrecognized URI can be rejected with an IllegalArgumentException
• moveToFirst() returns a boolean telling whether the move succeeded. This value was ignored and getString() could be called with an invalid cursor position, leading to a CursorIndexOutOfBoundsException

Both cases made Firefox crash. getFileNameForContentUris() now checks moveToFirst() result before calling getString(), and treats a RuntimeException from the query or from the cursor read as "no name available". A fallback name is now generated once, at the end of the function.

New unit tests have been added to cover these cases. The existing tests covering getFileNameForContentUris() have been renamed to use GIVEN ... WHEN ... THEN ... form for consistency.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/1961d663da0c
https://hg.mozilla.org/integration/autoland/rev/8b5b009b2b9d
Prevent crash when a content provider fails to return a file name r=android-reviewers,rebecatudor273

Comment 3

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/8b5b009b2b9d

Comment 4

[Corentin Bect [:corentin]]

[Tracking Requested - why for this release]: Fixed an issue that could cause Firefox to crash when selecting files from some Android file providers.

Comment 5

[Corentin Bect [:corentin]]

Release Note Request (optional, but appreciated)
[Why is this notable]:
[Affects Firefox for Android]:
[Suggested wording]: Fixed an issue that could cause Firefox to crash when selecting files from some Android file providers.
[Links (documentation, blog post, etc)]:

Comment 6

[Donal Meehan [:dmeehan]]

Thanks, added to the Fx156 nightly release notes, please allow 30 minutes for the site to update.