acra icon indicating copy to clipboard operation
acra copied to clipboard

Some Directory getFile methods are redudant or not safe at various supported API levels

Open Zardozz opened this issue 2 years ago • 0 comments

Describe the bug The EXTERNAL_FILES version of getFile method (https://github.com/ACRA/acra/blob/master/acra-core/src/main/java/org/acra/file/Directory.kt#L51 ) is redundant OR It will should generate a Null pointer exception as getExternalFilesDir will return null if used when ACRA is running from the minimum supported API (14) to API 18 because as per docs

Starting in Build.VERSION_CODES.KITKAT, no permissions are required to read or write to the returned path; it's always accessible to the calling app.

The NPE can be avoided by adding a suitable Manifest entry to the core library.

e.g. to https://github.com/ACRA/acra/blob/master/acra-core/src/main/AndroidManifest.xml add

<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" android:maxSdkVersion="18"/>

Also should really check that for any off the "EXTERNAL" methods storage is mounted as per "EXTERNAL" method in Context Docs i.e.

Shared storage may not always be available, since removable media can be ejected by the user. Media state can be checked using Environment#getExternalStorageState(File).

Also Directory.kt uses getExternalStorageDirectory and

Writing to this path requires the Manifest.permission.WRITE_EXTERNAL_STORAGE permission, and starting in Build.VERSION_CODES.KITKAT, read access requires the Manifest.permission.READ_EXTERNAL_STORAGE permission, which is automatically granted if you hold the write permission.

I've not found any direct usages, though it could probably be used in one of the optional config paths.

Expected behaviour Not to provide methods that won't work correctly at various supported by the library API versions or be configured with the right permission and state tests for them to work correct.

I would suggest that "EXTERNAL_FILES", "EXTERNAL_CACHE" and "EXTERNAL_STORAGE" variants of getFile are removed as the simplest solution.

Version

  • Android: API 14 onwards
  • ACRA 5.9.5

Zardozz avatar Aug 02 '22 13:08 Zardozz