Skip to content

fix(sdcard): return temporary image URIs in getImage#2045

Merged
bajrangCoder merged 2 commits into
Acode-Foundation:mainfrom
bajrangCoder:fix/sdcard-getimage-temp-uri
Apr 18, 2026
Merged

fix(sdcard): return temporary image URIs in getImage#2045
bajrangCoder merged 2 commits into
Acode-Foundation:mainfrom
bajrangCoder:fix/sdcard-getimage-temp-uri

Conversation

@bajrangCoder

Copy link
Copy Markdown
Member

No description provided.

@greptile-apps

greptile-apps Bot commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes getImage to return the raw content:// URI string instead of a DocumentFile JSON object. The change is architecturally correct: ACTION_GET_CONTENT results carry only a temporary read grant and do not support takePersistableUriPermission, so the previous approach (building a DocumentFile response with takePermission) would have thrown on modern Android. Null/ClipData fallback guards are also added defensively. The TypeScript types and README are updated consistently.

Confidence Score: 5/5

Safe to merge — core logic is correct and all remaining findings are minor style suggestions.

The fix correctly aligns the Java implementation with Android's permission model for ACTION_GET_CONTENT URIs, type definitions match the new return value, and the two open comments are cosmetic (a no-op intent flag and a documentation improvement). No logic bugs or breaking regressions were found.

No files require special attention.

Important Files Changed

Filename Overview
src/plugins/sdcard/src/android/SDcard.java PICK_FROM_GALLERY result handler now correctly returns uri.toString() with added null/ClipData guards; intentionally skips takePermission (correct for ACTION_GET_CONTENT URIs); FLAG_GRANT_READ_URI_PERMISSION on outgoing intent is a harmless no-op
src/plugins/sdcard/index.d.ts getImage callback type corrected from DocumentFile to string, matching the Java implementation; temporary URI lifecycle could use a stronger warning
src/plugins/sdcard/readme.md getImage docs updated to reflect string URI return and temporary grant semantics; consistent with index.d.ts changes
bun.lock @biomejs/biome bumped 2.4.6 → 2.4.11; webpack/webpack-cli and a few transitive deps removed; routine lockfile update

Sequence Diagram

sequenceDiagram
    participant JS as JS Caller
    participant Plugin as SDcard Plugin (Java)
    participant Android as Android Gallery
    participant AR as onActivityResult

    JS->>Plugin: getImage(mimeType, callback)
    Plugin->>Android: startActivityForResult(ACTION_GET_CONTENT, PICK_FROM_GALLERY)
    Android-->>AR: RESULT_OK + data (Uri)
    AR->>AR: null check data
    AR->>AR: data.getData() → uri
    alt uri == null && ClipData present
        AR->>AR: uri = ClipData.getItemAt(0).getUri()
    end
    alt uri != null
        AR-->>JS: callback.success(uri.toString())
        Note over AR,JS: Temporary grant — valid for app process lifetime only
    else uri == null
        AR-->>JS: callback.error("No file selected")
    end
Loading

Comments Outside Diff (1)

  1. src/plugins/sdcard/src/android/SDcard.java, line 225-234 (link)

    P2 FLAG_GRANT_READ_URI_PERMISSION on outgoing intent is a no-op

    Intent.FLAG_GRANT_READ_URI_PERMISSION is set on the outgoing ACTION_GET_CONTENT intent (line 230), but this flag only has meaning when returning a URI to another app via setResult. The gallery app automatically grants a read URI permission to the caller on its own — this flag on the request intent is silently ignored by Android and may mislead future maintainers into thinking a grant is being explicitly requested.

Reviews (1): Last reviewed commit: "fix(sdcard): return temporary image URIs..." | Re-trigger Greptile

Comment thread src/plugins/sdcard/index.d.ts
@bajrangCoder bajrangCoder merged commit 6121291 into Acode-Foundation:main Apr 18, 2026
6 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in The Code Board - Acode Apr 18, 2026
@bajrangCoder bajrangCoder deleted the fix/sdcard-getimage-temp-uri branch April 18, 2026 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant