Skip to content

fix: getFileMetadata skips NULL cursor columns instead of throwing - #12

Open
junwen-k wants to merge 1 commit into
ionic-team:mainfrom
junwen-k:fix/metadata-null-columns
Open

junwen-k wants to merge 1 commit into
ionic-team:mainfrom
junwen-k:fix/metadata-null-columns

Conversation

@junwen-k

Copy link
Copy Markdown

Description

In IONFILEContentHelper, the size, last-modified and created values are now read through a new Cursor.getLongForNames helper. The helper returns the value of the first listed column that exists and is not NULL for the current row, so when a column is NULL it tries the next one. Before, the first column that merely existed was picked, and its value was read unconditionally.

  • getCreatedTimestampForContentUri: when DATE_TAKEN is NULL, it falls back to DATE_ADDED.
  • getLastModifiedTimestampForContentUri: behaves the same for DATE_MODIFIED and COLUMN_LAST_MODIFIED, then falls back to the created time as before. The photo-picker comment is kept.
  • getSizeForContentUri: when SIZE is NULL, it falls back to openAssetFileDescriptor, which it already did when the column was missing.
  • getNameForContentUri still uses getColumnIndexForNames. When the name is NULL it keeps throwing UnknownError, as it does today.

Context

On a MediaStore URI such as content://media/external/file/18 (a PDF in Downloads), getFileMetadata throws NullPointerException: getString(...) must not be null. The Capacitor plugin reports that as OS-PLUG-FILE-0013.

MediaStore rows always have a datetaken column, but it is NULL for anything that is not a photo or video. getColumnIndexForNames chooses datetaken because the column exists, and the Kotlin null check on getString then throws. The row's _display_name, _size and date_added are all present, but they are lost with the exception.

This hits any app that receives such a URI, for example through "Open with" from Huawei's Files app. It reproduces on a stock Android 16 emulator.

The problem is not limited to MediaStore. Android documents all three numeric columns as nullable:

  • OpenableColumns.SIZE: "Null if unknown."
  • DocumentsContract.Document.COLUMN_SIZE: "null if unknown."
  • DocumentsContract.Document.COLUMN_LAST_MODIFIED: "null if unknown."

So a document provider for a remote file whose size or modified time it does not know throws in the same way. Android's own dumpImageMetaData sample calls isNull before it reads SIZE, for exactly this reason.

Fixes ionic-team/capacitor-filesystem#88

Type of changes

  • Fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Refactor (cosmetic changes)
  • Breaking change (change that would cause existing functionality to not work as expected)

Platforms affected

  • Android
  • iOS
  • JavaScript

Tests

  • The test content provider now has the column real MediaStore rows have. IONFILETestFileContentProvider gains a MediaStore.MediaColumns.DATE_TAKEN column, filled from a new TestFileContent.dateTaken: Long? = null. It defaults to NULL, which is what MediaStore does for non-media files.
  • The existing tests now cover the bug. With only that test change and no fix, the two existing getFileMetadata success tests fail at assertTrue(result.isSuccess). With the fix they pass again, still expecting TEST_TIMESTAMP (the DATE_ADDED value).
  • New test: given file with date taken, when getting metadata from content uri, date taken is returned as timestamps checks that a non-NULL DATE_TAKEN is still preferred over DATE_ADDED. Swapping the order of the two columns in getCreatedTimestampForContentUri makes this test fail.
  • ./gradlew testDebugUnitTest: 137 tests, 0 failures. Run with JDK 21 and Robolectric 4.14.

The helper reads with core-ktx's getStringOrNull, which checks isNull before reading; androidx.core:core-ktx is already a dependency. A plain getString would pass these tests too, because MatrixCursor returns null for a NULL cell. But the Cursor docs say what getString does on a NULL is implementation-defined, so the helper does not rely on it.

Checklist

  • Pull request title follows the format RNMT-XXXX <title> (external contributor, no ticket to use)
  • Code follows code style of this project
  • CHANGELOG.md file is correctly updated
  • Changes require an update to the documentation
    • Documentation has been updated accordingly

Follow-up, not in this PR: MediaStore's DATE_ADDED and DATE_MODIFIED are in seconds, while DATE_TAKEN and COLUMN_LAST_MODIFIED are in milliseconds. Timestamps read from the first two are therefore 1000× too small. Fixing that changes returned values, so I would rather raise it separately.

🤖 Generated with Claude Code

getColumnIndexForNames picks the first column that exists, and its value
was then read unconditionally. MediaStore rows always carry DATE_TAKEN,
but it is NULL for anything that is not a photo or video, so a PDF's
content:// uri threw "getString(...) must not be null".

Size, last-modified and created are now read through getLongForNames,
which takes the first listed column that exists and is not NULL for the
row, using core-ktx's getStringOrNull. SIZE and COLUMN_LAST_MODIFIED are
documented as null when unknown, so document providers are covered too.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(android): stat fails with OS-PLUG-FILE-0013 on MediaStore content:// URIs whose datetaken is NULL (e.g. PDFs)

1 participant